sql: add support for CREATE TABLE AS ... AS OF SYSTEM TIME#142147
sql: add support for CREATE TABLE AS ... AS OF SYSTEM TIME#142147craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
fqazi
left a comment
There was a problem hiding this comment.
Great work! Just a few minor questions.
Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @rytaft)
pkg/sql/conn_executor_exec.go line 2788 at r1 (raw file):
return nil } originalTxn := planner.txn
As a future thing we may want a protected timestamp
pkg/sql/conn_executor_exec.go line 2798 at r1 (raw file):
planner.extendedEvalCtx.Descs = historicalCollection restoreOriginalPlanner = func() { historicalCollection.ReleaseAll(ctx)
Does the historicalTxn itself need a clean up? Or will the planner commit / rollback the txn on its own.
spilchen
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/conn_executor_exec.go line 2788 at r1 (raw file):
Previously, fqazi (Faizan Qazi) wrote…
As a future thing we may want a protected timestamp
Would the protected timestamp prevent GC from removing deleted rows during backfill? If so, it seems kind of important.
pkg/sql/opt/optbuilder/select.go line 1489 at r1 (raw file):
switch b.stmt.(type) { case *tree.CreateTable: asOf.ForBackfill = true
Why do we need to set this here? It appears that the asOf struct is generated, validated, and then discarded. Or is this for the *b.evalCtx.AsOfSystemTime != asOf check below?
rafiss
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/conn_executor_exec.go line 2788 at r1 (raw file):
Previously, spilchen wrote…
Would the protected timestamp prevent GC from removing deleted rows during backfill? If so, it seems kind of important.
we already add a protected timestamp during the backfill here:
cockroach/pkg/sql/schema_changer.go
Lines 404 to 405 in d8818a3
@fqazi were you suggesting that we should add one during planning also? i don't think we should necessarily. planning should be very quick, and it is expected to fail if the chosen timestamp is from before the GC threshold.
pkg/sql/conn_executor_exec.go line 2798 at r1 (raw file):
Previously, fqazi (Faizan Qazi) wrote…
Does the historicalTxn itself need a clean up? Or will the planner commit / rollback the txn on its own.
good point; i added a commit
pkg/sql/opt/optbuilder/select.go line 1489 at r1 (raw file):
Previously, spilchen wrote…
Why do we need to set this here? It appears that the
asOfstruct is generated, validated, and then discarded. Or is this for the*b.evalCtx.AsOfSystemTime != asOfcheck below?
yeah, this is for the *b.evalCtx.AsOfSystemTime != asOf check below. i can mention this in the comment.
fqazi
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/conn_executor_exec.go line 2788 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
we already add a protected timestamp during the backfill here:
cockroach/pkg/sql/schema_changer.go
Lines 404 to 405 in d8818a3
@fqazi were you suggesting that we should add one during planning also? i don't think we should necessarily. planning should be very quick, and it is expected to fail if the chosen timestamp is from before the GC threshold.
Done.
Ah, so this is already covered. We shouldn't need one for planning
Previously, running a statement of the form
CREATE TABLE t AS SELECT ... FROM ... AS OF SYSTEM TIME x
was not supported.
Now, it is supported. The semantics are that the table creation happens
at the transaction timestamp, but the backfill that's performed to fetch
the data from the `SELECT` is performed at the user-specified timestamp
x.
This is useful for copying data from tables that are experiencing write
traffic. Reading the contended table's data at a historical timestamp
avoids contention on the CREATE TABLE AS.
Release note (sql change): CREATE TABLE AS SELECT ... FROM ... AS OF
SYSTEM TIME x is now supported. It cannot be executed within an explicit
transaction.
spilchen
left a comment
There was a problem hiding this comment.
Reviewed 3 of 3 files at r2, 1 of 1 files at r3.
Reviewable status:
|
tftrs! bors r+ |
|
This is amazing! 🥳 |
|
Will this support DML operations such as insert into A select * from B asot ? |
Previously, running a statement of the form
was not supported.
Now, it is supported. The semantics are that the table creation happens at the transaction timestamp, but the backfill that's performed to fetch the data from the
SELECTis performed at the user-specified timestamp x.This is useful for copying data from tables that are experiencing write traffic. Reading the contended table's data at a historical timestamp avoids contention on the CREATE TABLE AS.
informs #39123
Epic: CRDB-43310
Release note (sql change): CREATE TABLE AS SELECT ... FROM ... AS OF SYSTEM TIME x is now supported. It cannot be executed within an explicit transaction.