Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
PARSE-QUERY: separate single-quote quoting from dollar quoting in parser #945
Conversation
|
The following still fails to parse even after my PR: SELECT '$$ DO $$'; |
|
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" |
|
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? |
|
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. |
|
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. |
Fix for SELECT '$$ $$'. Fixes #944.
|
Hi @phoe ; did you figure out how to run 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 |
|
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. |
|
Ping? anything we should still do about this one? |
|
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. |
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.