capacitor-community / sqlite

Community plugin for native & electron SQLite databases
MIT License
426 stars 104 forks source link

Unable to run `executeSet` on SQL with `--` comments #521

Closed msfstef closed 2 months ago

msfstef commented 2 months ago

Describe the bug Using the executeSet API, running any SQL statement with comments in it (either -- or \* ... *\) results in:

ExecuteSet: error code 21: not an error

Removing the comments makes the statements run perfectly fine.

To Reproduce Run any SQL statement with comments in, e.g.:

-- foobar
SELECT * FROM bar;

or

/* foobar */
SELECT * FROM bar;

Expected behavior The executeSet API should be able to run SQL with comments

Context Same issue occurs on both iOS and Android with version 5.6.1 of the package.

jepiqueau commented 2 months ago

@msfstef

Having said that i will check again the executeSet with comments and revert back to you

msfstef commented 2 months ago

the executeSet command is for execute Queries like INSERT, UPDATE, DELETE ... but not for the SELECT

Sorry the example was perhaps not the best - we're running into this issue when creating tables and inserting rows etc, see adapter implementation for details.

the chapter Comments within SQL statements of the API.md file give a link to Comments within SQL

Other drivers (like the expo one, the react-native-sqlite-storage, wa-sqlite, etc) all work with the integrated comments so I'm sure it's not an issue with the way the statements and comments are formulated. e.g. for an actual statement that breaks:

-- Toggles for turning the triggers on and off
INSERT OR IGNORE INTO _electric_trigger_settings(tablename,flag) VALUES ('${tableFullName}', 1);
jepiqueau commented 2 months ago

@msfstef i test this

      let insertQuery = 'INSERT INTO contacts (name,FirstName,email,company,age,MobileNumber) VALUES (?, ?, ?, ?, ?, ?) -- Add Sue Hellen;';
      let bindValues = ["Hellen","Sue","sue.hellen@example.com",,42,"4406050807"];
      let ret = await db.run(insertQuery, bindValues);
      console.log(`>>> run ret 1: ${JSON.stringify(ret)}`)
      insertQuery = `INSERT INTO contacts /* some contacts */ (name,FirstName,email,company,age,MobileNumber) VALUES 
          ('Doe','John','john.doe@example.com', 'IBM', 30, '4403050926'), -- Add Doe
          ('Watson','Dave','dave.watson@example.com','Apple', 30, '4407050932') /* add Watson */,
          ('Smith', 'Jane', 'jane.smith@example.com', 'IBM', 27, '33607556142') /* Add Smith */-- End of add contact;`;
      bindValues = [];
      ret = await db.run(insertQuery, bindValues);
      console.log(`>>> run ret 2: ${JSON.stringify(ret)}`)

      let selectQuery = "SELECT * /* all columns */ FROM contacts WHERE company = 'IBM' -- for company IBM;";

      ret = await db.query(selectQuery);
      console.log(`>>> query "IBM" ret: ${JSON.stringify(ret)}`)

and it works and i test this

  const setContacts: Array<capSQLiteSet>  = [
    { statement:"INSERT INTO contacts /* Contact Simpson */ (name,FirstName,email,company,age,MobileNumber) VALUES (?,?,?,?,?,?);",
      values:["Simpson","Tom","Simpson@example.com",,69,"4405060708"]
    },
    { statement:"INSERT INTO contacts /* three more contacts */ (name,FirstName,email,company,age,MobileNumber) VALUES (?,?,?,?,?,?) -- Add Jones, Whiteley and Brown;",
      values:[
        ["Jones","David","Jones@example.com",,42.1,"4404030201"],
        ["Whiteley","Dave","Whiteley@example.com",,45.3,"4405162732"],
        ["Brown","John","Brown@example.com",,35,"4405243853"]
      ]
    },
    { statement:"UPDATE contacts SET age = ? , MobileNumber = ? WHERE id = ? -- Update Jones Contact;",
      values:[51.4,"4404030202",6]
    }
  ];

      ret = await db.executeSet(setContacts);

      selectQuery = "SELECT email /* only email */ FROM contacts WHERE company IS NULL -- for company not given;";
      ret = await db.query(selectQuery);
      console.log(`>>> query "NULL" ret: ${JSON.stringify(ret)}`)

and it works

the database where created with

        CREATE TABLE IF NOT EXISTS contacts (
            id INTEGER PRIMARY KEY NOT NULL,
            email TEXT UNIQUE NOT NULL,
            name TEXT,
            FirstName TEXT,
            company TEXT,
            size REAL,
            age INTEGER,
            MobileNumber TEXT
        );
        CREATE INDEX IF NOT EXISTS contacts_index_name ON contacts (name);
        CREATE INDEX IF NOT EXISTS contacts_index_email ON contacts (email);
jepiqueau commented 2 months ago

@msfstef I cannot access actual statement

msfstef commented 2 months ago

@msfstef I cannot access actual statement

Sorry I used the wrong link, here you go

FWIW most of them execute a set that has no bound parameters (and so an empty list [] for the params) - will play around a bit more to see if I can get a more minimal reproducible example.

jepiqueau commented 2 months ago

@msfstef Your comments are outside the SQL statements so i will have to look at it later

msfstef commented 2 months ago

@jepiqueau after some more testing it seems \* ... *\ comments work, but -- comments only work at the end of statements.

Having a -- comment either in between lines or outside of the statement separated by a line break doesn't work.

CREATE TABLE IF NOT EXISTS main.test (
  id INTEGER PRIMARY KEY AUTOINCREMENT, -- testing
  version TEXT NOT NULL UNIQUE,
  applied_at TEXT NOT NULL
);`

-- Error: prepareSQL prepare failed rc: 1 message: incomplete input
-- testing
CREATE TABLE IF NOT EXISTS main.test (
  id INTEGER PRIMARY KEY AUTOINCREMENT,
  version TEXT NOT NULL UNIQUE,
  applied_at TEXT NOT NULL
);`

-- Error: prepareSQL step failed rc: 21 message: not an error

Only one that works is like the one you wrote above:

CREATE TABLE IF NOT EXISTS main.test (
  id INTEGER PRIMARY KEY AUTOINCREMENT,
  version TEXT NOT NULL UNIQUE,
  applied_at TEXT NOT NULL
) -- testing;`

-- successful

Although placing the comment after the semicolon breaks with error 21 again.

jepiqueau commented 2 months ago

@msfstef if you look at the link i gave earlier it said In SQLite, a comment started with -- symbol is similar to a comment starting with # symbol. When using the -- symbol, the comment must be at the end of a line in your SQL statement with a line break after it. This method of commenting can only span a single line within your SQL and must be at the end of the line.

jepiqueau commented 2 months ago

@msfstef if you look at ChatGPT

msfstef commented 2 months ago

@jepiqueau yes - all comments starting from -- to the end of the line are valid, as long as the only thing that follows is a line break, like in the examples I gave above.

In fact the only one that works is one that I feel should not work, as it includes the semicolon inside the comment.

-- Add some employees INSERT INTO employees (name, age, department) VALUES ('John Doe', 30, 'Sales'), -- Add John Doe ('Jane Smith', 25, 'Marketing'); -- Add Jane Smith is incorrect

this one is incorrect because it includes a -- comment inline without a line break. If it was:

-- Add some employees
INSERT INTO employees (name, age, department) VALUES ('John Doe', 30, 'Sales'),
-- Add John Doe
('Jane Smith', 25, 'Marketing'); -- Add Jane Smith

it would be correct - you can try any SQLite implementation to check that (example)

jepiqueau commented 2 months ago

@msfstef if you look at what i gave earlier

 insertQuery = `INSERT INTO contacts /* some contacts */ (name,FirstName,email,company,age,MobileNumber) VALUES 
          ('Doe','John','john.doe@example.com', 'IBM', 30, '4403050926'), -- Add Doe
          ('Watson','Dave','dave.watson@example.com','Apple', 30, '4407050932') /* add Watson */,
          ('Smith', 'Jane', 'jane.smith@example.com', 'IBM', 27, '33607556142') /* Add Smith */-- End of add contact;`;

it works, the reason it is at the end of the first value so it is correct it has to be after the comma . i also test this and it works to

        CREATE TABLE IF NOT EXISTS contacts (
            id INTEGER PRIMARY KEY NOT NULL, -- testing
            email TEXT UNIQUE NOT NULL,
            name TEXT,
            FirstName TEXT,
            company TEXT,
            size REAL,
            age INTEGER,
            MobileNumber TEXT
        );
msfstef commented 2 months ago

@jepiqueau I agree that the comments need to be after all the SQL statements, so from -- to the linebreak \n is commented out - if it was before the comma, the comma would be included in the comment and thus there would be a syntax error.

As for the last example with creating a table, I get an error of Error: prepareSQL prepare failed rc: 1 message: incomplete input as mentioned above in my first snippet of creating a table with a comment at the end of the line. I'm trying this on iOS with the package version 5.6.1

Also, comments don't work before or after the statement on their own line, although they should by the spec

jepiqueau commented 2 months ago

@msfstef i test it only for the time being on Web so i look for iOS and Android

jepiqueau commented 2 months ago

@msfstef

CREATE TABLE IF NOT EXISTS contacts (
            id INTEGER PRIMARY KEY NOT NULL, -- testing
            email TEXT UNIQUE NOT NULL,
            name TEXT,
            FirstName TEXT,
            company TEXT,
            size REAL,
            age INTEGER,
            MobileNumber TEXT
        );

and ``ts INSERT INTO contacts /* some contacts */ (name,FirstName,email,company,age,MobileNumber) VALUES ('Doe','John','john.doe@example.com', 'IBM', 30, '4403050926'), -- Add Doe ('Watson','Dave','dave.watson@example.com','Apple', 30, '4407050932') /* add Watson */, ('Smith', 'Jane', 'jane.smith@example.com', 'IBM', 27, '33607556142') /* Add Smith */-- End of add contact;;


these two do not work on iOS and Android.
There is not much i can do for this as it is the internal sqlite commands which send an error message which is not the case for the web sqlite
jepiqueau commented 2 months ago

@msfstef after some moifications, i can make this working in iOS,

      insertQuery = `INSERT INTO contacts /* some contacts */ (name,FirstName,email,company,age,MobileNumber) VALUES 
          ('Doe','John','john.doe@example.com', 'IBM', 30, '4403050926') -- add Doe,
          ('Watson','Dave','dave.watson@example.com','Apple', 30, '4407050932') /* add Watson */,
          ('Smith', 'Jane', 'jane.smith@example.com', 'IBM', 27, '33607556142') /* Add Smith */-- End of add contact;`;

Notice that -- add Doe is before the comma and not after and

  const setMessages: Array<capSQLiteSet>  = [
    { statement:`
    /* Define the messages table */
    CREATE TABLE IF NOT EXISTS messages (
      id INTEGER PRIMARY KEY NOT NULL,
      contactid INTEGER, -- key to contacts(id)
      title TEXT NOT NULL,
      body TEXT NOT NULL,
      last_modified INTEGER DEFAULT (strftime('%s', 'now')),
      FOREIGN KEY (contactid) REFERENCES contacts(id) ON DELETE SET DEFAULT
    );`,
      values:[]
    },
  ];

      ret = await db.executeSet(setMessages);

i have now to check in Android, Electron and Web if it works

msfstef commented 2 months ago

Looking into the iOS code, for executeSet the statements first go through UtilSQLCipher.executeSet, then they get processed by UtilSQLStatement.getStmtAndRetColNames, which in turn calls the confusing UtilSQLStatement.isReturning that mutates the statements themselves.

See these lines: https://github.com/capacitor-community/sqlite/blob/e0466f4a4b03c62af8ec51677d60bb0125b0ddc0/ios/Plugin/Utils/UtilsSQLStatement.swift#L328-L330

The isReturning should not really mutate the provided SQL statement (the statement should end up in the underlying SQLite implementation as is, which clearly handles comments as per the spec), but it does by removing all newlines and messing with semicolons, ultimately making the syntax invalid as the newlines are an important part of determining what is a comment.

I've messed around a bit and not mutating the statement within isReturning comments work as they should. I'm reluctant to open a PR as I'm not fully understanding what isReturning is needed for (it seems to split some statements from their RETURNING clauses), but the parsing and mutating happening there is the culprit for the lack of comment support. You can see that the execute API does not go through this route and supports comments properly.

jepiqueau commented 2 months ago

@msfstef Thanks for your help in searching for a fix. i will have a look back starting on your finding and see how i can solve this.

jepiqueau commented 2 months ago

@msfstef the returning mode is use when you have in an INSERT, ... clause a RETURNING keyword meaning that you want to get the result of the whatever clause used

jepiqueau commented 2 months ago

@msfstef Fix in v5.6.2 i add some comment examples in the API. Can you check on your side and please close the issue when done

jepiqueau commented 2 months ago

@msfstef Is it working now?

msfstef commented 2 months ago

@jepiqueau yep tested on both android and iOS, working fine now!

I've encountered another issue on Android only with statements that are not properly terminated with a semicolon but I'll raise a separate issue if I get to the bottom of it.

Thanks for sorting this out!

jepiqueau commented 2 months ago

@msfstef the statements must end with ; you do not need to raise à New issue