Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
62 changes: 62 additions & 0 deletions conn_with_mockserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,65 @@ 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))")
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should make sure that we get consistent behavior both when in a transaction and when outside a transaction. So can we add:

  1. Tests for executing a DDL statement both in a read-only and a read/write transaction using QueryContext, and ensure that these also return an error early (that is: Without having to call rows.Next() first)
  2. Maybe: Additional handling in the QueryContext function to ensure that we get the behavior that we want in point 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I have added few tests to ensure that the behaviour is correct for both the cases ie. with transaction and without transaction.

if err == nil {
t.Fatal("expected error for DDL statement using QueryContext, 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 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)
}
}
Loading