Skip to content

Commit

Permalink
sql: permit non-synthetic future time AOST queries
Browse files Browse the repository at this point in the history
Informs #101938.

This commit removes the restriction that future-time AOST queries must
use "synthetic" timestamps. This restriction was introduced back when it
was unsafe for requests to operate at future timestamps unless they were
marked as synthetic. This is no longer the case. It is important that we
allow this, because future-time queries can issue subqueries with AOST
timestamps (e.g. for SQL leasing with the "count-leases" internal
executor use).

Requests that are too far in the future will still be rejected during
range lease checks. For details, see `Replica.checkRequestTimeRLocked`.

Release note: None
  • Loading branch information
nvanbenschoten committed Dec 27, 2023
1 parent 4f31fba commit f719e85
Show file tree
Hide file tree
Showing 8 changed files with 11 additions and 26 deletions.
2 changes: 1 addition & 1 deletion pkg/ccl/changefeedccl/changefeed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5021,7 +5021,7 @@ func TestChangefeedErrors(t *testing.T) {
)

sqlDB.ExpectErrWithTimeout(
t, `cannot specify timestamp in the future`,
t, `request timestamp .* too far in future`,
`EXPERIMENTAL CHANGEFEED FOR foo WITH cursor=$1`, timeutil.Now().Add(time.Hour),
)

Expand Down
11 changes: 3 additions & 8 deletions pkg/sql/as_of_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,13 @@ func TestAsOfTime(t *testing.T) {
t.Fatal(err)
}

// Future queries shouldn't work if not marked as synthetic.
if err := db.QueryRow("SELECT a FROM d.t AS OF SYSTEM TIME '2200-01-01'").Scan(&i); !testutils.IsError(err, "pq: AS OF SYSTEM TIME: cannot specify timestamp in the future") {
t.Fatal(err)
}

// Future queries shouldn't work if too far in the future.
if err := db.QueryRow("SELECT a FROM d.t AS OF SYSTEM TIME '+10h?'").Scan(&i); !testutils.IsError(err, "pq: request timestamp .* too far in future") {
if err := db.QueryRow("SELECT a FROM d.t AS OF SYSTEM TIME '+10h'").Scan(&i); !testutils.IsError(err, "pq: request timestamp .* too far in future") {
t.Fatal(err)
}

// Future queries work if marked as synthetic and only slightly in future.
if err := db.QueryRow("SELECT a FROM d.t AS OF SYSTEM TIME '+10ms?'").Scan(&i); err != nil {
// Future queries work if only slightly in the future.
if err := db.QueryRow("SELECT a FROM d.t AS OF SYSTEM TIME '+10ms'").Scan(&i); err != nil {
t.Fatal(err)
}

Expand Down
5 changes: 0 additions & 5 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -2057,11 +2057,6 @@ func (p *planner) EvalAsOfTimestamp(
if err != nil {
return eval.AsOfSystemTime{}, err
}
ts := asOf.Timestamp
if now := p.execCfg.Clock.Now(); now.Less(ts) && !ts.Synthetic {
return eval.AsOfSystemTime{}, errors.Errorf(
"AS OF SYSTEM TIME: cannot specify timestamp in the future (%s > %s)", ts, now)
}
return asOf, nil
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/as_of
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,13 @@ SELECT * FROM t AS OF SYSTEM TIME follower_read_timestamp('boom')
statement error pq: AS OF SYSTEM TIME: only constant expressions, with_min_timestamp, with_max_staleness, or follower_read_timestamp are allowed
SELECT * FROM t AS OF SYSTEM TIME now()

statement error pq: AS OF SYSTEM TIME: interval value '10s' too large, AS OF interval must be <= -1µs
statement error pq: request timestamp .* too far in future
SELECT * FROM t AS OF SYSTEM TIME '10s'

statement error pq: AS OF SYSTEM TIME: interval value '00:00:00.000001' too large, AS OF interval must be <= -1µs
query I
SELECT * FROM t AS OF SYSTEM TIME interval '1 microsecond'
----
2

# Verify that the TxnTimestamp used to generate now() and current_timestamp() is
# set to the historical timestamp.
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/pgwire/testdata/pgtest/bind_and_resolve
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ ReadyForQuery

# This is crdb_only because Postgres does not support AS OF SYSTEM TIME.
send crdb_only
Query {"String": "BEGIN AS OF SYSTEM TIME '1s'"}
Query {"String": "BEGIN AS OF SYSTEM TIME '0s'"}
Sync
----

Expand Down
8 changes: 1 addition & 7 deletions pkg/sql/sem/asof/as_of.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,7 @@ func DatumToHLC(
if iv, err := tree.ParseDInterval(evalCtx.GetIntervalStyle(), s); err == nil {
if (iv.Duration == duration.Duration{}) {
convErr = errors.Errorf("interval value %v too small, absolute value must be >= %v", d, time.Microsecond)
} else if (usage == AsOf && iv.Duration.Compare(duration.Duration{}) > 0 && !syn) {
convErr = errors.Errorf("interval value %v too large, AS OF interval must be <= -%v", d, time.Microsecond)
} else if (usage == Split && iv.Duration.Compare(duration.Duration{}) < 0) {
// Do we need to consider if the timestamp is synthetic (see
// hlc.Timestamp.Synthetic), as for AS OF stmt?
convErr = errors.Errorf("interval value %v too small, SPLIT AT interval must be >= %v", d, time.Microsecond)
}
ts.WallTime = duration.Add(stmtTimestamp, iv.Duration).UnixNano()
Expand All @@ -287,9 +283,7 @@ func DatumToHLC(
case *tree.DDecimal:
ts, convErr = hlc.DecimalToHLC(&d.Decimal)
case *tree.DInterval:
if (usage == AsOf && d.Duration.Compare(duration.Duration{}) > 0) {
convErr = errors.Errorf("interval value %v too large, AS OF interval must be <= -%v", d, time.Microsecond)
} else if (usage == Split && d.Duration.Compare(duration.Duration{}) < 0) {
if (usage == Split && d.Duration.Compare(duration.Duration{}) < 0) {
convErr = errors.Errorf("interval value %v too small, SPLIT interval must be >= %v", d, time.Microsecond)
}
ts.WallTime = duration.Add(stmtTimestamp, d.Duration).UnixNano()
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/tests/rsg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,6 @@ var ignoredErrorPatterns = []string{
"index .* in the middle of being added",
"could not mark job .* as succeeded",
"failed to read backup descriptor",
"AS OF SYSTEM TIME: cannot specify timestamp in the future",
"AS OF SYSTEM TIME: timestamp before 1970-01-01T00:00:00Z is invalid",
"BACKUP for requested time needs option 'revision_history'",
"RESTORE timestamp: supplied backups do not cover requested time",
Expand Down
2 changes: 1 addition & 1 deletion pkg/testutils/sqlutils/sql_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func (sr *SQLRunner) ExpectErrSucceedsSoon(
})
}

// ExpectErrWithTimeout wraps ExpectErr with a timeout..
// ExpectErrWithTimeout wraps ExpectErr with a timeout.
func (sr *SQLRunner) ExpectErrWithTimeout(
t Fataler, errRE string, query string, args ...interface{},
) {
Expand Down

0 comments on commit f719e85

Please sign in to comment.