PARSE-QUERY: separate single-quote quoting from dollar quoting in parser

[phoe]

Attempts to fix #944

I do not know how to run the pgloader unit/regression test suite - please tell me how to do it and/or verify that my patch does not break anything

TODO: Insert a regression test in the automated test suite that ensures that the following two queries are read properly.

DO $$ BEGIN
  -- ';'
END; $$ LANGUAGE plpgsql;

DO $A$ BEGIN
  DO $B$ BEGIN
    -- ';'
  END; $B$ LANGUAGE plpgsql;
END; $A$ LANGUAGE plpgsql;

[phoe]

The following still fails to parse even after my PR:

SELECT '$$ DO $$';

[phoe]

The new commit seems to fix the issue.

CL-USER> (with-input-from-string (s "SELECT '$$ $$';")
           (pgloader.sql::parse-query s))
"SELECT '$$ $$'"

CL-USER> (with-input-from-string (s "DO $$ BEGIN
        -- ';'
END; $$ LANGUAGE plpgsql;")
           (pgloader.sql::parse-query s))
"DO $$ BEGIN
        -- ';'
END; $$ LANGUAGE plpgsql"

CL-USER> (with-input-from-string (s "DO $A$ BEGIN
  DO $B$ BEGIN
    -- ';'
  END; $B$ LANGUAGE plpgsql;
END; $A$ LANGUAGE plpgsql;")
           (pgloader.sql::parse-query s))
"DO $A$ BEGIN
  DO $B$ BEGIN
    -- ';'
  END; $B$ LANGUAGE plpgsql;
END; $A$ LANGUAGE plpgsql"

[dimitri]

Your current PR includes a FASL file it seems, of course I won't include it in the final merge. Also, you mentioned on IRC wanting to add unit testing to the SQL parser. This PR might be a nice place to do that, what do you think?

[phoe]

Welp - I should not have committed the FASL file. I will skip it.

Sure - do you have any examples of SQL syntax that should be declined by this parser? I will do my best to prepare such myself, but external examples will also be handy.

[phoe]

There - PR cleaned up. Sorry for the massive delay and the duplicate - I have realized that I've already mentioned (and fixed!) this issue only after I've submitted the duplicate.

As for tests for this, @sabracrolleton has created a series of tests for the version we've pulled into postmodern at https://github.com/marijnh/Postmodern/tree/master/postmodern/tests - you should be able to incorporate them back into pgloader.

[dimitri]

Hi @phoe ; did you figure out how to run make check or make check-saved to pass the unit tests locally? They depend only on having a local Postgres instance, you don't need fancy source support such as SQLite or MySQL, though it's good practice to also have those handy and manually run some of the test cases for them.

Are you interested in contributor access to this repository and merging your PR, maybe with more tests if you feel like it? At the moment the make check test suite does not contain unit tests, only integration tests for things that are supposed to be working... So having a manual way to tests things from the SLIME REPL would be considered good enough here, really.

[phoe]

Hey - I haven't had the time to figure that out. Maybe next week I will find some time and set up a pgloader development environment on my machine, though, so let's keep our hopes up.

Once I do that, I'll come back to this issue, and possibly try to import Sabra's tests and integrate it into pgloader's unit test suite.

[dimitri]

Ping? anything we should still do about this one?

[phoe]

Yes - I still haven't written tests for that one. I'll assign myself to it for the time being and poke it in spare time.

[ffried]

Any news on this? I just ran into an issue, that seemed unrelated at first, but is also fixed by this PR: #1585.

[ffried]

Note that single quotes in comments still lead to an error:

E.g. the following SQL file:

-- Don't use single quotes in comments.
DO $$
BEGIN
    EXECUTE 'SELECT 1;';
END;
$$;

Also, $ have the same problem. This also fails:

DO $$
BEGIN
    PERFORM 'abc' ~ 'bc$';  -- The dollar sign on this line causes the problem.
END;
$$;