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

Support passing parse mode to parse/fingerprint functions #216

Merged
merged 4 commits into from
Dec 20, 2023

Conversation

lcheruka
Copy link
Contributor

Fixes #215

Added parseMode to json output for PLpgSQL_expr struct
src/pg_query_parse.c Outdated Show resolved Hide resolved
@lfittl
Copy link
Member

lfittl commented Dec 4, 2023

Mainly commenting so I don't forget:

For the newly introduced _opt functions, I wonder if we should also consider the use cases requested in #50 here, i.e. to set GUCs that affect parsing.

Need to think through this more how it could be part of the API, but might be worth doing now so we don't have to break the _opt function API in the future.

Most importantly, this allows turning off standard_conforming_strings
to support legacy applications. It also adds support for turning
off backslash_quote and escape_string_warning.

These settings can be set by passing the following flags to the _opts
function variants. Use bitwise OR to combine flags, as well as set the
parse mode.

- PG_QUERY_DISABLE_BACKSLASH_QUOTE (backslash_quote = off)
- PG_QUERY_DISABLE_STANDARD_CONFORMING_STRINGS (standard_conforming_strings = off)
- PG_QUERY_DISABLE_ESCAPE_STRING_WARNING (escape_string_warning = off)

Fixes pganalyze#50
@lfittl
Copy link
Member

lfittl commented Dec 20, 2023

Need to think through this more how it could be part of the API, but might be worth doing now so we don't have to break the _opt function API in the future.

I've gone ahead and done that change now on this branch (thanks for allowing maintainer edits!), by utilizing an integer as a bitmask instead of a simple enum. From a usability perspective this is essentially the same as if you just care about the parse mode, but if you want to set the options you can just use bitwise OR to enable them.

Overall I think I'm happy with this now. @seanlinsley might be worth if you take another look to see if there are any concerns with that last commit, but otherwise I think we can merge.

@lfittl lfittl merged commit 7a819b4 into pganalyze:15-latest Dec 20, 2023
16 checks passed
@lfittl
Copy link
Member

lfittl commented Dec 20, 2023

@lcheruka Thanks for the contribution! Merged with slight revisions to allow passing additional options beyond the parse mode.

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

Successfully merging this pull request may close these issues.

Parsing queries from plpgsql
4 participants