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

Postgres: Correct parsing of multiline statements #8512

Merged
merged 10 commits into from
Mar 17, 2020

Conversation

tyrannosaurus-becks
Copy link
Contributor

Fixes #6098

Postgres was splitting statements on a semicolon. This would be fine normally, but sometimes people do send statements with semicolons that need to be parsed as a single query.

Example:

// User sends:
DO $$ BEGIN IF NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname='my_role') THEN CREATE ROLE my_role; END IF; END $$

// In the current state, Postgres executes the first piece as an independent piece
// causing a syntax error: 
DO $$ BEGIN IF NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname='my_role') THEN CREATE ROLE my_role

// After this PR, it will be sent as the original syntax:
DO $$ BEGIN IF NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname='my_role') THEN CREATE ROLE my_role; END IF; END $$

@tyrannosaurus-becks tyrannosaurus-becks added bug Used to indicate a potential bug secret/database labels Mar 9, 2020
@tyrannosaurus-becks tyrannosaurus-becks added this to the 1.4 milestone Mar 9, 2020
@ncabatoff
Copy link
Collaborator

I wonder if it would make more sense to define a multiline string as one starting with DO . That's AFAIK the only way to begin a multi-line statement. And although dollar quoting is recommended for the body of the anonymous DO function, it's not required, so in theory they could put a multi-line statement within single quotes I think.

@tyrannosaurus-becks
Copy link
Contributor Author

Hi @ncabatoff ! Thanks for the review! :-)

Regarding this:

I wonder if it would make more sense to define a multiline string as one starting with DO . That's AFAIK the only way to begin a multi-line statement. And although dollar quoting is recommended for the body of the anonymous DO function, it's not required, so in theory they could put a multi-line statement within single quotes I think.

I did some poking around and it looks like it would be possible for multiline statements to not include DO.

@ncabatoff
Copy link
Collaborator

I did some poking around and it looks like it would be possible for multiline statements to not include DO.

Hmm, you're right. I was making the assumption that they wouldn't be issuing DDL statements because that would be weird, but people do weird stuff all the time...

ncabatoff
ncabatoff previously approved these changes Mar 16, 2020
@tyrannosaurus-becks tyrannosaurus-becks merged commit 87d7180 into master Mar 17, 2020
@tyrannosaurus-becks tyrannosaurus-becks deleted the 6098-postgres-statement-parsing branch March 17, 2020 19:45
@pbernal pbernal modified the milestones: 1.4, 1.5 Mar 17, 2020
@Legogris
Copy link

Looking in the changelog, it seems this has not been released yet? Was it simply omitted from the changelog or is it yet to be released?

(Encountering the same issue in 1.5.0)

@ncabatoff
Copy link
Collaborator

Looking in the changelog, it seems this has not been released yet? Was it simply omitted from the changelog or is it yet to be released?

(Encountering the same issue in 1.5.0)

Sorry, we missed adding it to the 1.5.0 changelog (I just added it there now.). Please open a new issue if you're still having problems of this sort with 1.5.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug secret/database
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vault splits PostgreSQL policies on literal semicolons, breaking block statement
6 participants