Skip to content

Commit 9eea142

Browse files
authored
chore: move commit_response to a connection variable (#506)
Move commit_response and commit_timestamp to generic connection variables and remove the regex-based SHOW statement for them.
1 parent a552d5e commit 9eea142

File tree

9 files changed

+169
-61
lines changed

9 files changed

+169
-61
lines changed

client_side_statement.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -48,19 +48,6 @@ import (
4848
type statementExecutor struct {
4949
}
5050

51-
func (s *statementExecutor) ShowCommitTimestamp(_ context.Context, c *conn, _ string, opts *ExecOptions, _ []driver.NamedValue) (driver.Rows, error) {
52-
ts, err := c.CommitTimestamp()
53-
var commitTs *time.Time
54-
if err == nil {
55-
commitTs = &ts
56-
}
57-
it, err := createTimestampIterator("CommitTimestamp", commitTs)
58-
if err != nil {
59-
return nil, err
60-
}
61-
return createRows(it, opts), nil
62-
}
63-
6451
func (s *statementExecutor) ShowRetryAbortsInternally(_ context.Context, c *conn, _ string, opts *ExecOptions, _ []driver.NamedValue) (driver.Rows, error) {
6552
it, err := createBooleanIterator("RetryAbortsInternally", c.RetryAbortsInternally())
6653
if err != nil {

client_side_statement_test.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,12 @@ func TestStatementExecutor_ReadOnlyStaleness(t *testing.T) {
295295
func TestShowCommitTimestamp(t *testing.T) {
296296
t.Parallel()
297297

298-
c := &conn{logger: noopLogger, state: createInitialConnectionState(connectionstate.TypeNonTransactional, map[string]connectionstate.ConnectionPropertyValue{})}
299-
s := &statementExecutor{}
298+
parser, _ := newStatementParser(databasepb.DatabaseDialect_GOOGLE_STANDARD_SQL, 1000)
299+
c := &conn{
300+
logger: noopLogger,
301+
parser: parser,
302+
state: createInitialConnectionState(connectionstate.TypeNonTransactional, map[string]connectionstate.ConnectionPropertyValue{}),
303+
}
300304
ctx := context.Background()
301305

302306
ts := time.Now()
@@ -307,17 +311,17 @@ func TestShowCommitTimestamp(t *testing.T) {
307311
{nil},
308312
} {
309313
if test.wantValue == nil {
310-
c.commitResponse = nil
314+
c.clearCommitResponse()
311315
} else {
312-
c.commitResponse = &spanner.CommitResponse{CommitTs: *test.wantValue}
316+
c.setCommitResponse(&spanner.CommitResponse{CommitTs: *test.wantValue})
313317
}
314318

315-
it, err := s.ShowCommitTimestamp(ctx, c, "", &ExecOptions{}, nil)
319+
it, err := c.QueryContext(ctx, "show variable commit_timestamp", []driver.NamedValue{})
316320
if err != nil {
317321
t.Fatalf("could not get current commit timestamp from connection: %v", err)
318322
}
319323
cols := it.Columns()
320-
wantCols := []string{"CommitTimestamp"}
324+
wantCols := []string{"commit_timestamp"}
321325
if !cmp.Equal(cols, wantCols) {
322326
t.Fatalf("column names mismatch\nGot: %v\nWant: %v", cols, wantCols)
323327
}

client_side_statements_json.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,6 @@ package spannerdriver
1717
var jsonFile = `{
1818
"statements":
1919
[
20-
{
21-
"name": "SHOW VARIABLE COMMIT_TIMESTAMP",
22-
"executorName": "ClientSideStatementNoParamExecutor",
23-
"resultType": "RESULT_SET",
24-
"regex": "(?is)\\A\\s*show\\s+variable\\s+commit_timestamp\\s*\\z",
25-
"method": "statementShowCommitTimestamp",
26-
"exampleStatements": ["show variable commit_timestamp"],
27-
"examplePrerequisiteStatements": ["update foo set bar=1"]
28-
},
2920
{
3021
"name": "SHOW VARIABLE RETRY_ABORTS_INTERNALLY",
3122
"executorName": "ClientSideStatementNoParamExecutor",

conn.go

Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -245,18 +245,17 @@ type SpannerConn interface {
245245
var _ SpannerConn = &conn{}
246246

247247
type conn struct {
248-
parser *statementParser
249-
connector *connector
250-
closed bool
251-
client *spanner.Client
252-
adminClient *adminapi.DatabaseAdminClient
253-
connId string
254-
logger *slog.Logger
255-
tx contextTransaction
256-
prevTx contextTransaction
257-
resetForRetry bool
258-
commitResponse *spanner.CommitResponse
259-
database string
248+
parser *statementParser
249+
connector *connector
250+
closed bool
251+
client *spanner.Client
252+
adminClient *adminapi.DatabaseAdminClient
253+
connId string
254+
logger *slog.Logger
255+
tx contextTransaction
256+
prevTx contextTransaction
257+
resetForRetry bool
258+
database string
260259

261260
execSingleQuery func(ctx context.Context, c *spanner.Client, statement spanner.Statement, bound spanner.TimestampBound, options *ExecOptions) *spanner.RowIterator
262261
execSingleQueryTransactional func(ctx context.Context, c *spanner.Client, statement spanner.Statement, options *ExecOptions) (rowIterator, *spanner.CommitResponse, error)
@@ -286,17 +285,33 @@ func (c *conn) UnderlyingClient() (*spanner.Client, error) {
286285
}
287286

288287
func (c *conn) CommitTimestamp() (time.Time, error) {
289-
if c.commitResponse == nil {
288+
ts := propertyCommitTimestamp.GetValueOrDefault(c.state)
289+
if ts == nil {
290290
return time.Time{}, spanner.ToSpannerError(status.Error(codes.FailedPrecondition, "this connection has not executed a read/write transaction that committed successfully"))
291291
}
292-
return c.commitResponse.CommitTs, nil
292+
return *ts, nil
293293
}
294294

295295
func (c *conn) CommitResponse() (commitResponse *spanner.CommitResponse, err error) {
296-
if c.commitResponse == nil {
296+
resp := propertyCommitResponse.GetValueOrDefault(c.state)
297+
if resp == nil {
297298
return nil, spanner.ToSpannerError(status.Error(codes.FailedPrecondition, "this connection has not executed a read/write transaction that committed successfully"))
298299
}
299-
return c.commitResponse, nil
300+
return resp, nil
301+
}
302+
303+
func (c *conn) clearCommitResponse() {
304+
_ = propertyCommitResponse.SetValue(c.state, nil, connectionstate.ContextUser)
305+
_ = propertyCommitTimestamp.SetValue(c.state, nil, connectionstate.ContextUser)
306+
}
307+
308+
func (c *conn) setCommitResponse(commitResponse *spanner.CommitResponse) {
309+
if commitResponse == nil {
310+
c.clearCommitResponse()
311+
return
312+
}
313+
_ = propertyCommitResponse.SetValue(c.state, commitResponse, connectionstate.ContextUser)
314+
_ = propertyCommitTimestamp.SetValue(c.state, &commitResponse.CommitTs, connectionstate.ContextUser)
300315
}
301316

302317
func (c *conn) showConnectionVariable(identifier identifier) (any, bool, error) {
@@ -715,7 +730,6 @@ func (c *conn) ResetSession(_ context.Context) error {
715730
return driver.ErrBadConn
716731
}
717732
}
718-
c.commitResponse = nil
719733
c.batch = nil
720734

721735
_ = c.state.Reset(connectionstate.ContextUser)
@@ -805,7 +819,7 @@ func (c *conn) QueryContext(ctx context.Context, query string, args []driver.Nam
805819

806820
func (c *conn) queryContext(ctx context.Context, query string, execOptions *ExecOptions, args []driver.NamedValue) (driver.Rows, error) {
807821
// Clear the commit timestamp of this connection before we execute the query.
808-
c.commitResponse = nil
822+
c.clearCommitResponse()
809823
// Check if the execution options contains an instruction to execute
810824
// a specific partition of a PartitionedQuery.
811825
if pq := execOptions.PartitionedQueryOptions.ExecutePartition.PartitionedQuery; pq != nil {
@@ -830,7 +844,7 @@ func (c *conn) queryContext(ctx context.Context, query string, execOptions *Exec
830844
if err != nil {
831845
return nil, err
832846
}
833-
c.commitResponse = commitResponse
847+
c.setCommitResponse(commitResponse)
834848
} else if execOptions.PartitionedQueryOptions.PartitionQuery {
835849
return nil, spanner.ToSpannerError(status.Errorf(codes.FailedPrecondition, "PartitionQuery is only supported in batch read-only transactions"))
836850
} else if execOptions.PartitionedQueryOptions.AutoPartitionQuery {
@@ -877,7 +891,7 @@ func (c *conn) ExecContext(ctx context.Context, query string, args []driver.Name
877891

878892
func (c *conn) execContext(ctx context.Context, query string, execOptions *ExecOptions, args []driver.NamedValue) (driver.Result, error) {
879893
// Clear the commit timestamp of this connection before we execute the statement.
880-
c.commitResponse = nil
894+
c.clearCommitResponse()
881895

882896
statementInfo := c.parser.detectStatementType(query)
883897
// Use admin API if DDL statement is provided.
@@ -917,7 +931,7 @@ func (c *conn) execContext(ctx context.Context, query string, execOptions *ExecO
917931
if dmlMode == Transactional {
918932
res, commitResponse, err = c.execSingleDMLTransactional(ctx, c.client, ss, statementInfo, execOptions)
919933
if err == nil {
920-
c.commitResponse = commitResponse
934+
c.setCommitResponse(commitResponse)
921935
}
922936
} else if dmlMode == PartitionedNonAtomic {
923937
var rowsAffected int64
@@ -1174,7 +1188,7 @@ func (c *conn) BeginTx(ctx context.Context, driverOpts driver.TxOptions) (driver
11741188
c.prevTx = c.tx
11751189
c.tx = nil
11761190
if commitErr == nil {
1177-
c.commitResponse = commitResponse
1191+
c.setCommitResponse(commitResponse)
11781192
if result == txResultCommit {
11791193
_ = c.state.Commit()
11801194
} else {
@@ -1187,7 +1201,7 @@ func (c *conn) BeginTx(ctx context.Context, driverOpts driver.TxOptions) (driver
11871201
// Disable internal retries if any of these options have been set.
11881202
retryAborts: !disableInternalRetries && !disableRetryAborts,
11891203
}
1190-
c.commitResponse = nil
1204+
c.clearCommitResponse()
11911205
return c.tx, nil
11921206
}
11931207

connection_properties.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,33 @@ var propertyDisableStatementCache = createConnectionProperty(
406406
connectionstate.ConvertBool,
407407
)
408408

409+
// Generated read-only properties. These cannot be set by the user anywhere.
410+
var propertyCommitTimestamp = createReadOnlyConnectionProperty(
411+
"commit_timestamp",
412+
"The commit timestamp of the last implicit or explicit read/write transaction that "+
413+
"was executed on the connection, or an error if the connection has not executed a read/write transaction "+
414+
"that committed successfully. The timestamp is in the local timezone.",
415+
(*time.Time)(nil),
416+
false,
417+
nil,
418+
connectionstate.ContextUser,
419+
)
420+
var propertyCommitResponse = createReadOnlyConnectionProperty(
421+
"commit_response",
422+
"The commit response of the last implicit or explicit read/write transaction that "+
423+
"was executed on the connection, or an error if the connection has not executed a read/write transaction "+
424+
"that committed successfully.",
425+
(*spanner.CommitResponse)(nil),
426+
false,
427+
nil,
428+
connectionstate.ContextUser,
429+
)
430+
431+
func createReadOnlyConnectionProperty[T comparable](name, description string, defaultValue T, hasDefaultValue bool, validValues []T, context connectionstate.Context) *connectionstate.TypedConnectionProperty[T] {
432+
converter := connectionstate.CreateReadOnlyConverter[T](name)
433+
return createConnectionProperty(name, description, defaultValue, hasDefaultValue, validValues, context, converter)
434+
}
435+
409436
func createConnectionProperty[T comparable](name, description string, defaultValue T, hasDefaultValue bool, validValues []T, context connectionstate.Context, converter func(value string) (T, error)) *connectionstate.TypedConnectionProperty[T] {
410437
prop := connectionstate.CreateConnectionProperty(name, description, defaultValue, hasDefaultValue, validValues, context, converter)
411438
connectionProperties[prop.Key()] = prop

connectionstate/converters.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ import (
2525
"google.golang.org/grpc/status"
2626
)
2727

28+
func CreateReadOnlyConverter[T any](property string) func(value string) (T, error) {
29+
return func(value string) (T, error) {
30+
var t T
31+
return t, status.Errorf(codes.FailedPrecondition, "property %s is read-only", property)
32+
}
33+
}
34+
2835
func ConvertBool(value string) (bool, error) {
2936
return strconv.ParseBool(value)
3037
}

driver_test.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -487,9 +487,10 @@ func TestConnection_Reset(t *testing.T) {
487487
connector: &connector{
488488
connectorConfig: ConnectorConfig{},
489489
},
490-
state: createInitialConnectionState(connectionstate.TypeTransactional, map[string]connectionstate.ConnectionPropertyValue{}),
491-
batch: &batch{tp: dml},
492-
commitResponse: &spanner.CommitResponse{},
490+
state: createInitialConnectionState(connectionstate.TypeTransactional, map[string]connectionstate.ConnectionPropertyValue{
491+
propertyCommitResponse.Key(): propertyCommitResponse.CreateTypedInitialValue(nil),
492+
}),
493+
batch: &batch{tp: dml},
493494
tx: &readOnlyTransaction{
494495
logger: noopLogger,
495496
close: func(_ txResult) {
@@ -498,6 +499,7 @@ func TestConnection_Reset(t *testing.T) {
498499
},
499500
}
500501

502+
c.setCommitResponse(&spanner.CommitResponse{})
501503
if err := c.SetReadOnlyStaleness(spanner.ExactStaleness(time.Second)); err != nil {
502504
t.Fatal(err)
503505
}
@@ -510,7 +512,7 @@ func TestConnection_Reset(t *testing.T) {
510512
if c.inBatch() {
511513
t.Error("failed to clear batch")
512514
}
513-
if c.commitResponse != nil {
515+
if _, err := c.CommitResponse(); err == nil {
514516
t.Errorf("failed to clear commit response")
515517
}
516518
if !txClosed {

driver_with_mockserver_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3363,6 +3363,77 @@ func TestShowVariableCommitTimestamp(t *testing.T) {
33633363
t.Fatalf("got zero commit timestamp: %v", ts)
33643364
}
33653365
}
3366+
3367+
// Verify that we cannot manually set the commit_timestamp variable.
3368+
_, err = conn.ExecContext(ctx, "set commit_timestamp='2025-09-02T10:00:00Z'")
3369+
if g, w := spanner.ErrCode(err), codes.FailedPrecondition; g != w {
3370+
t.Fatalf("set commit timestamp error code mismatch\n Got: %v\nWant: %v", g, w)
3371+
}
3372+
if g, w := err.Error(), "rpc error: code = FailedPrecondition desc = property commit_timestamp is read-only"; g != w {
3373+
t.Fatalf("set commit timestamp error message mismatch\n Got: %v\nWant: %v", g, w)
3374+
}
3375+
}
3376+
3377+
func TestShowVariableCommitResponse(t *testing.T) {
3378+
t.Parallel()
3379+
3380+
ctx := context.Background()
3381+
db, _, teardown := setupTestDBConnection(t)
3382+
defer teardown()
3383+
3384+
conn, err := db.Conn(ctx)
3385+
defer func() {
3386+
if err := conn.Close(); err != nil {
3387+
t.Fatal(err)
3388+
}
3389+
}()
3390+
if err != nil {
3391+
t.Fatalf("failed to get a connection: %v", err)
3392+
}
3393+
tx, err := conn.BeginTx(ctx, nil)
3394+
if err != nil {
3395+
t.Fatalf("failed to start transaction: %v", err)
3396+
}
3397+
if err := tx.Commit(); err != nil {
3398+
t.Fatalf("commit failed: %v", err)
3399+
}
3400+
// Get the commit response from the connection using a custom SQL statement.
3401+
// We do this in a simple loop to verify that we can get it multiple times.
3402+
for i := 0; i < 2; i++ {
3403+
var resp string
3404+
if err := conn.QueryRowContext(ctx, "SHOW VARIABLE commit_response").Scan(&resp); err != nil {
3405+
t.Fatalf("failed to get commit response: %v", err)
3406+
}
3407+
if resp == "" {
3408+
t.Fatalf("got empty commit response: %v", resp)
3409+
}
3410+
}
3411+
3412+
// Verify that we cannot manually set the commit_response variable.
3413+
_, err = conn.ExecContext(ctx, "set commit_response='2025-09-02T10:00:00Z'")
3414+
if g, w := spanner.ErrCode(err), codes.FailedPrecondition; g != w {
3415+
t.Fatalf("set commit response error code mismatch\n Got: %v\nWant: %v", g, w)
3416+
}
3417+
if g, w := err.Error(), "rpc error: code = FailedPrecondition desc = property commit_response is read-only"; g != w {
3418+
t.Fatalf("set commit response error message mismatch\n Got: %v\nWant: %v", g, w)
3419+
}
3420+
}
3421+
3422+
func TestSetCommitTimestampInConnectionString(t *testing.T) {
3423+
t.Parallel()
3424+
3425+
_, server, teardown := setupTestDBConnection(t)
3426+
defer teardown()
3427+
3428+
_, err := sql.Open(
3429+
"spanner",
3430+
fmt.Sprintf("%s/projects/p/instances/i/databases/d?useplaintext=true;commit_timestamp='2025-09-02T10:00:00Z'", server.Address))
3431+
if g, w := spanner.ErrCode(err), codes.FailedPrecondition; g != w {
3432+
t.Fatalf("error code mismatch\n Got: %v\nWant: %v", g, w)
3433+
}
3434+
if g, w := err.Error(), "rpc error: code = FailedPrecondition desc = property commit_timestamp is read-only"; g != w {
3435+
t.Fatalf("error message mismatch\n Got: %v\nWant: %v", g, w)
3436+
}
33663437
}
33673438

33683439
func TestMinSessions(t *testing.T) {

statements.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package spannerdriver
33
import (
44
"context"
55
"database/sql/driver"
6+
"encoding/json"
67
"fmt"
78
"time"
89

@@ -112,15 +113,19 @@ func (s *parsedShowStatement) queryContext(ctx context.Context, c *conn, params
112113
case *time.Time:
113114
it, err = createTimestampIterator(col, val)
114115
default:
115-
if stringerVal, ok := val.(fmt.Stringer); ok {
116-
if hasValue {
117-
it, err = createStringIterator(col, stringerVal.String())
116+
stringVal := ""
117+
if hasValue {
118+
if stringerVal, ok := val.(fmt.Stringer); ok {
119+
stringVal = stringerVal.String()
118120
} else {
119-
it, err = createStringIterator(col, "")
121+
jsonVal, err := json.Marshal(val)
122+
if err != nil {
123+
return nil, status.Errorf(codes.InvalidArgument, "unsupported type: %T", val)
124+
}
125+
stringVal = string(jsonVal)
120126
}
121-
} else {
122-
err = status.Errorf(codes.InvalidArgument, "unsupported type: %T", val)
123127
}
128+
it, err = createStringIterator(col, stringVal)
124129
}
125130
if err != nil {
126131
return nil, err

0 commit comments

Comments
 (0)