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

feat: support PreparedStatement#getParameterMetaData() #1218

Merged
merged 7 commits into from Dec 22, 2023

Conversation

olavloite
Copy link
Collaborator

@olavloite olavloite commented Apr 29, 2023

Add actual support for PreparedStatement#getParameterMetaData(). The first time this method is called for a PreparedStatement, the connection will now call Connection#analyzeQuery or Connection#analyzeUpdate without any parameter values. This will instruct Cloud Spanner to return the names and types of any query parameters in the statement. The information that is returned is used to create an instance of ParameterMetaData.

Previously, this class was only constructed with information based on values that had already been set on the statement, which meant that it could not be used to determine what types that Cloud Spanner expected for a given query parameter.

Fixes #35

Add actual support for `PreparedStatement#getParameterMetaData()`. The first time
this method is called for a PreparedStatement, the connection will now send the
query to Cloud Spanner in analyze mode and without any parameter values. This
will instruct Cloud Spanner to return the names and types of any query parameters
in the statement.

Fixes #35
@olavloite olavloite requested a review from a team as a code owner April 29, 2023 05:18
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner-jdbc API. labels Apr 29, 2023
@olavloite olavloite added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 29, 2023
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Dec 18, 2023
@olavloite olavloite removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 18, 2023
@@ -69,7 +70,74 @@ static int extractColumnType(Type type) {
}
}

/** Extract Spanner type name from {@link java.sql.Types} code. */
static String getSpannerTypeName(Type type, Dialect dialect) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would creating enum here( with both names for each type) improve readability?
In general another possibility could be to have a structure like a type manager for mapping pg, googlesql, java types, so the type checking and conversions like in functions such as ParameterTypeFromValue can also be simplified from one place?

Copy link
Collaborator Author

@olavloite olavloite Dec 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My plan was to add that to the Java client library instead of directly in the JDBC driver, so we can use it in more places. I've expedited that and added a PR for it here: googleapis/java-spanner#2763

When that has been merged, a new release of the client library has been cut, and the dependency in the JDBC driver updated to the new version, then we can simplify this implementation.

this.statement = statement;
statement.getParameters().fetchMetaData(statement.getConnection());
this.parameters = resultSet.getMetadata().getUndeclaredParameters();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my understanding of this field from Cloud Spanner code, UndeclaredParameters field only returns types for untyped parameters, i.e. when the types of the parameters are not passed in from the client and they are sent as proto.value.
So is this the case for all the prepared statements we execute from the JDBC driver?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case the name of this field in the proto definition is a bit unfortunate. It says 'undeclared parameters', but it actually returns information about all parameters, including the ones that included a type and/or value when it was sent by the client. So if the user included type information for one or more parameters, it will also show up in this map.

@olavloite olavloite requested a review from manu2 December 20, 2023 14:52
@olavloite olavloite merged commit 721ff45 into main Dec 22, 2023
25 checks passed
@olavloite olavloite deleted the prepared-statement-param-metadata branch December 22, 2023 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-spanner-jdbc API. size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Let ParameterMetadata for PreparedStatement report correct ParameterType before execution
2 participants