crosscluster: add new insert/update/delete replication statements #143157
Conversation
|
This change is |
These statements take advantage of the KV level LWW validation added by PR cockroachdb#143100 and do not explicitly validate the origin timestamp. Update and Delete specify the whole row in the where clause, which is intended to enable future SQL optimizations that generate a CPUT to replace the row instead Release note: none Epic: CRDB-48647
jeffswenson
force-pushed
the
jeffswenson-ldr-crud-sql-statements
branch
from
50c5686 to
c52a758
Compare
| columns := table.AllColumns() | ||
| result := make([]catalog.Column, 0, len(columns)) | ||
| for _, col := range columns { | ||
| if !col.IsComputed() && !col.IsVirtual() && !col.IsSystemColumn() { |
There was a problem hiding this comment.
Based on some semantic checks when building INSERT statements, I believe we also need col.IsPublic() to avoid mutation columns and !col.IsInaccessible() to avoid inaccessible columns (which are used by expression indexes among other things).
(Or it might be easier to use table.AccessibleColumns() instead of table.AllColumns().)
There was a problem hiding this comment.
Hmm, but it looks like changefeeds use WritableColumns, i.e. they include some mutation columns. I'm a little confused about the interaction of LDR with ALTER. 🤔
| names := tree.NameList{nameNode} | ||
|
|
||
| // Create a placeholder for the new value (num_columns+i+1) since we | ||
| // placholders to the where and placholders are 1 indexed. |
There was a problem hiding this comment.
Is this comment missing a word?
| func newDeleteStatement(table catalog.TableDescriptor) (tree.Statement, error) { | ||
| columns := getPhysicalColumns(table) | ||
|
|
||
| // Create WHERE clause placeholders for every column in the table |
There was a problem hiding this comment.
nit: might be able to factor this out of both here and newUpdateStatement
|
|
||
| show-delete table=products | ||
| ---- | ||
| DELETE FROM [108 AS replication_target] WHERE ((((id = $1) AND (name = $2)) AND (unit_price = $3)) AND (quantity = $4)) AND (last_updated = $5) |
There was a problem hiding this comment.
Cool test!
It might be nice to also see the statements for:
- a regional by row table
- a table with an expression index
- a table with a partial index
- a table with an inverted index
- a table with a virtual computed column
These statements take advantage of the KV level LWW validation added by PR #143100 and do not explicitly validate the origin timestamp. Update and Delete specify the whole row in the where clause, which is intended to enable future SQL optimizations that generate a CPUT to replace the row instead
Release note: none
Epic: CRDB-48647