Skip to content

Commit

Permalink
one more round of fixes
Browse files Browse the repository at this point in the history
* change AstDisplay for ResolvedItemName to not print the version
* remove commented our impl of previous sorting
* update legacy upgrade test
  • Loading branch information
ParkMyCar committed Dec 18, 2024
1 parent 9f5efee commit 1b30ffa
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 37 deletions.
11 changes: 7 additions & 4 deletions misc/python/materialize/checks/all_checks/alter_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

class AlterTableAddColumn(Check):
def _can_run(self, e: Executor) -> bool:
return self.base_version >= MzVersion.parse_mz("v0.127.0-dev")
return self.base_version >= MzVersion.parse_mz("v0.128.0-dev")

def initialize(self) -> Testdrive:
return Testdrive(
Expand All @@ -25,9 +25,6 @@ def initialize(self) -> Testdrive:
$postgres-execute connection=postgres://mz_system:materialize@${testdrive.materialize-internal-sql-addr}
ALTER SYSTEM SET enable_alter_table_add_column = true;
$postgres-execute connection=postgres://mz_system:materialize@${testdrive.materialize-internal-sql-addr}
ALTER SYSTEM SET persist_dangerous_enable_schema_evolution = true;
> CREATE TABLE alter_table1 (f1 INTEGER, f2 INTEGER NOT NULL DEFAULT 1234);
> INSERT INTO alter_table1 VALUES (100, 100);
> CREATE VIEW alter_table_view_1 AS SELECT * FROM alter_table1;
Expand All @@ -43,12 +40,18 @@ def manipulate(self) -> list[Testdrive]:
Testdrive(dedent(s))
for s in [
"""
$postgres-execute connection=postgres://mz_system:materialize@${testdrive.materialize-internal-sql-addr}
ALTER SYSTEM SET enable_alter_table_add_column = true;
> ALTER TABLE alter_table1 ADD COLUMN f3 text;
> INSERT INTO alter_table1 VALUES (200, 200, 'hello'), (NULL, 300, 'world'), (400, 400, NULL), (NULL, 500, NULL);
> CREATE MATERIALIZED VIEW alter_table_mv1 AS SELECT alter_table2.f1 as f1_2, alter_table1.f1 as f1_1 FROM alter_table1, alter_table2;
> CREATE INDEX alter_table1_idx ON alter_table1 (f1);
""",
"""
$postgres-execute connection=postgres://mz_system:materialize@${testdrive.materialize-internal-sql-addr}
ALTER SYSTEM SET enable_alter_table_add_column = true;
> ALTER TABLE alter_table2 ADD COLUMN f2 INTEGER;
> INSERT INTO alter_table2 VALUES ('foo', 900), ('bar', 800);
> CREATE VIEW alter_table_view_2 AS SELECT * FROM alter_table2;
Expand Down
3 changes: 3 additions & 0 deletions misc/python/materialize/mzcompose/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ def get_default_system_parameters(
"enable_table_keys": "true",
"enable_variadic_left_join_lowering": "true",
"enable_worker_core_affinity": "true",
"enable_alter_table_add_column": (
"true" if version >= MzVersion.parse_mz("v0.128.0-dev") else "false"
),
"persist_record_schema_id": (
"true" if version > MzVersion.parse_mz("v0.127.0-dev") else "false"
),
Expand Down
10 changes: 6 additions & 4 deletions src/sql/src/names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,10 +490,12 @@ impl AstDisplay for ResolvedItemName {
f.write_str(".");
f.write_node(&Ident::new_unchecked(&full_name.item));

if let RelationVersionSelector::Specific(version) = version {
let version: Version = (*version).into();
f.write_str(" VERSION ");
f.write_node(&version);
if *print_id {
if let RelationVersionSelector::Specific(version) = version {
let version: Version = (*version).into();
f.write_str(" VERSION ");
f.write_node(&version);
}
}

if *print_id {
Expand Down
20 changes: 0 additions & 20 deletions src/storage-client/src/storage_collections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1560,26 +1560,6 @@ where
.rev()
.chain(collections_to_register.into_iter());

// to_register.sort_unstable_by(|(a_id, a_desc, ..), (b_id, b_desc, ..)| {
// match (&a_desc.data_source, &b_desc.data_source) {
// // Earlier versions of a table depend on later versions.
// (DataSource::Table { primary: a_col }, DataSource::Table { primary: b_col }) => {
// match (a_col, b_col) {
// // Primary collections depend on nothing, so just sort by GlobalId.
// (None, None) => a_id.cmp(b_id),
// // Sort collections with no dependencies before those with dependencies.
// (None, Some(_)) => std::cmp::Ordering::Less,
// (Some(_), None) => std::cmp::Ordering::Greater,
// // Sort collections with greater GlobalIds before those with lower GlobalIds.
// (Some(a_col_id), Some(b_col_id)) => b_col_id.cmp(a_col_id),
// }
// }
// // Tables should sort before everything.
// (DataSource::Table { .. }, _other) => std::cmp::Ordering::Less,
// (_a, _b) => a_id.cmp(b_id),
// }
// });

// We hold this lock for a very short amount of time, just doing some
// hashmap inserts and unbounded channel sends.
let mut self_collections = self.collections.lock().expect("lock poisoned");
Expand Down
14 changes: 7 additions & 7 deletions test/legacy-upgrade/check-from-v0.128.0-alter-table.td
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@
100 <null>

> SELECT * FROM alter_table_t1_mv2 ORDER BY a DESC;
<null> <null>
600 orange
500 apple
400 world
300 <null>
200 hello
100 <null>
<null>
600
500
400
300
200
100
4 changes: 2 additions & 2 deletions test/sqllogictest/alter-table.slt
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ CREATE VIEW v2 AS SELECT * FROM t2;
query TT
SHOW CREATE VIEW v1
----
materialize.public.v1 CREATE␠VIEW␠"materialize"."public"."v1"␠AS␠SELECT␠*␠FROM␠"materialize"."public"."t2"␠VERSION␠0
materialize.public.v1 CREATE␠VIEW␠"materialize"."public"."v1"␠AS␠SELECT␠*␠FROM␠"materialize"."public"."t2"

# Note: When the feature is off we should not record versions.
query TT
Expand Down Expand Up @@ -87,7 +87,7 @@ materialize.public.t1 CREATE␠TABLE␠"materialize"."public"."t1"␠("a"␠"pg
query TT
SHOW CREATE VIEW v1;
----
materialize.public.v1 CREATE␠VIEW␠"materialize"."public"."v1"␠AS␠SELECT␠*␠FROM␠"materialize"."public"."t1"␠VERSION␠0
materialize.public.v1 CREATE␠VIEW␠"materialize"."public"."v1"␠AS␠SELECT␠*␠FROM␠"materialize"."public"."t1"

statement ok
CREATE VIEW v2 AS SELECT * FROM t1;
Expand Down

0 comments on commit 1b30ffa

Please sign in to comment.