-
Notifications
You must be signed in to change notification settings - Fork 113
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
Changes from all commits
8d14d34
b49de95
7333057
3eea195
4070a75
1e4e914
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,15 +33,12 @@ | |
import com.google.cloud.spanner.SpannerExceptionFactory; | ||
import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement; | ||
import com.google.cloud.spanner.connection.AbstractStatementParser.StatementType; | ||
import com.google.cloud.spanner.connection.Connection.InternalMetadataQuery; | ||
import com.google.common.annotations.VisibleForTesting; | ||
import com.google.common.base.Preconditions; | ||
import com.google.spanner.admin.database.v1.DatabaseAdminGrpc; | ||
import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; | ||
import com.google.spanner.v1.SpannerGrpc; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.concurrent.Callable; | ||
|
||
|
@@ -121,29 +118,6 @@ public ApiFuture<ResultSet> executeQueryAsync( | |
final ParsedStatement statement, | ||
AnalyzeMode analyzeMode, | ||
QueryOption... options) { | ||
if (options != null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
for (int i = 0; i < options.length; i++) { | ||
if (options[i] instanceof InternalMetadataQuery) { | ||
Preconditions.checkNotNull(statement); | ||
Preconditions.checkArgument(statement.isQuery(), "Statement is not a query"); | ||
Preconditions.checkArgument( | ||
analyzeMode == AnalyzeMode.NONE, "Analyze is not allowed for DDL batch"); | ||
// Queries marked with internal metadata queries are allowed during a DDL batch. | ||
// These can only be generated by library internal methods and may be used to check | ||
// whether a database object such as table or an index exists. | ||
List<QueryOption> temp = new ArrayList<>(); | ||
Collections.addAll(temp, options); | ||
temp.remove(i); | ||
final QueryOption[] internalOptions = temp.toArray(new QueryOption[0]); | ||
Callable<ResultSet> callable = | ||
() -> | ||
DirectExecuteResultSet.ofResultSet( | ||
dbClient.singleUse().executeQuery(statement.getStatement(), internalOptions)); | ||
return executeStatementAsync( | ||
callType, statement, callable, SpannerGrpc.getExecuteStreamingSqlMethod()); | ||
} | ||
} | ||
} | ||
// Queries are by default not allowed on DDL batches. | ||
throw SpannerExceptionFactory.newSpannerException( | ||
ErrorCode.FAILED_PRECONDITION, "Executing queries is not allowed for DDL batches."); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,15 +40,12 @@ | |
import com.google.cloud.spanner.Dialect; | ||
import com.google.cloud.spanner.ErrorCode; | ||
import com.google.cloud.spanner.Mutation; | ||
import com.google.cloud.spanner.ReadContext; | ||
import com.google.cloud.spanner.ResultSet; | ||
import com.google.cloud.spanner.SpannerBatchUpdateException; | ||
import com.google.cloud.spanner.SpannerException; | ||
import com.google.cloud.spanner.SpannerExceptionFactory; | ||
import com.google.cloud.spanner.Statement; | ||
import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement; | ||
import com.google.cloud.spanner.connection.AbstractStatementParser.StatementType; | ||
import com.google.cloud.spanner.connection.Connection.InternalMetadataQuery; | ||
import com.google.cloud.spanner.connection.UnitOfWork.CallType; | ||
import com.google.cloud.spanner.connection.UnitOfWork.UnitOfWorkState; | ||
import com.google.protobuf.Timestamp; | ||
|
@@ -157,25 +154,6 @@ public void testExecuteCreateDatabase() { | |
.parse(Statement.of("CREATE DATABASE foo")))); | ||
} | ||
|
||
@Test | ||
public void testExecuteMetadataQuery() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
Statement statement = Statement.of("SELECT * FROM INFORMATION_SCHEMA.TABLES"); | ||
ParsedStatement parsedStatement = mock(ParsedStatement.class); | ||
when(parsedStatement.isQuery()).thenReturn(true); | ||
when(parsedStatement.getStatement()).thenReturn(statement); | ||
DatabaseClient dbClient = mock(DatabaseClient.class); | ||
ReadContext singleUse = mock(ReadContext.class); | ||
ResultSet resultSet = mock(ResultSet.class); | ||
when(singleUse.executeQuery(statement)).thenReturn(resultSet); | ||
when(dbClient.singleUse()).thenReturn(singleUse); | ||
DdlBatch batch = createSubject(createDefaultMockDdlClient(), dbClient); | ||
assertThat( | ||
get(batch.executeQueryAsync( | ||
CallType.SYNC, parsedStatement, AnalyzeMode.NONE, InternalMetadataQuery.INSTANCE)) | ||
.hashCode(), | ||
is(equalTo(resultSet.hashCode()))); | ||
} | ||
|
||
@Test | ||
public void testExecuteUpdate() { | ||
DdlBatch batch = createSubject(); | ||
|
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.
This was dead code. Only the class in the public
Connection
interface was used.