Skip to content

Commit

Permalink
sql/sem: add check for interval for asof.DatumToHLC()
Browse files Browse the repository at this point in the history
fixes #91021

Currently, if the given interval for `AS OF SYSTEM TIME interval` is a small
postive duration, the query will incorrectly pass. It's because when we call
`clock.Now()`, it has passed the threashold ('statement timestamp + duration').

Now we add a check for the duration value. It has to be negative, with the
absolute value >= a nanosecond.

Similarly, when the logic is used under the SPLIT stmt, the interval's value has
to be positive (for the experation duration), with an absolute value >= a
nanosecond.

link epic CRDB-17785

Release note (bug fix): add restriction for duration value for AS OF SYSTEM TIME
and SPLIT statement.
  • Loading branch information
ZhouXing19 committed Dec 7, 2022
1 parent 916ca31 commit cd07b13
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 8 deletions.
14 changes: 10 additions & 4 deletions pkg/sql/logictest/testdata/logic_test/as_of
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,15 @@ 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 cannot specify timestamp in the future
statement error pq: AS OF SYSTEM TIME: interval value should be negative with absolute value >= 1ns
SELECT * FROM t AS OF SYSTEM TIME '10s'

statement error pq: AS OF SYSTEM TIME: interval value should be negative with absolute value >= 1ns
SELECT * FROM t AS OF SYSTEM TIME interval '20 microsecond'

statement error pq: AS OF SYSTEM TIME: interval value should be negative with absolute value >= 1ns
SELECT * FROM t AS OF SYSTEM TIME interval '1 microsecond'

# Verify that the TxnTimestamp used to generate now() and current_timestamp() is
# set to the historical timestamp.

Expand All @@ -70,13 +76,13 @@ SELECT * FROM (SELECT now()) AS OF SYSTEM TIME '2018-01-01 00:00:00-1:00'

# Verify that zero intervals indistinguishable from zero cause an error.

statement error pq: AS OF SYSTEM TIME: interval value '0.1us' too small, absolute value must be >= 1µs
statement error pq: AS OF SYSTEM TIME: interval value should be negative with absolute value >= 1ns: unsupported interval duration: '0.1us'
SELECT * FROM t AS OF SYSTEM TIME '0.1us'

statement error pq: AS OF SYSTEM TIME: interval value '0-0' too small, absolute value must be >= 1µs
statement error pq: AS OF SYSTEM TIME: interval value should be negative with absolute value >= 1ns: unsupported interval duration: '0-0'
SELECT * FROM t AS OF SYSTEM TIME '0-0'

statement error pq: AS OF SYSTEM TIME: interval value '-0.1us' too small, absolute value must be >= 1µs
statement error pq: AS OF SYSTEM TIME: interval value should be negative with absolute value >= 1ns: unsupported interval duration: '-0.1us'
SELECT * FROM t AS OF SYSTEM TIME '-0.1us'

statement error pq: AS OF SYSTEM TIME: zero timestamp is invalid
Expand Down
41 changes: 39 additions & 2 deletions pkg/sql/sem/asof/as_of.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,16 +199,30 @@ func Eval(
}

stmtTimestamp := evalCtx.GetStmtTimestamp()
ret.Timestamp, err = DatumToHLC(evalCtx, stmtTimestamp, d)
ret.Timestamp, err = DatumToHLC(evalCtx, stmtTimestamp, d, AsOf)
if err != nil {
return eval.AsOfSystemTime{}, errors.Wrap(err, "AS OF SYSTEM TIME")
}
return ret, nil
}

// DatumToHLCUsage specifies which statement DatumToHLC() is used for.
type DatumToHLCUsage int64

const (
// AsOf is when the DatumToHLC() is used for an AS OF SYSTEM TIME statement.
// In this case, if the interval is not synthetic, its value has to be negative
// and last longer than a nanosecond.
AsOf DatumToHLCUsage = iota
// Split is when the DatumToHLC() is used for a SPLIT statement.
// In this case, if the interval is not synthetic, its value has to be positive
// and last longer than a nanosecond.
Split
)

// DatumToHLC performs the conversion from a Datum to an HLC timestamp.
func DatumToHLC(
evalCtx *eval.Context, stmtTimestamp time.Time, d tree.Datum,
evalCtx *eval.Context, stmtTimestamp time.Time, d tree.Datum, usage DatumToHLCUsage,
) (hlc.Timestamp, error) {
ts := hlc.Timestamp{}
var convErr error
Expand Down Expand Up @@ -237,6 +251,17 @@ 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.Wrapf(
errors.Newf("unsupported interval duration: %v", d),
"interval value should be negative with absolute value >= %v",
time.Nanosecond,
)
} else if (usage == Split && iv.Duration.Compare(duration.Duration{}) < 0 && !syn) {
convErr = errors.Wrapf(
errors.Newf("unsupported interval duration: %v", d),
"SPLIT AT: expiration interval must be non-negative",
)
}
ts.WallTime = duration.Add(stmtTimestamp, iv.Duration).UnixNano()
ts.Synthetic = syn
Expand All @@ -252,6 +277,18 @@ 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.Wrapf(
errors.Newf("unsupported interval duration: %v", d),
"interval value should be negative with absolute value >= %v",
time.Nanosecond,
)
} else if (usage == Split && d.Duration.Compare(duration.Duration{}) < 0) {
convErr = errors.Wrapf(
errors.Newf("unsupported interval duration: %v", d),
"expiration interval must be non-negative",
)
}
ts.WallTime = duration.Add(stmtTimestamp, d.Duration).UnixNano()
default:
convErr = errors.WithSafeDetails(
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/split.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func parseExpirationTime(
return hlc.MaxTimestamp, nil
}
stmtTimestamp := evalCtx.GetStmtTimestamp()
ts, err := asof.DatumToHLC(evalCtx, stmtTimestamp, d)
ts, err := asof.DatumToHLC(evalCtx, stmtTimestamp, d, asof.Split)
if err != nil {
return ts, errors.Wrap(err, "SPLIT AT")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/split_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func TestSplitAt(t *testing.T) {
},
{
in: "ALTER TABLE d.i SPLIT AT VALUES (17) WITH EXPIRATION '-1 day'::interval",
error: "SPLIT AT: expiration time should be greater than or equal to current time",
error: "SPLIT AT: expiration interval must be non-negative",
},
{
in: "ALTER TABLE d.i SPLIT AT VALUES (17) WITH EXPIRATION '0.1us'",
Expand Down

0 comments on commit cd07b13

Please sign in to comment.