-
Notifications
You must be signed in to change notification settings - Fork 123
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
feat: parse query parameters in PostgreSQL query #1732
Conversation
PostgreSQL supports newline characters in string literals and quoted identifiers. Trying to execute a statement with a string literal or quoted identifier that contained a newline character would cause an 'Unclosed string literal' error. Fixes #1730
Adds a helper method to get the parameters from a PostgreSQL query. This is needed for DESCRIBE statement messages in PGAdapter, as it must return the data types of all query parameters in a query string. Even though this parser is not able to determine the parameter types, it is able to determine the number of parameters. This again makes it possible to PGAdapter to return Oid.UNSPECIFIED for each parameter in the query string, which is enough for most clients.
* @return A set containing all the parameters in the SQL-string. | ||
*/ | ||
@InternalApi | ||
public Set<String> getQueryParameters(String sql) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine we don't need to consider malformed queries here (select col$4" from some_table ...
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. That is caught by the preceding step (removeCommentsAndTrim
) that should be taken by any well-informed users of the API.
char currentChar = sql.charAt(currentIndex); | ||
if (currentChar == SINGLE_QUOTE || currentChar == DOUBLE_QUOTE) { | ||
result.append(currentChar); | ||
appendIfNotNull(result, currentChar); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: will it ever be null
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, result
will be null if the caller does not need the result. That's a (minor) performance optimization not to build a resulting query string when the call just wants to scan through the SQL string to extract specific information. So in this specific case we skip building a result string as we are only interested in extracting the query parameters, and not building a new SQL string with for example a different style query parameters (e.g. when converting JDBC style ?
parameters to PostgreSQL style parameters $1
).
…cp/templates/java_library/.kokoro (#1732) build(deps): bump certifi Bumps [certifi](https://github.com/certifi/python-certifi) from 2022.9.24 to 2022.12.7. - [Release notes](https://github.com/certifi/python-certifi/releases) - [Commits](certifi/python-certifi@2022.09.24...2022.12.07) --- updated-dependencies: - dependency-name: certifi dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jeff Ching <[email protected]> Source-Link: googleapis/synthtool@ae0d43e Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:9de537d592b60e5eac73b374a28263969bae91ecdb29b445e894576fbf54851c
…cp/templates/java_library/.kokoro (#1732) (#2194) build(deps): bump certifi Bumps [certifi](https://togithub.com/certifi/python-certifi) from 2022.9.24 to 2022.12.7. - [Release notes](https://togithub.com/certifi/python-certifi/releases) - [Commits](https://togithub.com/certifi/python-certifi/compare/2022.09.24...2022.12.07) --- updated-dependencies: - dependency-name: certifi dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jeff Ching <[email protected]> Source-Link: https://togithub.com/googleapis/synthtool/commit/ae0d43e5f17972981fe501ecf5a5d20055128bea Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:9de537d592b60e5eac73b374a28263969bae91ecdb29b445e894576fbf54851c
Adds a helper method to get the parameters from a PostgreSQL query. This
is needed for DESCRIBE statement messages in PGAdapter, as it must
return the data types of all query parameters in a query string. Even
though this parser is not able to determine the parameter types, it is
able to determine the number of parameters. This again makes it possible
to PGAdapter to return Oid.UNSPECIFIED for each parameter in the query
string, which is enough for most clients.
cc @pratickchokhani