sqlite icon indicating copy to clipboard operation
sqlite copied to clipboard

Multi row statement should fail if no values are replaced

Open lasher23 opened this issue 9 months ago • 4 comments

Is your feature request related to a problem? Please describe. Currently it is not easy visible if you make a mistake while using the multi row statements.

Describe the solution you'd like If nothing is replaced with the regex i would like that an error is thrown. Replace the following: image With something like this:

    public String replacePlaceholders(String stmt, String sqlBuilder) {
        return replaceWithCheck(stmt, "(?i)VALUES \\((\\?(?:,\\s*\\?\\s*)*)\\)",  "VALUES " + sqlBuilder)
    }

    public static String replaceWithCheck(String input, String regex, String replacement) throws Exception {
        String newString = input.replaceAll(regex, replacement);
        if (input.equals(newString)) {
            throw new Exception("No values found to replace for multi row statements");
        }
        return newString;
    }

lasher23 avatar Apr 30 '24 16:04 lasher23

@lasher23 not really it is for the case where you have "INSERT INTO TABLE (name, email) VALUES ('Jeep','[email protected]') " this should pass through

jepiqueau avatar Apr 30 '24 17:04 jepiqueau

@jepiqueau but then you dont provide a two dimensional array as parameter. The logic is only triggered if the array is a two dimensional array.

lasher23 avatar Apr 30 '24 18:04 lasher23

@lasher23 i will have an other look give me an example of statements that you want me to add to my test program thanks

jepiqueau avatar Apr 30 '24 20:04 jepiqueau

@jepiqueau If i call executeSet with a two dimensional array. I always want to get into the multi statement logic. A simple example is i dont get it that i have to pass ? as parameter. What you are doing now is just execute the statement with an empty array as args which makes it replace it with null.

For example the following code raises no error: executeSet("insert into table", [['a'],['b']])

This for sure is an error in my statement. But it would be very nice if the Plugin tells me that i am not allowed to execute this. For example it was very hard to spot the bugs with the case insensitive 'VALUES' because there was never an error from the plugin, it just did not replace the values.

lasher23 avatar May 07 '24 13:05 lasher23

@lasher23 What you show is a pure SQLite syntax error INSERT without VALUES or insert with number of ? not equal to the number of table's column, the plugin cannot check for any possible mistake or syntax error that the developer is doing. all the syntax of sqlite statements are either given in the plugin docs or in the web SQLite doc. Sorry for you that your waste some time but it is part of the learning process.

jepiqueau avatar Jun 09 '24 13:06 jepiqueau

@jepiqueau i dont agree with you

My example is wrong sql. My request for this is because of bugs in your previous implementation.

Lets take the example that values was needed to be uppercase for your multi statement regex to work. This has nothing to do with the SQLite Logic but with your plugin. Your implementation decides to replace multi value statements in that way. Now with your logic of just providing an empty list as arguments, when nothing is replaced, it replaces every question mark with null.

My statement with lower case

insert into table(name) values (?)

This inserted null. This was extremly hard to spot because the call was successfull, but my values from the parameters were not respected because of the bug that no values in lowercase was supported.

In my Opinion it is just bad to default my provided arguments with empty. Because as soon as i provide a two dimensional array as argument i expect that the multi value statment is triggered. And if its not triggered because of either a bug in your implementation or my call i want an error. This has nothing to do with SQLite or any learning Curve. This would just be a more developer friendly implementation.

And if a developer calls executeSet with a two dimensional array as second arg and does not want it to replace values, its just wrong usage of the plugin and an error is not wrong

For Example: exeucteSet("insert into table(name) values ('hello'), [["hello"]]")

lasher23 avatar Jun 17 '24 11:06 lasher23

@lasher23 i will not argue on this.

jepiqueau avatar Jun 18 '24 10:06 jepiqueau