Skip to content

Commit

Permalink
sql/schemachanger: add a special case for simple add column
Browse files Browse the repository at this point in the history 
Before this change, when we added a column, we'd always create a new primary
index and swap into it. In the case where we're adding a nullable column with
no default value or computed expression, we have no reason to do this whole
dance. Indeed, backfilling that new index was a major regression for this
common case when compared to the legacy schema changer.

In this commit, we detect this special case and just add the new column to
the existing primary index.

Release note: None
  • Loading branch information
@ajwerner
ajwerner committed Jun 28, 2022
1 parent c54f99d commit 0c3f215
Show file tree
Hide file tree
Showing 11 changed files with 426 additions and 1,094 deletions.
27 changes: 18 additions & 9 deletions pkg/ccl/changefeedccl/changefeed_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6311,14 +6311,25 @@ func TestChangefeedPredicateWithSchemaChange(t *testing.T) {
}

for _, tc := range []testCase{
// The default policy is to skip schema changes which add new columns which
{
name: "add column",
name: "add column no default",
createFeedStmt: "CREATE CHANGEFEED AS SELECT * FROM foo",
initialPayload: initialPayload,
alterStmt: "ALTER TABLE foo ADD COLUMN new STRING",
afterAlterStmt: "INSERT INTO foo (a, b) VALUES (3, 'tres')",
payload: []string{
`foo: [3, "tres"]->{"after": {"a": 3, "b": "tres", "c": null, "e": "inactive", "new": null}}`,
},
},
{
name: "add column",
createFeedStmt: "CREATE CHANGEFEED AS SELECT * FROM foo",
initialPayload: initialPayload,
alterStmt: "ALTER TABLE foo ADD COLUMN new STRING DEFAULT 'new'",
payload: []string{
`foo: [1, "one"]->{"after": {"a": 1, "b": "one", "c": null, "e": "inactive", "new": null}}`,
`foo: [2, "two"]->{"after": {"a": 2, "b": "two", "c": "c string", "e": "open", "new": null}}`,
`foo: [1, "one"]->{"after": {"a": 1, "b": "one", "c": null, "e": "inactive", "new": "new"}}`,
`foo: [2, "two"]->{"after": {"a": 2, "b": "two", "c": "c string", "e": "open", "new": "new"}}`,
},
},
{
Expand All @@ -6328,11 +6339,11 @@ func TestChangefeedPredicateWithSchemaChange(t *testing.T) {
name: "add alt.status",
createFeedStmt: "CREATE CHANGEFEED AS SELECT * FROM foo",
initialPayload: initialPayload,
alterStmt: "ALTER TABLE foo ADD COLUMN alt alt.status",
alterStmt: "ALTER TABLE foo ADD COLUMN alt alt.status DEFAULT 'alt_closed'",
afterAlterStmt: "INSERT INTO foo (a, b, alt) VALUES (3, 'tres', 'alt_open')",
payload: []string{
`foo: [1, "one"]->{"after": {"a": 1, "alt": null, "b": "one", "c": null, "e": "inactive"}}`,
`foo: [2, "two"]->{"after": {"a": 2, "alt": null, "b": "two", "c": "c string", "e": "open"}}`,
`foo: [1, "one"]->{"after": {"a": 1, "alt": "alt_closed", "b": "one", "c": null, "e": "inactive"}}`,
`foo: [2, "two"]->{"after": {"a": 2, "alt": "alt_closed", "b": "two", "c": "c string", "e": "open"}}`,
`foo: [3, "tres"]->{"after": {"a": 3, "alt": "alt_open", "b": "tres", "c": null, "e": "inactive"}}`,
},
},
Expand Down Expand Up @@ -6417,9 +6428,7 @@ func TestChangefeedPredicateWithSchemaChange(t *testing.T) {
},
} {
t.Run(tc.name, func(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
cdcTest(t, testFn(tc), feedTestEnterpriseSinks)
})
cdcTest(t, testFn(tc), feedTestEnterpriseSinks)
})
}
}
Expand Down
20 changes: 2 additions & 18 deletions pkg/ccl/kvccl/kvfollowerreadsccl/testdata/boundedstaleness/single_row
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,40 +241,24 @@ query idx=2
SELECT * FROM t2 AS OF SYSTEM TIME with_min_timestamp(now() - '10s', true) WHERE pk = 2
----
pq: referenced descriptor ID 105: descriptor not found
events (15 found):
events (7 found):
* event 1: colbatchscan trace on node_idx 2: local read
* event 2: transaction retry on node_idx: 2
* event 3: colbatchscan trace on node_idx 2: local read
* event 4: transaction retry on node_idx: 2
* event 5: colbatchscan trace on node_idx 2: local read
* event 6: transaction retry on node_idx: 2
* event 7: colbatchscan trace on node_idx 2: local read
* event 8: transaction retry on node_idx: 2
* event 9: colbatchscan trace on node_idx 2: local read
* event 10: transaction retry on node_idx: 2
* event 11: colbatchscan trace on node_idx 2: local read
* event 12: transaction retry on node_idx: 2
* event 13: colbatchscan trace on node_idx 2: local read
* event 14: transaction retry on node_idx: 2
* event 15: colbatchscan trace on node_idx 2: local read

query idx=2
SELECT * FROM t2 AS OF SYSTEM TIME with_min_timestamp(now() - '10s', true) WHERE pk = 2
----
pq: referenced descriptor ID 105: descriptor not found
events (15 found):
events (7 found):
* event 1: colbatchscan trace on node_idx 2: local read
* event 2: transaction retry on node_idx: 2
* event 3: colbatchscan trace on node_idx 2: local read
* event 4: transaction retry on node_idx: 2
* event 5: colbatchscan trace on node_idx 2: local read
* event 6: transaction retry on node_idx: 2
* event 7: colbatchscan trace on node_idx 2: local read
* event 8: transaction retry on node_idx: 2
* event 9: colbatchscan trace on node_idx 2: local read
* event 10: transaction retry on node_idx: 2
* event 11: colbatchscan trace on node_idx 2: local read
* event 12: transaction retry on node_idx: 2
* event 13: colbatchscan trace on node_idx 2: local read
* event 14: transaction retry on node_idx: 2
* event 15: colbatchscan trace on node_idx 2: local read
6 changes: 3 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/information_schema
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1421,7 +1421,7 @@ SELECT table_catalog, table_name, version
WHERE table_catalog != 'system' AND version > 1 AND table_schema = 'public' ORDER BY 1,2
----
table_catalog table_name version
other_db xyz 10
other_db xyz 6

user testuser

Expand All @@ -1431,7 +1431,7 @@ query TTTTTI colnames
SELECT * FROM other_db.information_schema.tables WHERE table_catalog = 'other_db' AND table_schema = 'public'
----
table_catalog table_schema table_name table_type is_insertable_into version
other_db public xyz BASE TABLE YES 10
other_db public xyz BASE TABLE YES 6
other_db public abc VIEW NO 1


Expand All @@ -1448,7 +1448,7 @@ SELECT * FROM other_db.information_schema.tables WHERE table_catalog = 'other_db
----
table_catalog table_schema table_name table_type is_insertable_into version
other_db public abc VIEW NO 2
other_db public xyz BASE TABLE YES 10
other_db public xyz BASE TABLE YES 6

user root

Expand Down
Loading

0 comments on commit 0c3f215

Please sign in to comment.