From 1b30ffa4669b459d29ba3e6d1bde605fa4bdd629 Mon Sep 17 00:00:00 2001 From: Parker Timmerman Date: Tue, 17 Dec 2024 15:57:38 -0500 Subject: [PATCH] one more round of fixes * change AstDisplay for ResolvedItemName to not print the version * remove commented our impl of previous sorting * update legacy upgrade test --- .../checks/all_checks/alter_table.py | 11 ++++++---- misc/python/materialize/mzcompose/__init__.py | 3 +++ src/sql/src/names.rs | 10 ++++++---- src/storage-client/src/storage_collections.rs | 20 ------------------- .../check-from-v0.128.0-alter-table.td | 14 ++++++------- test/sqllogictest/alter-table.slt | 4 ++-- 6 files changed, 25 insertions(+), 37 deletions(-) diff --git a/misc/python/materialize/checks/all_checks/alter_table.py b/misc/python/materialize/checks/all_checks/alter_table.py index bde56fd3888ae..97969fcdc0970 100644 --- a/misc/python/materialize/checks/all_checks/alter_table.py +++ b/misc/python/materialize/checks/all_checks/alter_table.py @@ -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( @@ -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; @@ -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; diff --git a/misc/python/materialize/mzcompose/__init__.py b/misc/python/materialize/mzcompose/__init__.py index b74ad162f2e2f..fa64c8bc36ad3 100644 --- a/misc/python/materialize/mzcompose/__init__.py +++ b/misc/python/materialize/mzcompose/__init__.py @@ -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" ), diff --git a/src/sql/src/names.rs b/src/sql/src/names.rs index 1bd70b5b8b476..27a0f87a818dd 100644 --- a/src/sql/src/names.rs +++ b/src/sql/src/names.rs @@ -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 { diff --git a/src/storage-client/src/storage_collections.rs b/src/storage-client/src/storage_collections.rs index 13ea3222dbd56..62f569e9d5c9b 100644 --- a/src/storage-client/src/storage_collections.rs +++ b/src/storage-client/src/storage_collections.rs @@ -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"); diff --git a/test/legacy-upgrade/check-from-v0.128.0-alter-table.td b/test/legacy-upgrade/check-from-v0.128.0-alter-table.td index 38c99b43f7cb7..5995ffebe3f81 100644 --- a/test/legacy-upgrade/check-from-v0.128.0-alter-table.td +++ b/test/legacy-upgrade/check-from-v0.128.0-alter-table.td @@ -47,10 +47,10 @@ 100 > SELECT * FROM alter_table_t1_mv2 ORDER BY a DESC; - -600 orange -500 apple -400 world -300 -200 hello -100 + +600 +500 +400 +300 +200 +100 diff --git a/test/sqllogictest/alter-table.slt b/test/sqllogictest/alter-table.slt index 11bdee97dbdbb..0ac7f36c7ec59 100644 --- a/test/sqllogictest/alter-table.slt +++ b/test/sqllogictest/alter-table.slt @@ -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 @@ -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;