Skip to content

Commit

Permalink
opt: avoid using virtual columns that are being mutated
Browse files Browse the repository at this point in the history
Previously, when schema changes involving dropping a virtual
compute column were concurrently run with UPDATE/DELETE, we
would encounter a panic inside the optimizer. Specifically,
because mutated virtual columns would not have information
populated. To address this, this patch similar to SELECT's,
will skip these columns when evaluating DROP's/UPDATE's.

Epic: none

Release note (bug fix): Fixed a bug where concurrent UPDATE/DELETE
while dropping a virtual computed column could fail with a panic.
  • Loading branch information
fqazi committed Mar 17, 2023
1 parent 0f36130 commit 72b76d2
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 13 deletions.
14 changes: 8 additions & 6 deletions pkg/sql/opt/optbuilder/mutation_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,10 @@ func (mb *mutationBuilder) buildInputForUpdate(
mb.fetchScope = mb.b.buildScan(
mb.b.addTable(mb.tab, &mb.alias),
tableOrdinals(mb.tab, columnKinds{
includeMutations: true,
includeSystem: true,
includeInverted: false,
includeMutations: true,
includeSystem: true,
includeInverted: false,
excludeVirtualComputedMutations: true,
}),
indexFlags,
noRowLocking,
Expand Down Expand Up @@ -399,9 +400,10 @@ func (mb *mutationBuilder) buildInputForDelete(
mb.fetchScope = mb.b.buildScan(
mb.b.addTable(mb.tab, &mb.alias),
tableOrdinals(mb.tab, columnKinds{
includeMutations: true,
includeSystem: true,
includeInverted: false,
includeMutations: true,
includeSystem: true,
includeInverted: false,
excludeVirtualComputedMutations: true,
}),
indexFlags,
noRowLocking,
Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/opt/optbuilder/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,9 @@ type columnKinds struct {

// If true, include inverted index columns.
includeInverted bool

// If true, virtual computed column mutations are excluded.
excludeVirtualComputedMutations bool
}

// tableOrdinals returns a slice of ordinals that correspond to table columns of
Expand All @@ -758,6 +761,11 @@ func tableOrdinals(tab cat.Table, k columnKinds) []int {
for i := 0; i < n; i++ {
col := tab.Column(i)
if shouldInclude[col.Kind()] {
// This behaviour matches the Builder.addComputedColsForTable, which
// will exclude any computed columns in mutation.
if k.excludeVirtualComputedMutations && col.IsVirtualComputed() && col.IsMutation() {
continue
}
ordinals = append(ordinals, i)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
setup
CREATE TABLE t (i INT PRIMARY KEY, j INT, k INT DEFAULT 32 ON UPDATE 42, INDEX((j+1), k));
INSERT INTO t VALUES(-1);
INSERT INTO t VALUES(-2);
INSERT INTO t VALUES(-3);
----
...
+object {100 101 t} -> 104
Expand All @@ -11,37 +14,64 @@ INSERT INTO t (i, j, k) VALUES($stageKey, $stageKey, $stageKey);
pq: column "j" does not exist

stage-exec phase=PostCommitPhase stage=:
INSERT INTO t (i) VALUES($stageKey);
INSERT INTO t VALUES($stageKey);
INSERT INTO t VALUES($stageKey + 1);
DELETE FROM t WHERE i=-1;
DELETE FROM t WHERE i=$stageKey;
INSERT INTO t VALUES($stageKey);
INSERT INTO t VALUES(-1);
----

stage-exec phase=PostCommitPhase stage=:
UPDATE t SET k=$stageKey;
----
pq: column "k" does not exist

stage-exec phase=PostCommitPhase stage=:
SELECT j+1, k FROM t
----
pq: column "j" does not exist

stage-exec phase=PostCommitPhase stage=:
SELECT count(i) FROM t
# Each insert will be injected twice per stage, plus 3 injected
# at the start.
stage-query phase=PostCommitPhase stage=:
SELECT count(*)=($successfulStageCount*2)+3 FROM t;
----
$successfulStageCount
true

stage-exec phase=PostCommitNonRevertiblePhase stage=:
INSERT INTO t (i, j, k) VALUES($stageKey, $stageKey, $stageKey);
----
pq: column "j" does not exist

stage-exec phase=PostCommitNonRevertiblePhase stage=:
INSERT INTO t (i) VALUES($stageKey);
INSERT INTO t VALUES($stageKey);
INSERT INTO t VALUES($stageKey + 1);
DELETE FROM t WHERE i=-1;
DELETE FROM t WHERE i=$stageKey;
INSERT INTO t VALUES($stageKey);
INSERT INTO t VALUES(-1);
----

stage-exec phase=PostCommitNonRevertiblePhase stage=:
UPDATE t SET k=$stageKey;
----
pq: column "k" does not exist

stage-exec phase=PostCommitNonRevertiblePhase stage=:
SELECT j+1, k FROM t
----
pq: column "j" does not exist

stage-exec phase=PostCommitNonRevertiblePhase stage=:
SELECT j+1, k FROM t
----
pq: column "j" does not exist

stage-exec phase=PostCommitNonRevertiblePhase stage=:
SELECT count(i) FROM t
SELECT count(*)=($successfulStageCount*2)+3 FROM t;
----
$successfulStageCount
true

test
ALTER TABLE t DROP COLUMN j CASCADE;
Expand Down

0 comments on commit 72b76d2

Please sign in to comment.