Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Importing from SQL dump fails because of bugged removeComments(sql) #51

Open
pgiobbi0 opened this issue Mar 15, 2020 · 1 comment
Open

Comments

@pgiobbi0
Copy link

I have encountered some issues when importing data from SQL dumps through the sqlitePorter.importSqlToDb function

Reproduction steps:

version: 1.1.1

Try to ingest

CREATE TRIGGER IF NOT EXISTS update_person_updated_at AFTER UPDATE ON person
BEGIN
    UPDATE person SET updated_at = (strftime('%Y-%m-%d %H:%M:%f', 'now', 'utc')) WHERE id = OLD.id;
END;

Error

"Failed to import SQL; message=sqlite3_prepare_v2 failure: incomplete input","code":5,"statement":"CREATE TRIGGER update_person_updated_at UPDATE ON person\nBEGIN\n    UPDATE person SET updated_at = (strftime('%Y-%m-%d %H:%M:%f', 'now', 'utc')) WHERE id = OLD.id"}

I believe that the bug is caused by the removeComments(sql) function, which I have inspected and shows that the SQL command is interpreted as two different statements:

  1. First statement: BEGIN UPDATE person SET updated_at = (strftime('%Y-%m-%d %H:%M:%f', 'now', 'utc')) WHERE id = OLD.id
  2. Second statement: END

This of course is not right since it must be interpreted as a single SQL statement.

I have made a temporary fix with:

sqlitePorter.importSqlToDb = function (db, sql, opts){
    opts = opts || {};
    if(!isValidDB(db, opts)) return;
    db.transaction(function(tx) {
        try {
            //Clean SQL + split into statements
            var totalCount, currentCount;

            var tempStatements = removeComments(sql).match(statementRegEx);;
            var statements = [];
            for (let i = 0; i < tempStatements.length; i++) {
                if (tempStatements[i].toUpperCase() === 'END') {
                    statements[statements.length - 1] += ';\nEND';
                } else {
                    statements.push(tempStatements[i]);
                }
            }
# ...

Which works, but it the best approach is solving it directly inside the removeComments function.

It would be nice to have an official fix. Let me know if more details are needed.

@Ryk97
Copy link

Ryk97 commented Feb 14, 2021

I encountered the same problem when trying to create Triggers within a seed.sql file.
However I do not think that the problem is lying in the removeComments(sql) function.

Instead the problem is the RegEx which is used to match each query. This query expects the semicolon as the separator between the queries and will therefore return one match for each string that ends with a semicolon, as long as it is not encapsulated with either singel or double quotation marks.

I think an actual solution would be to enable the usage of another options parameter called e.g. 'separator'. Using this you should be able to specify the separator that shall be used to separate the SQL statements from your SQL file. The semicolon in the statementRegEx would then have to be changed to the separator that has been specified and the custom RegEx has to be used for splitting the statements.

If I find time, I will try to come up with a possibility to do this and post the solution here and open a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants