Skip to content

Commit

Permalink
tree: make SET TRANSACTION a walkable statement
Browse files Browse the repository at this point in the history
SET TRANSACTION is now a walkable statement. This fixes a bug where
placeholders in a SET TRANSACTION statement were not resolved correctly.

The statement must be walkable for the placeholderAnnotationVisitor to
work correctly -- it is an AST visitor that relies on walkStmt to
discover all placeholders in a statement.

Release note (bug fix): Placeholder arguments can now be used in SET
TRANSACTION statements.
  • Loading branch information
rafiss committed Nov 14, 2023
1 parent cedd409 commit 1ea4f20
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 0 deletions.
39 changes: 39 additions & 0 deletions pkg/sql/pgwire/testdata/pgtest/set_transaction
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# This test relies on a CoockroachDB-specific feature, so everything
# is marked as crdb_only.

only crdb
----

send
Query {"String": "BEGIN"}
----

until
ReadyForQuery
----
{"Type":"CommandComplete","CommandTag":"BEGIN"}
{"Type":"ReadyForQuery","TxStatus":"T"}

send
Parse {"Query": "SET TRANSACTION AS OF SYSTEM TIME $1::timestamp"}
Bind {"Parameters": [{"text":"2019-01-01 00:00:00"}]}
Execute
Sync
Query {"String": "SELECT now()"}
Query {"String": "COMMIT"}
----

until ignore=RowDescription
ReadyForQuery
ReadyForQuery
ReadyForQuery
----
{"Type":"ParseComplete"}
{"Type":"BindComplete"}
{"Type":"CommandComplete","CommandTag":"SET TRANSACTION"}
{"Type":"ReadyForQuery","TxStatus":"T"}
{"Type":"DataRow","Values":[{"text":"2019-01-01 00:00:00+00"}]}
{"Type":"CommandComplete","CommandTag":"SELECT 1"}
{"Type":"ReadyForQuery","TxStatus":"T"}
{"Type":"CommandComplete","CommandTag":"COMMIT"}
{"Type":"ReadyForQuery","TxStatus":"I"}
19 changes: 19 additions & 0 deletions pkg/sql/sem/tree/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,25 @@ func (node *SetTransaction) Format(ctx *FmtCtx) {
ctx.FormatNode(&node.Modes)
}

// copyNode makes a copy of this Statement.
func (stmt *SetTransaction) copyNode() *SetTransaction {
stmtCopy := *stmt
return &stmtCopy
}

// walkStmt is part of the walkableStmt interface.
func (stmt *SetTransaction) walkStmt(v Visitor) Statement {
ret := stmt
if stmt.Modes.AsOf.Expr != nil {
e, changed := WalkExpr(v, stmt.Modes.AsOf.Expr)
if changed {
ret = stmt.copyNode()
ret.Modes.AsOf.Expr = e
}
}
return ret
}

// SetSessionAuthorizationDefault represents a SET SESSION AUTHORIZATION DEFAULT
// statement. This can be extended (and renamed) if we ever support names in the
// last position.
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/sem/tree/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -2039,6 +2039,7 @@ var _ walkableStmt = &Restore{}
var _ walkableStmt = &SelectClause{}
var _ walkableStmt = &Select{}
var _ walkableStmt = &SetClusterSetting{}
var _ walkableStmt = &SetTransaction{}
var _ walkableStmt = &SetVar{}
var _ walkableStmt = &ShowFingerprints{}
var _ walkableStmt = &ShowTenantClusterSetting{}
Expand Down

0 comments on commit 1ea4f20

Please sign in to comment.