From fe7b392b6bf047d757312bbeeb5e4e3e48d1c2e3 Mon Sep 17 00:00:00 2001 From: Gabor Gevay Date: Tue, 14 Jul 2026 05:29:29 +0200 Subject: [PATCH] adapter: don't store txn read holds for timestamp-less determinations Fixes SQL-520, an instance of the "insufficient read holds" family (DB-156): the frontend peek sequencing stored the read holds it acquired for a timestamp-independent query (e.g. a query over a constant-folded view) as the transaction's read holds. `txn_read_holds` play a double role for multi-statement read transactions: they hold back the sinces of the transaction's timedomain, and their id set is the allowed timedomain that later statements are validated against. Both only work if the holds are established by the same statement that determines the transaction timestamp, over the same id bundle. A timestamp-independent query gets a `NoTimestamp` determination, which doesn't pin the transaction timestamp, so a later query determines the transaction timestamp over its own timedomain. The stored holds of the timestamp-independent query then wrongly widened the allowed timedomain: a subsequent statement could pass the timedomain validation and read a collection whose since is not covered by the transaction timestamp. Concretely, in the Parallel Workload (rename + naughty identifiers) failure of SQL-520: 1. The session created a temp table (whose since starts at its exact creation timestamp) and a temp view whose optimized expression is constant (a join on a constant-false condition). 2. The first query of a read-only transaction selected from the constant temp view: TimestampIndependent, `NoTimestamp` determination, but its timedomain (the whole mz_temp schema, including the fresh temp table) was stored as the transaction's read holds. 3. The second query determined the transaction timestamp over its own, disjoint timedomain, choosing a timestamp behind the temp table's since (its inputs included a source whose upper lagged behind the temp table's creation time). 4. The third query selected from the temp table: the poisoned hold set made the timedomain validation pass instead of returning `RelationOutsideTimeDomain`, and the peek's as_of was behind the temp table's since, tripping the read hold assert (and, without soft asserts, `SinceViolation`). The fix mirrors the old peek sequencing (`sequence_peek_timestamp`), which only stores read holds into `txn_read_holds` when the determination has a timestamp. This also fixes a spurious `RelationOutsideTimeDomain` error when a statement sequenced by the old path (e.g. EXECUTE) followed a timestamp-independent statement sequenced by the frontend path in the same transaction, because the old path validates against the stored hold set whenever one exists. Co-Authored-By: Claude Fable 5 --- src/adapter/src/frontend_peek.rs | 14 +++++--- test/sqllogictest/timedomain.slt | 60 ++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/adapter/src/frontend_peek.rs b/src/adapter/src/frontend_peek.rs index 65a552871a2ed..016365b994cc6 100644 --- a/src/adapter/src/frontend_peek.rs +++ b/src/adapter/src/frontend_peek.rs @@ -732,10 +732,16 @@ impl PeekClient { ) .await?; - // If this is the first (non-AS OF) query in a multi-statement transaction, store - // the read holds in the coordinator, so subsequent queries can validate against - // them. - if in_immediate_multi_stmt_txn { + // If this query pins the timestamp of a multi-statement transaction, store + // the read holds in the coordinator, so subsequent queries can validate + // against them. The stored holds define the transaction's timedomain, so + // only the statement that determines the transaction timestamp may establish + // them, over the same id bundle. A timestamp-less determination (e.g. a + // constant query) doesn't pin the transaction timestamp, so its holds must + // not be stored. + if in_immediate_multi_stmt_txn + && determination.timestamp_context.contains_timestamp() + { self.call_coordinator(|tx| Command::StoreTransactionReadHolds { conn_id: session.conn_id().clone(), read_holds: read_holds.clone(), diff --git a/test/sqllogictest/timedomain.slt b/test/sqllogictest/timedomain.slt index cb18868631148..a99286d6bfc87 100644 --- a/test/sqllogictest/timedomain.slt +++ b/test/sqllogictest/timedomain.slt @@ -257,3 +257,63 @@ EOF statement ok COMMIT + +# Regression test for SQL-520: a timestamp-independent query must not extend +# the transaction's timedomain. The frontend peek sequencing used to store the +# read holds it acquired for a timestamp-independent query as the +# transaction's read holds. A later statement then established the +# transaction's timestamp determination over a smaller id bundle, but the +# timedomain validation of subsequent statements ran against the stored +# (larger) read hold set. Such statements could thus read collections whose +# since was not covered by the transaction timestamp, leading to +# SinceViolation panics. + +statement ok +CREATE SCHEMA other + +statement ok +CREATE VIEW other.const_view AS SELECT 1 AS x + +statement ok +CREATE TABLE other.t2 (i INT) + +simple +BEGIN; +SELECT * FROM other.const_view; +SELECT * FROM t; +---- +COMPLETE 0 +1 +COMPLETE 1 +COMPLETE 0 + +# `other.t2` is outside the timedomain established by `SELECT * FROM t`. The +# constant view query must not have widened the timedomain to the `other` +# schema. +query error Transactions can only reference objects in the same timedomain. +SELECT * FROM other.t2 + +statement ok +ROLLBACK + +# The same bug also affected statements sequenced by the old peek path +# (EXECUTE always is): the stored read holds of the timestamp-independent +# query made the old path validate the timedomain against them and spuriously +# fail. +simple +BEGIN; +SELECT * FROM other.const_view; +PREPARE p AS SELECT * FROM t; +EXECUTE p; +---- +COMPLETE 0 +1 +COMPLETE 1 +COMPLETE 0 +COMPLETE 0 + +statement ok +ROLLBACK + +statement ok +DROP SCHEMA other CASCADE