Skip to content

Commit

Permalink
sql: error in cluster_logical_timestamp when not SERIALIZABLE
Browse files Browse the repository at this point in the history
Previously, running `cluster_logical_timestamp` within an isolation
level that tolerated write skew would result in an unhandled panic.

This commit adds a gate at the SQL level that will instead return a
FeatureNotSupported pgerror. This behavior may be permanent or may be
removed once we determine how CommitTimestamp should function at these
isolation levels.

Epic:  CRDB-26546
Part of: #103245
Release note (sql change): cluster_logical_timestamp now returns an
error when called at isolation levels lower than SERIALIZABLE.
  • Loading branch information
chrisseto committed Jul 18, 2023
1 parent 6f0fe89 commit 31c798a
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 1 deletion.
11 changes: 11 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/txn
Expand Up @@ -1523,3 +1523,14 @@ regular
statement error pq: SET CLUSTER SETTING cannot be used inside a multi-statement transaction
SET CLUSTER SETTING sql.defaults.use_declarative_schema_changer = 'on';
SET CLUSTER SETTING sql.defaults.use_declarative_schema_changer = 'off';

statement ok
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
SELECT cluster_logical_timestamp();

statement ok
ROLLBACK

statement error pq: cluster_logical_timestamp\(\): unsupported in ReadCommitted isolation
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED;
SELECT cluster_logical_timestamp();
4 changes: 3 additions & 1 deletion pkg/sql/sem/builtins/builtins.go
Expand Up @@ -2779,7 +2779,9 @@ nearest replica.`, builtinconstants.DefaultFollowerReadDuration),
a CockroachDB HLC in decimal form.
Note that uses of this function disable server-side optimizations and
may increase either contention or retry errors, or both.`,
may increase either contention or retry errors, or both.
Returns an error if run in a transaction with an isolation level weaker than SERIALIZABLE.`
Volatility: volatility.Volatile,
},
),
Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/sem/eval/context.go
Expand Up @@ -535,6 +535,14 @@ func (ec *Context) GetClusterTimestamp() (*tree.DDecimal, error) {
if ec.Txn == nil {
return nil, ErrNilTxnInClusterContext
}

// CommitTimestamp panics for isolation levels that can operate across
// multiple timestamps. Prevent this with a gate at the SQL level and return
// a pgerror until we decide how this will officially behave. See #103245.
if ec.TxnIsoLevel.ToleratesWriteSkew() {
return nil, pgerror.Newf(pgcode.FeatureNotSupported, "unsupported in %s isolation", ec.TxnIsoLevel.String())
}

ts := ec.Txn.CommitTimestamp()
if ts.IsEmpty() {
return nil, errors.AssertionFailedf("zero cluster timestamp in txn")
Expand Down

0 comments on commit 31c798a

Please sign in to comment.