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

fix: always allow metadata queries #2580

Merged
merged 6 commits into from
Aug 11, 2023
Merged

Conversation

olavloite
Copy link
Collaborator

Internal metadata queries should always be allowed, regardless of the type of transaction that is currently running or any special mode that has been set on the connection. Internal metadata queries are system queries that are generated by a connection to return metadata to an application, for example methods like getTables() in JDBC.

Internal metadata queries should always be allowed, regardless of the type
of transaction that is currently running or any special mode that has been
set on the connection. Internal metadata queries are system queries that
are generated by a connection to return metadata to an application, for
example methods like getTables() in JDBC.
@olavloite olavloite requested a review from a team as a code owner August 11, 2023 05:29
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner API. labels Aug 11, 2023
@olavloite olavloite requested a review from rajatbhatta August 11, 2023 05:30
@@ -133,17 +133,6 @@ enum BatchMode {
DML
}

/**
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was dead code. Only the class in the public Connection interface was used.

@@ -121,29 +118,6 @@ public ApiFuture<ResultSet> executeQueryAsync(
final ParsedStatement statement,
AnalyzeMode analyzeMode,
QueryOption... options) {
if (options != null) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This special handling is no longer needed, as it is generically handled in the connection.

@gcf-owl-bot gcf-owl-bot bot requested a review from a team as a code owner August 11, 2023 05:32
@@ -157,25 +157,6 @@ public void testExecuteCreateDatabase() {
.parse(Statement.of("CREATE DATABASE foo"))));
}

@Test
public void testExecuteMetadataQuery() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test specifically checked whether a DDL batch could execute an internal metadata query. That is no longer needed, as a DDL batch no longer handles that special case itself. Instead, this is handled directly in the connection for all types of transactions.

@@ -157,25 +157,6 @@ public void testExecuteCreateDatabase() {
.parse(Statement.of("CREATE DATABASE foo"))));
}

@Test
public void testExecuteMetadataQuery() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test specifically checked whether a DDL batch could execute an internal metadata query. That is no longer needed, as a DDL batch no longer handles that special case itself. Instead, this is handled directly in the connection for all types of transactions.

Copy link
Contributor

@rajatbhatta rajatbhatta left a comment

Choose a reason for hiding this comment

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

LGTM, apart from just one nit.

@olavloite olavloite merged commit ebb17fc into main Aug 11, 2023
@olavloite olavloite deleted the allow-internal-metadata-queries branch August 11, 2023 17:07
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 API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants