Skip to content

Commit

Permalink
sql: support AOST queries with synthetic timestamps
Browse files Browse the repository at this point in the history
This commit updates the handling of AS OF SYSTEM TIME clauses to accept
future-time synthetic timestamps. This functionality is needed when performing
certain kinds of schema changes on GLOBAL tables, as the operating transaction
will get its commit timestamp bumped into the future and will then try to call
into `CountLeases` from `CheckTwoVersionInvariant` with the commit timestamp.

We're already protected from users shooting themselves in the foot too badly
with this, as we only allow timestamps to lead present time by a few seconds
before they are rejected by KV.

Release justification: Needed for new functionality
  • Loading branch information
nvanbenschoten committed Mar 4, 2021
1 parent 37cdf01 commit 34dc5ae
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 4 deletions.
12 changes: 11 additions & 1 deletion pkg/sql/as_of_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,21 @@ func TestAsOfTime(t *testing.T) {
}
})

// Future queries shouldn't work.
// 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") {
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 {
t.Fatal(err)
}

// Verify queries with positive scale work properly.
if _, err := db.Query("SELECT a FROM d.t AS OF SYSTEM TIME 1e1"); !testutils.IsError(err, `pq: relation "d.t" does not exist`) {
t.Fatal(err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/catalog/lease/lease.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ func CountLeases(
)
}

stmt := fmt.Sprintf(`SELECT count(1) FROM system.public.lease AS OF SYSTEM TIME %s WHERE `,
stmt := fmt.Sprintf(`SELECT count(1) FROM system.public.lease AS OF SYSTEM TIME '%s' WHERE `,
at.AsOfSystemTime()) +
strings.Join(whereClauses, " OR ")
values, err := executor.QueryRowEx(
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -1222,7 +1222,7 @@ func (p *planner) EvalAsOfTimestamp(
if err != nil {
return hlc.Timestamp{}, err
}
if now := p.execCfg.Clock.Now(); now.Less(ts) {
if now := p.execCfg.Clock.Now(); now.Less(ts) && !ts.Synthetic {
return hlc.Timestamp{}, errors.Errorf(
"AS OF SYSTEM TIME: cannot specify timestamp in the future (%s > %s)", ts, now)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/sem/tree/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//pkg/base",
"//pkg/clusterversion",
"//pkg/geo",
"//pkg/geo/geopb",
"//pkg/keys",
Expand Down
12 changes: 12 additions & 0 deletions pkg/sql/sem/tree/as_of.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"time"

"github.com/cockroachdb/apd/v2"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
Expand Down Expand Up @@ -106,14 +107,24 @@ func DatumToHLC(evalCtx *EvalContext, stmtTimestamp time.Time, d Datum) (hlc.Tim
switch d := d.(type) {
case *DString:
s := string(*d)
// Parse synthetic flag.
syn := false
if strings.HasSuffix(s, "?") && evalCtx.Settings.Version.IsActive(evalCtx.Context, clusterversion.PriorReadSummaries) {
// NOTE: we don't parse this in mixed-version clusters because v20.2
// nodes will not know how to handle synthetic timestamps.
s = s[:len(s)-1]
syn = true
}
// Attempt to parse as timestamp.
if dt, _, err := ParseDTimestamp(evalCtx, s, time.Nanosecond); err == nil {
ts.WallTime = dt.Time.UnixNano()
ts.Synthetic = syn
break
}
// Attempt to parse as a decimal.
if dec, _, err := apd.NewFromString(s); err == nil {
ts, convErr = DecimalToHLC(dec)
ts.Synthetic = syn
break
}
// Attempt to parse as an interval.
Expand All @@ -122,6 +133,7 @@ func DatumToHLC(evalCtx *EvalContext, stmtTimestamp time.Time, d Datum) (hlc.Tim
convErr = errors.Errorf("interval value %v too small, absolute value must be >= %v", d, time.Microsecond)
}
ts.WallTime = duration.Add(stmtTimestamp, iv.Duration).UnixNano()
ts.Synthetic = syn
break
}
convErr = errors.Errorf("value is neither timestamp, decimal, nor interval")
Expand Down
6 changes: 5 additions & 1 deletion pkg/util/hlc/timestamp.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,11 @@ func ParseTimestamp(str string) (_ Timestamp, err error) {

// AsOfSystemTime returns a string to be used in an AS OF SYSTEM TIME query.
func (t Timestamp) AsOfSystemTime() string {
return fmt.Sprintf("%d.%010d", t.WallTime, t.Logical)
syn := ""
if t.Synthetic {
syn = "?"
}
return fmt.Sprintf("%d.%010d%s", t.WallTime, t.Logical, syn)
}

// IsEmpty retruns true if t is an empty Timestamp.
Expand Down
3 changes: 3 additions & 0 deletions pkg/util/hlc/timestamp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,9 @@ func TestAsOfSystemTime(t *testing.T) {
{makeTS(145, 0), "145.0000000000"},
{makeTS(145, 123), "145.0000000123"},
{makeTS(145, 1123456789), "145.1123456789"},
{makeSynTS(145, 0), "145.0000000000?"},
{makeSynTS(145, 123), "145.0000000123?"},
{makeSynTS(145, 1123456789), "145.1123456789?"},
}
for _, c := range testCases {
assert.Equal(t, c.exp, c.ts.AsOfSystemTime())
Expand Down

0 comments on commit 34dc5ae

Please sign in to comment.