From 8b77b4fe40d3e4c3b82ed2bc9ec2ad1cc6c3af86 Mon Sep 17 00:00:00 2001 From: Akash Anand Date: Tue, 27 May 2025 10:37:46 +0000 Subject: [PATCH 1/2] fix: return explicit error message when using DDL statements with QueryContext --- conn.go | 2 ++ conn_with_mockserver_test.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/conn.go b/conn.go index b9bf1f12..6c71b0f5 100644 --- a/conn.go +++ b/conn.go @@ -773,6 +773,8 @@ func (c *conn) queryContext(ctx context.Context, query string, execOptions ExecO return nil, spanner.ToSpannerError(status.Errorf(codes.FailedPrecondition, "PartitionQuery is only supported in batch read-only transactions")) } else if execOptions.PartitionedQueryOptions.AutoPartitionQuery { return c.executeAutoPartitionedQuery(ctx, query, args) + } else if statementType.statementType == statementTypeDdl { + return nil, spanner.ToSpannerError(status.Errorf(codes.FailedPrecondition, "failed to perform DDL operation. Use ExecContext method to execute DDL statements")) } else { // The statement was either detected as being a query, or potentially not recognized at all. // In that case, just default to using a single-use read-only transaction and let Spanner diff --git a/conn_with_mockserver_test.go b/conn_with_mockserver_test.go index 05ad707c..b946c48c 100644 --- a/conn_with_mockserver_test.go +++ b/conn_with_mockserver_test.go @@ -239,3 +239,20 @@ func TestIsolationLevelAutoCommit(t *testing.T) { } } } + +func TestDDLUsingQueryContext(t *testing.T) { + t.Parallel() + + db, _, teardown := setupTestDBConnection(t) + defer teardown() + ctx := context.Background() + + // DDL statements should not use the query context. + _, err := db.QueryContext(ctx, "CREATE TABLE Foo (Bar STRING(100))") + if err == nil { + t.Fatal("expected error for DDL statement using QueryContext, got nil") + } + if g, w := err.Error(), `spanner: code = "FailedPrecondition", desc = "failed to perform DDL operation. Use ExecContext method to execute DDL statements"`; g != w { + t.Fatalf("error mismatch\n Got: %v\nWant: %v", g, w) + } +} From f1af4a4a47b83598653dd6b44537a9d6555b2b99 Mon Sep 17 00:00:00 2001 From: Akash Anand Date: Tue, 27 May 2025 13:23:14 +0000 Subject: [PATCH 2/2] Added new tests for verifying errors in both RO and RW transactions --- conn.go | 8 +++--- conn_with_mockserver_test.go | 47 +++++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/conn.go b/conn.go index 6c71b0f5..12408082 100644 --- a/conn.go +++ b/conn.go @@ -758,9 +758,13 @@ func (c *conn) queryContext(ctx context.Context, query string, execOptions ExecO if err != nil { return nil, err } + statementType := detectStatementType(query) + // DDL statements are not supported in QueryContext so fail early. + if statementType.statementType == statementTypeDdl { + return nil, spanner.ToSpannerError(status.Errorf(codes.FailedPrecondition, "QueryContext does not support DDL statements, use ExecContext instead")) + } var iter rowIterator if c.tx == nil { - statementType := detectStatementType(query) if statementType.statementType == statementTypeDml { // Use a read/write transaction to execute the statement. var commitTs time.Time @@ -773,8 +777,6 @@ func (c *conn) queryContext(ctx context.Context, query string, execOptions ExecO return nil, spanner.ToSpannerError(status.Errorf(codes.FailedPrecondition, "PartitionQuery is only supported in batch read-only transactions")) } else if execOptions.PartitionedQueryOptions.AutoPartitionQuery { return c.executeAutoPartitionedQuery(ctx, query, args) - } else if statementType.statementType == statementTypeDdl { - return nil, spanner.ToSpannerError(status.Errorf(codes.FailedPrecondition, "failed to perform DDL operation. Use ExecContext method to execute DDL statements")) } else { // The statement was either detected as being a query, or potentially not recognized at all. // In that case, just default to using a single-use read-only transaction and let Spanner diff --git a/conn_with_mockserver_test.go b/conn_with_mockserver_test.go index b946c48c..b64af250 100644 --- a/conn_with_mockserver_test.go +++ b/conn_with_mockserver_test.go @@ -252,7 +252,52 @@ func TestDDLUsingQueryContext(t *testing.T) { if err == nil { t.Fatal("expected error for DDL statement using QueryContext, got nil") } - if g, w := err.Error(), `spanner: code = "FailedPrecondition", desc = "failed to perform DDL operation. Use ExecContext method to execute DDL statements"`; g != w { + if g, w := err.Error(), `spanner: code = "FailedPrecondition", desc = "QueryContext does not support DDL statements, use ExecContext instead"`; g != w { + t.Fatalf("error mismatch\n Got: %v\nWant: %v", g, w) + } +} + +func TestDDLUsingQueryContextInReadOnlyTx(t *testing.T) { + t.Parallel() + + db, _, teardown := setupTestDBConnection(t) + defer teardown() + ctx := context.Background() + tx, err := db.BeginTx(ctx, &sql.TxOptions{ReadOnly: true}) + if err != nil { + t.Fatal(err) + } + defer tx.Rollback() + + // DDL statements should not use the query context in a read-only transaction. + _, err = tx.QueryContext(ctx, "CREATE TABLE Foo (Bar STRING(100))") + if err == nil { + t.Fatal("expected error for DDL statement using QueryContext in read-only transaction, got nil") + } + if g, w := err.Error(), `spanner: code = "FailedPrecondition", desc = "QueryContext does not support DDL statements, use ExecContext instead"`; g != w { + t.Fatalf("error mismatch\n Got: %v\nWant: %v", g, w) + } +} + +func TestDDLUsingQueryContextInReadWriteTransaction(t *testing.T) { + t.Parallel() + + db, _, teardown := setupTestDBConnection(t) + defer teardown() + ctx := context.Background() + + tx, err := db.BeginTx(ctx, &sql.TxOptions{}) + if err != nil { + t.Fatal(err) + } + defer tx.Rollback() + + // DDL statements should not use the query context in a read-write transaction. + _, err = tx.QueryContext(ctx, "CREATE TABLE Foo (Bar STRING(100))") + if err == nil { + t.Fatal("expected error for DDL statement using QueryContext in read-write transaction, got nil") + } + if g, w := err.Error(), `spanner: code = "FailedPrecondition", desc = "QueryContext does not support DDL statements, use ExecContext instead"`; g != w { t.Fatalf("error mismatch\n Got: %v\nWant: %v", g, w) } }