Skip to content

Commit 97811b1

Browse files
fqazifqazi
committed
sql/schemachanger: enforce order for swapping default column values
Previously if the default value of an expression was being swapped, the order was implicit enforced by the order in which objects were added. This works fine until more complex rule evaluation is needed, for example schema_locked is set on the table, so the order of operations can end up shifting. To address this, this patch adds an explicit order for default column expression swaps. Fixes: #144241 Release note: None
1 parent 6146578 commit 97811b1

File tree

8 files changed

+221
-0
lines changed

8 files changed

+221
-0
lines changed

‎pkg/ccl/schemachangerccl/backup_base_generated_test.go‎

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/sql/schemachanger/scplan/internal/rules/current/dep_drop_column.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,3 +305,25 @@ func init() {
305305
},
306306
)
307307
}
308+
309+
// Special rules to ensure that swapping default expressions is done in order.
310+
func init() {
311+
registerDepRule(
312+
"handle default column expression swaps",
313+
scgraph.Precedence,
314+
"old-column-expression", "new-column-expression",
315+
func(from, to NodeVars) rel.Clauses {
316+
return rel.Clauses{
317+
from.Type((*scpb.ColumnDefaultExpression)(nil), (*scpb.ColumnOnUpdateExpression)(nil)),
318+
to.Type((*scpb.ColumnDefaultExpression)(nil), (*scpb.ColumnOnUpdateExpression)(nil)),
319+
from.El.AttrEqVar(rel.Type, "same-type"),
320+
to.El.AttrEqVar(rel.Type, "same-type"),
321+
JoinOnColumnID(from, to, "table-id", "col-id"),
322+
from.TargetStatus(scpb.ToAbsent),
323+
from.CurrentStatus(scpb.Status_ABSENT),
324+
to.TargetStatus(scpb.ToPublic),
325+
to.CurrentStatus(scpb.Status_PUBLIC),
326+
}
327+
},
328+
)
329+
}

‎pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3676,6 +3676,22 @@ deprules
36763676
- $function-parent-Node[CurrentStatus] = PUBLIC
36773677
- joinTargetNode($function-name, $function-name-Target, $function-name-Node)
36783678
- joinTargetNode($function-parent, $function-parent-Target, $function-parent-Node)
3679+
- name: handle default column expression swaps
3680+
from: old-column-expression-Node
3681+
kind: Precedence
3682+
to: new-column-expression-Node
3683+
query:
3684+
- $old-column-expression[Type] IN ['*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression']
3685+
- $new-column-expression[Type] IN ['*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression']
3686+
- $old-column-expression[Type] = $same-type
3687+
- $new-column-expression[Type] = $same-type
3688+
- joinOnColumnID($old-column-expression, $new-column-expression, $table-id, $col-id)
3689+
- $old-column-expression-Target[TargetStatus] = ABSENT
3690+
- $old-column-expression-Node[CurrentStatus] = ABSENT
3691+
- $new-column-expression-Target[TargetStatus] = PUBLIC
3692+
- $new-column-expression-Node[CurrentStatus] = PUBLIC
3693+
- joinTargetNode($old-column-expression, $old-column-expression-Target, $old-column-expression-Node)
3694+
- joinTargetNode($new-column-expression, $new-column-expression-Target, $new-column-expression-Node)
36793695
- name: index data exists as soon as index accepts backfills
36803696
from: index-name-Node
36813697
kind: SameStagePrecedence
@@ -8581,6 +8597,22 @@ deprules
85818597
- $function-parent-Node[CurrentStatus] = PUBLIC
85828598
- joinTargetNode($function-name, $function-name-Target, $function-name-Node)
85838599
- joinTargetNode($function-parent, $function-parent-Target, $function-parent-Node)
8600+
- name: handle default column expression swaps
8601+
from: old-column-expression-Node
8602+
kind: Precedence
8603+
to: new-column-expression-Node
8604+
query:
8605+
- $old-column-expression[Type] IN ['*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression']
8606+
- $new-column-expression[Type] IN ['*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression']
8607+
- $old-column-expression[Type] = $same-type
8608+
- $new-column-expression[Type] = $same-type
8609+
- joinOnColumnID($old-column-expression, $new-column-expression, $table-id, $col-id)
8610+
- $old-column-expression-Target[TargetStatus] = ABSENT
8611+
- $old-column-expression-Node[CurrentStatus] = ABSENT
8612+
- $new-column-expression-Target[TargetStatus] = PUBLIC
8613+
- $new-column-expression-Node[CurrentStatus] = PUBLIC
8614+
- joinTargetNode($old-column-expression, $old-column-expression-Target, $old-column-expression-Node)
8615+
- joinTargetNode($new-column-expression, $new-column-expression-Target, $new-column-expression-Node)
85848616
- name: index data exists as soon as index accepts backfills
85858617
from: index-name-Node
85868618
kind: SameStagePrecedence

‎pkg/sql/schemachanger/sctest_generated_test.go‎

Lines changed: 42 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/sql/schemachanger/testdata/end_to_end/alter_table_alter_column_swap_default/alter_table_alter_column_swap_default.definition‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
setup
2+
CREATE TABLE t (i INT PRIMARY KEY, j INT DEFAULT 32);
3+
----
4+
5+
test
6+
ALTER TABLE t ALTER COLUMN j SET DEFAULT 42;
7+
----

‎pkg/sql/schemachanger/testdata/end_to_end/alter_table_alter_column_swap_default/alter_table_alter_column_swap_default.explain‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/* setup */
2+
CREATE TABLE t (i INT PRIMARY KEY, j INT DEFAULT 32);
3+
4+
/* test */
5+
EXPLAIN (DDL) ALTER TABLE t ALTER COLUMN j SET DEFAULT 42;
6+
----
7+
Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ALTER COLUMN ‹j› SET DEFAULT ‹42›;
8+
├── StatementPhase
9+
│ └── Stage 1 of 1 in StatementPhase
10+
│ ├── 1 element transitioning toward PUBLIC
11+
│ │ └── ABSENT → PUBLIC ColumnDefaultExpression:{DescID: 104 (t), ColumnID: 2 (j), Expr: 42:::INT8}
12+
│ ├── 1 element transitioning toward ABSENT
13+
│ │ └── PUBLIC → ABSENT ColumnDefaultExpression:{DescID: 104 (t), ColumnID: 2 (j), Expr: 32:::INT8}
14+
│ └── 2 Mutation operations
15+
│ ├── RemoveColumnDefaultExpression {"ColumnID":2,"TableID":104}
16+
│ └── AddColumnDefaultExpression {"Default":{"ColumnID":2,"TableID":104}}
17+
└── PreCommitPhase
18+
├── Stage 1 of 2 in PreCommitPhase
19+
│ ├── 1 element transitioning toward PUBLIC
20+
│ │ └── PUBLIC → ABSENT ColumnDefaultExpression:{DescID: 104 (t), ColumnID: 2 (j), Expr: 42:::INT8}
21+
│ ├── 1 element transitioning toward ABSENT
22+
│ │ └── ABSENT → PUBLIC ColumnDefaultExpression:{DescID: 104 (t), ColumnID: 2 (j), Expr: 32:::INT8}
23+
│ └── 1 Mutation operation
24+
│ └── UndoAllInTxnImmediateMutationOpSideEffects
25+
└── Stage 2 of 2 in PreCommitPhase
26+
├── 1 element transitioning toward PUBLIC
27+
│ └── ABSENT → PUBLIC ColumnDefaultExpression:{DescID: 104 (t), ColumnID: 2 (j), Expr: 42:::INT8}
28+
├── 1 element transitioning toward ABSENT
29+
│ └── PUBLIC → ABSENT ColumnDefaultExpression:{DescID: 104 (t), ColumnID: 2 (j), Expr: 32:::INT8}
30+
└── 2 Mutation operations
31+
├── RemoveColumnDefaultExpression {"ColumnID":2,"TableID":104}
32+
└── AddColumnDefaultExpression {"Default":{"ColumnID":2,"TableID":104}}

‎pkg/sql/schemachanger/testdata/end_to_end/alter_table_alter_column_swap_default/alter_table_alter_column_swap_default.explain_shape‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/* setup */
2+
CREATE TABLE t (i INT PRIMARY KEY, j INT DEFAULT 32);
3+
4+
/* test */
5+
EXPLAIN (DDL, SHAPE) ALTER TABLE t ALTER COLUMN j SET DEFAULT 42;
6+
----
7+
Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ALTER COLUMN ‹j› SET DEFAULT ‹42›;
8+
└── execute 1 system table mutations transaction

‎pkg/sql/schemachanger/testdata/end_to_end/alter_table_alter_column_swap_default/alter_table_alter_column_swap_default.side_effects‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/* setup */
2+
CREATE TABLE t (i INT PRIMARY KEY, j INT DEFAULT 32);
3+
----
4+
...
5+
+object {100 101 t} -> 104
6+
7+
/* test */
8+
ALTER TABLE t ALTER COLUMN j SET DEFAULT 42;
9+
----
10+
begin transaction #1
11+
# begin StatementPhase
12+
checking for feature: ALTER TABLE
13+
increment telemetry for sql.schema.alter_table
14+
increment telemetry for sql.schema.alter_table.set_default
15+
## StatementPhase stage 1 of 1 with 2 MutationType ops
16+
upsert descriptor #104
17+
...
18+
oid: 20
19+
width: 64
20+
- - defaultExpr: 32:::INT8
21+
+ - defaultExpr: 42:::INT8
22+
id: 2
23+
name: j
24+
...
25+
time: {}
26+
unexposedParentSchemaId: 101
27+
- version: "1"
28+
+ version: "2"
29+
# end StatementPhase
30+
# begin PreCommitPhase
31+
## PreCommitPhase stage 1 of 2 with 1 MutationType op
32+
undo all catalog changes within txn #1
33+
persist all catalog changes to storage
34+
## PreCommitPhase stage 2 of 2 with 2 MutationType ops
35+
upsert descriptor #104
36+
...
37+
oid: 20
38+
width: 64
39+
- - defaultExpr: 32:::INT8
40+
+ - defaultExpr: 42:::INT8
41+
id: 2
42+
name: j
43+
...
44+
time: {}
45+
unexposedParentSchemaId: 101
46+
- version: "1"
47+
+ version: "2"
48+
persist all catalog changes to storage
49+
# end PreCommitPhase
50+
commit transaction #1

0 commit comments

Comments
 (0)