From 6065de3073b10bcd41886ee5fddbabd308453576 Mon Sep 17 00:00:00 2001 From: zhangwanyun1 <643044969@qq.com> Date: Sun, 13 Oct 2024 17:52:35 +0800 Subject: [PATCH 1/8] Add setting check_table_structure_completely --- .../statements/alter/partition.md | 6 +- src/Core/SettingsChangesHistory.cpp | 1 + src/Storages/MergeTree/MergeTreeData.cpp | 8 +- src/Storages/MergeTree/MergeTreeSettings.cpp | 2 +- .../__init__.py | 0 ..._with_check_table_structure_completely.xml | 5 + ...thout_check_table_structure_completely.xml | 5 + .../test.py | 420 ++++++++++++++++++ 8 files changed, 440 insertions(+), 7 deletions(-) create mode 100644 tests/integration/test_attach_with_different_projections_or_indices/__init__.py create mode 100644 tests/integration/test_attach_with_different_projections_or_indices/configs/config_with_check_table_structure_completely.xml create mode 100644 tests/integration/test_attach_with_different_projections_or_indices/configs/config_without_check_table_structure_completely.xml create mode 100644 tests/integration/test_attach_with_different_projections_or_indices/test.py diff --git a/docs/en/sql-reference/statements/alter/partition.md b/docs/en/sql-reference/statements/alter/partition.md index 11926b2aa08..cbf83499a0f 100644 --- a/docs/en/sql-reference/statements/alter/partition.md +++ b/docs/en/sql-reference/statements/alter/partition.md @@ -131,8 +131,8 @@ For the query to run successfully, the following conditions must be met: - Both tables must have the same structure. - Both tables must have the same partition key, the same order by key and the same primary key. -- Both tables must have the same indices and projections. - Both tables must have the same storage policy. +- The dest table should have at least the same definitions as the source table, and can have more projections and secondary indices(If check_table_structure_completely is set to true, both tables must have the same indices and projections). ## REPLACE PARTITION @@ -151,8 +151,8 @@ For the query to run successfully, the following conditions must be met: - Both tables must have the same structure. - Both tables must have the same partition key, the same order by key and the same primary key. -- Both tables must have the same indices and projections. - Both tables must have the same storage policy. +- The dest table should have at least the same definitions as the source table, and can have more projections and secondary indices(If check_table_structure_completely is set to true, both tables must have the same indices and projections). ## MOVE PARTITION TO TABLE @@ -166,9 +166,9 @@ For the query to run successfully, the following conditions must be met: - Both tables must have the same structure. - Both tables must have the same partition key, the same order by key and the same primary key. -- Both tables must have the same indices and projections. - Both tables must have the same storage policy. - Both tables must be the same engine family (replicated or non-replicated). +- The dest table should have at least the same definitions as the source table, and can have more projections and secondary indices(If check_table_structure_completely is set to true, both tables must have the same indices and projections). ## CLEAR COLUMN IN PARTITION diff --git a/src/Core/SettingsChangesHistory.cpp b/src/Core/SettingsChangesHistory.cpp index 42c92481fce..34e3e9ec3f4 100644 --- a/src/Core/SettingsChangesHistory.cpp +++ b/src/Core/SettingsChangesHistory.cpp @@ -68,6 +68,7 @@ static std::initializer_listgetPrimaryKeyAST()) != query_to_string(src_snapshot->getPrimaryKeyAST())) throw Exception(ErrorCodes::BAD_ARGUMENTS, "Tables have different primary key"); - - const auto check_definitions = [](const auto & my_descriptions, const auto & src_descriptions) + const auto check_definitions = [this](const auto & my_descriptions, const auto & src_descriptions) { - if (my_descriptions.size() != src_descriptions.size()) + if (((*getSettings())[MergeTreeSetting::check_table_structure_completely] && my_descriptions.size() != src_descriptions.size()) + || (!(*getSettings())[MergeTreeSetting::check_table_structure_completely] && my_descriptions.size() < src_descriptions.size())) return false; std::unordered_set my_query_strings; diff --git a/src/Storages/MergeTree/MergeTreeSettings.cpp b/src/Storages/MergeTree/MergeTreeSettings.cpp index 77cff4ca527..cc4cd5a32ea 100644 --- a/src/Storages/MergeTree/MergeTreeSettings.cpp +++ b/src/Storages/MergeTree/MergeTreeSettings.cpp @@ -92,7 +92,7 @@ namespace ErrorCodes M(String, merge_workload, "", "Name of workload to be used to access resources for merges", 0) \ M(String, mutation_workload, "", "Name of workload to be used to access resources for mutations", 0) \ M(Milliseconds, background_task_preferred_step_execution_time_ms, 50, "Target time to execution of one step of merge or mutation. Can be exceeded if one step takes longer time", 0) \ - \ + M(Bool, check_table_structure_completely, false, "If true, tables must have identical definitions, including projections and secondary indices. Otherwise, the source table's projections and secondary indices must be a subset of those in the target table.", 0) \ /** Inserts settings. */ \ M(UInt64, parts_to_delay_insert, 1000, "If table contains at least that many active parts in single partition, artificially slow down insert into table. Disabled if set to 0", 0) \ M(UInt64, inactive_parts_to_delay_insert, 0, "If table contains at least that many inactive parts in single partition, artificially slow down insert into table.", 0) \ diff --git a/tests/integration/test_attach_with_different_projections_or_indices/__init__.py b/tests/integration/test_attach_with_different_projections_or_indices/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/integration/test_attach_with_different_projections_or_indices/configs/config_with_check_table_structure_completely.xml b/tests/integration/test_attach_with_different_projections_or_indices/configs/config_with_check_table_structure_completely.xml new file mode 100644 index 00000000000..dd9ae5e83d5 --- /dev/null +++ b/tests/integration/test_attach_with_different_projections_or_indices/configs/config_with_check_table_structure_completely.xml @@ -0,0 +1,5 @@ + + + true + + diff --git a/tests/integration/test_attach_with_different_projections_or_indices/configs/config_without_check_table_structure_completely.xml b/tests/integration/test_attach_with_different_projections_or_indices/configs/config_without_check_table_structure_completely.xml new file mode 100644 index 00000000000..734294478ba --- /dev/null +++ b/tests/integration/test_attach_with_different_projections_or_indices/configs/config_without_check_table_structure_completely.xml @@ -0,0 +1,5 @@ + + + false + + \ No newline at end of file diff --git a/tests/integration/test_attach_with_different_projections_or_indices/test.py b/tests/integration/test_attach_with_different_projections_or_indices/test.py new file mode 100644 index 00000000000..bff993e608d --- /dev/null +++ b/tests/integration/test_attach_with_different_projections_or_indices/test.py @@ -0,0 +1,420 @@ +import pytest +from helpers.cluster import ClickHouseCluster +from helpers.client import QueryRuntimeException +cluster = ClickHouseCluster(__file__) +node1 = cluster.add_instance( + "node1", main_configs=["configs/config_with_check_table_structure_completely.xml"] +) +# node1 = cluster.add_instance("node1") +node2 = cluster.add_instance( + "node2", main_configs=["configs/config_without_check_table_structure_completely.xml"] +) +@pytest.fixture(scope="module") +def start_cluster(): + try: + cluster.start() + yield cluster + finally: + cluster.shutdown() +# def test_setting_check_table_structure_completely(start_cluster): +# assert node1.query("""select value from system.merge_tree_settings where name='check_table_structure_completely';""") == "0\n" +def test_check_completely_attach_with_different_indices(start_cluster): + node1.query( + """ + CREATE TABLE attach_partition_t1 + ( + `a` UInt32, + `b` String, + `c` String, + INDEX bf b TYPE tokenbf_v1(8192, 3, 0) GRANULARITY 1 + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + node1.query("INSERT INTO attach_partition_t1 SELECT number, toString(number), toString(number) FROM numbers(10);") + node1.query( + """ + CREATE TABLE attach_partition_t2 + ( + `a` UInt32, + `b` String, + `c` String, + INDEX bf b TYPE bloom_filter GRANULARITY 1 + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + # serverError 36 + with pytest.raises(QueryRuntimeException) as exc: + node1.query("ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;") + assert "Tables have different secondary indices" in str(exc.value) + node1.query( + """ + CREATE TABLE attach_partition_t3 + ( + `a` UInt32, + `b` String, + `c` String, + INDEX bf b TYPE tokenbf_v1(8192, 3, 0) GRANULARITY 1, + INDEX cf c TYPE tokenbf_v1(8192, 3, 0) GRANULARITY 1 + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + # serverError 36 + with pytest.raises(QueryRuntimeException) as exc: + node1.query("ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;") + assert "Tables have different secondary indices" in str(exc.value) + node1.query("DROP TABLE attach_partition_t1") + node1.query("DROP TABLE attach_partition_t2") + node1.query("DROP TABLE attach_partition_t3") +def test_check_attach_with_different_indices(start_cluster): + node2.query( + """ + CREATE TABLE attach_partition_t1 + ( + `a` UInt32, + `b` String, + `c` String, + INDEX bf b TYPE tokenbf_v1(8192, 3, 0) GRANULARITY 1 + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + node2.query("INSERT INTO attach_partition_t1 SELECT number, toString(number), toString(number) FROM numbers(10);") + node2.query( + """ + CREATE TABLE attach_partition_t2 + ( + `a` UInt32, + `b` String, + `c` String, + INDEX bf b TYPE bloom_filter GRANULARITY 1 + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + # serverError 36 + with pytest.raises(QueryRuntimeException) as exc: + node2.query("ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;") + assert "Tables have different secondary indices" in str(exc.value) + node2.query( + """ + CREATE TABLE attach_partition_t3 + ( + `a` UInt32, + `b` String, + `c` String, + INDEX bf b TYPE tokenbf_v1(8192, 3, 0) GRANULARITY 1, + INDEX cf c TYPE bloom_filter GRANULARITY 1 + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + node2.query("ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;") + assert node2.query("SELECT COUNT() FROM attach_partition_t3") == "10\n" + assert node2.query("SELECT `a` FROM attach_partition_t3 WHERE `b` = '1'") == "1\n" + assert node2.query("SELECT `a` FROM attach_partition_t3 WHERE `c` = '1'") == "1\n" + node2.query("DROP TABLE attach_partition_t1") + node2.query("DROP TABLE attach_partition_t2") + node2.query("DROP TABLE attach_partition_t3") +def test_check_completely_attach_with_different_projections(start_cluster): + node1.query( + """ + CREATE TABLE attach_partition_t1 + ( + `a` UInt32, + `b` String, + PROJECTION proj1 ( + SELECT + b, + sum(a) + GROUP BY b + ) + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + node1.query("INSERT INTO attach_partition_t1 SELECT number, toString(number) FROM numbers(10);") + node1.query( + """ + CREATE TABLE attach_partition_t2 + ( + `a` UInt32, + `b` String, + PROJECTION differently_named_proj ( + SELECT + b, + sum(a) + GROUP BY b + ) + ) + ENGINE = MergeTree + ORDER BY a; + """ + ) + # serverError 36 + with pytest.raises(QueryRuntimeException) as exc: + node1.query("ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;") + assert "Tables have different projections" in str(exc.value) + node1.query( + """ + CREATE TABLE attach_partition_t3 + ( + `a` UInt32, + `b` String, + PROJECTION proj1 ( + SELECT + b, + sum(a) + GROUP BY b + ), + PROJECTION proj2 ( + SELECT + b, + avg(a) + GROUP BY b + ) + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + # serverError 36 + with pytest.raises(QueryRuntimeException) as exc: + node1.query("ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;") + assert "Tables have different projections" in str(exc.value) + node1.query("DROP TABLE attach_partition_t1") + node1.query("DROP TABLE attach_partition_t2") + node1.query("DROP TABLE attach_partition_t3") +def test_check_attach_with_different_projections(start_cluster): + node2.query( + """ + CREATE TABLE attach_partition_t1 + ( + `a` UInt32, + `b` String, + PROJECTION proj1 ( + SELECT + b, + sum(a) + GROUP BY b + ) + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + node2.query("INSERT INTO attach_partition_t1 SELECT number, toString(number) FROM numbers(10);") + node2.query( + """ + CREATE TABLE attach_partition_t2 + ( + `a` UInt32, + `b` String, + PROJECTION differently_named_proj ( + SELECT + b, + sum(a) + GROUP BY b + ) + ) + ENGINE = MergeTree + ORDER BY a; + """ + ) + # serverError 36 + with pytest.raises(QueryRuntimeException) as exc: + node2.query("ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;") + assert "Tables have different projections" in str(exc.value) + node2.query( + """ + CREATE TABLE attach_partition_t3 + ( + `a` UInt32, + `b` String, + PROJECTION proj1 ( + SELECT + b, + sum(a) + GROUP BY b + ), + PROJECTION proj2 ( + SELECT + b, + avg(a) + GROUP BY b + ) + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + node2.query("ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;") + assert node2.query("SELECT COUNT() FROM attach_partition_t3") == "10\n" + node2.query("DROP TABLE attach_partition_t1") + node2.query("DROP TABLE attach_partition_t2") + node2.query("DROP TABLE attach_partition_t3") +def test_check_completely_attach_with_different_indices_and_projections(start_cluster): + node1.query( + """ + CREATE TABLE attach_partition_t1 + ( + `a` UInt32, + `b` String, + `c` String, + PROJECTION proj1 ( + SELECT + b, + sum(a) + GROUP BY b + ), + INDEX bf b TYPE tokenbf_v1(8192, 3, 0) GRANULARITY 1 + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + node1.query("INSERT INTO attach_partition_t1 SELECT number, toString(number), toString(number) FROM numbers(10);") + node1.query( + """ + CREATE TABLE attach_partition_t2 + ( + `a` UInt32, + `b` String, + `c` String, + PROJECTION proj ( + SELECT + b, + sum(a) + GROUP BY b + ), + INDEX bf b TYPE bloom_filter GRANULARITY 1, + INDEX cf c TYPE tokenbf_v1(8192, 3, 0) GRANULARITY 1 + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + # serverError 36 + with pytest.raises(QueryRuntimeException) as exc: + node1.query("ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;") + assert "Tables have different secondary indices" in str(exc.value) + node1.query( + """ + CREATE TABLE attach_partition_t3 + ( + `a` UInt32, + `b` String, + `c` String, + PROJECTION proj1 ( + SELECT + b, + sum(a) + GROUP BY b + ), + PROJECTION proj2 ( + SELECT + b, + avg(a) + GROUP BY b + ), + INDEX bf b TYPE tokenbf_v1(8192, 3, 0) GRANULARITY 1, + INDEX cf c TYPE bloom_filter GRANULARITY 1 + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + # serverError 36 + with pytest.raises(QueryRuntimeException) as exc: + node1.query("ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;") + assert "Tables have different secondary indices" in str(exc.value) + node1.query("DROP TABLE attach_partition_t1") + node1.query("DROP TABLE attach_partition_t2") + node1.query("DROP TABLE attach_partition_t3") +def test_check_attach_with_different_indices_and_projections(start_cluster): + node2.query( + """ + CREATE TABLE attach_partition_t1 + ( + `a` UInt32, + `b` String, + `c` String, + PROJECTION proj1 ( + SELECT + b, + sum(a) + GROUP BY b + ), + INDEX bf b TYPE tokenbf_v1(8192, 3, 0) GRANULARITY 1 + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + node2.query("INSERT INTO attach_partition_t1 SELECT number, toString(number), toString(number) FROM numbers(10);") + node2.query( + """ + CREATE TABLE attach_partition_t2 + ( + `a` UInt32, + `b` String, + `c` String, + PROJECTION proj ( + SELECT + b, + sum(a) + GROUP BY b + ), + INDEX bf b TYPE bloom_filter GRANULARITY 1, + INDEX cf c TYPE tokenbf_v1(8192, 3, 0) GRANULARITY 1 + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + # serverError 36 + with pytest.raises(QueryRuntimeException) as exc: + node2.query("ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;") + assert "Tables have different secondary indices" in str(exc.value) + node2.query( + """ + CREATE TABLE attach_partition_t3 + ( + `a` UInt32, + `b` String, + `c` String, + PROJECTION proj1 ( + SELECT + b, + sum(a) + GROUP BY b + ), + PROJECTION proj2 ( + SELECT + b, + avg(a) + GROUP BY b + ), + INDEX bf b TYPE tokenbf_v1(8192, 3, 0) GRANULARITY 1, + INDEX cf c TYPE bloom_filter GRANULARITY 1 + ) + ENGINE = MergeTree + ORDER BY a + """ + ) + node2.query("ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;") + assert node2.query("SELECT COUNT() FROM attach_partition_t3") == "10\n" + assert node2.query("SELECT `a` FROM attach_partition_t3 WHERE `b` = '1'") == "1\n" + assert node2.query("SELECT `a` FROM attach_partition_t3 WHERE `c` = '1'") == "1\n" + node2.query("DROP TABLE attach_partition_t1") + node2.query("DROP TABLE attach_partition_t2") + node2.query("DROP TABLE attach_partition_t3") From d8230416d1a43fe262ace9568c16aa95f18ad1c0 Mon Sep 17 00:00:00 2001 From: zhangwanyun1 <643044969@qq.com> Date: Wed, 16 Oct 2024 11:29:06 +0800 Subject: [PATCH 2/8] clarify the documentation --- src/Storages/MergeTree/MergeTreeSettings.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Storages/MergeTree/MergeTreeSettings.cpp b/src/Storages/MergeTree/MergeTreeSettings.cpp index cc4cd5a32ea..5bc84a73334 100644 --- a/src/Storages/MergeTree/MergeTreeSettings.cpp +++ b/src/Storages/MergeTree/MergeTreeSettings.cpp @@ -92,7 +92,7 @@ namespace ErrorCodes M(String, merge_workload, "", "Name of workload to be used to access resources for merges", 0) \ M(String, mutation_workload, "", "Name of workload to be used to access resources for mutations", 0) \ M(Milliseconds, background_task_preferred_step_execution_time_ms, 50, "Target time to execution of one step of merge or mutation. Can be exceeded if one step takes longer time", 0) \ - M(Bool, check_table_structure_completely, false, "If true, tables must have identical definitions, including projections and secondary indices. Otherwise, the source table's projections and secondary indices must be a subset of those in the target table.", 0) \ + M(Bool, check_table_structure_completely, false, "Whether to check table structure completely when manipulate partitions. If true, the source and target tables must have identical definitions including projections and secondary indices. Otherwise, the source table's projections and secondary indices must be a subset of those in the target table.", 0) \ /** Inserts settings. */ \ M(UInt64, parts_to_delay_insert, 1000, "If table contains at least that many active parts in single partition, artificially slow down insert into table. Disabled if set to 0", 0) \ M(UInt64, inactive_parts_to_delay_insert, 0, "If table contains at least that many inactive parts in single partition, artificially slow down insert into table.", 0) \ From 66de8310c31db09dcf0b13028909b065f34f1dd3 Mon Sep 17 00:00:00 2001 From: zhangwanyun1 <643044969@qq.com> Date: Fri, 18 Oct 2024 01:34:14 +0800 Subject: [PATCH 3/8] fix ci --- .../test.py | 93 +++++++++++++++---- 1 file changed, 73 insertions(+), 20 deletions(-) diff --git a/tests/integration/test_attach_with_different_projections_or_indices/test.py b/tests/integration/test_attach_with_different_projections_or_indices/test.py index bff993e608d..75ce524b006 100644 --- a/tests/integration/test_attach_with_different_projections_or_indices/test.py +++ b/tests/integration/test_attach_with_different_projections_or_indices/test.py @@ -1,14 +1,19 @@ import pytest -from helpers.cluster import ClickHouseCluster + from helpers.client import QueryRuntimeException +from helpers.cluster import ClickHouseCluster + cluster = ClickHouseCluster(__file__) node1 = cluster.add_instance( "node1", main_configs=["configs/config_with_check_table_structure_completely.xml"] ) # node1 = cluster.add_instance("node1") node2 = cluster.add_instance( - "node2", main_configs=["configs/config_without_check_table_structure_completely.xml"] + "node2", + main_configs=["configs/config_without_check_table_structure_completely.xml"], ) + + @pytest.fixture(scope="module") def start_cluster(): try: @@ -16,6 +21,8 @@ def start_cluster(): yield cluster finally: cluster.shutdown() + + # def test_setting_check_table_structure_completely(start_cluster): # assert node1.query("""select value from system.merge_tree_settings where name='check_table_structure_completely';""") == "0\n" def test_check_completely_attach_with_different_indices(start_cluster): @@ -32,7 +39,9 @@ def test_check_completely_attach_with_different_indices(start_cluster): ORDER BY a """ ) - node1.query("INSERT INTO attach_partition_t1 SELECT number, toString(number), toString(number) FROM numbers(10);") + node1.query( + "INSERT INTO attach_partition_t1 SELECT number, toString(number), toString(number) FROM numbers(10);" + ) node1.query( """ CREATE TABLE attach_partition_t2 @@ -48,7 +57,9 @@ def test_check_completely_attach_with_different_indices(start_cluster): ) # serverError 36 with pytest.raises(QueryRuntimeException) as exc: - node1.query("ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;") + node1.query( + "ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;" + ) assert "Tables have different secondary indices" in str(exc.value) node1.query( """ @@ -66,11 +77,15 @@ def test_check_completely_attach_with_different_indices(start_cluster): ) # serverError 36 with pytest.raises(QueryRuntimeException) as exc: - node1.query("ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;") + node1.query( + "ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;" + ) assert "Tables have different secondary indices" in str(exc.value) node1.query("DROP TABLE attach_partition_t1") node1.query("DROP TABLE attach_partition_t2") node1.query("DROP TABLE attach_partition_t3") + + def test_check_attach_with_different_indices(start_cluster): node2.query( """ @@ -85,7 +100,9 @@ def test_check_attach_with_different_indices(start_cluster): ORDER BY a """ ) - node2.query("INSERT INTO attach_partition_t1 SELECT number, toString(number), toString(number) FROM numbers(10);") + node2.query( + "INSERT INTO attach_partition_t1 SELECT number, toString(number), toString(number) FROM numbers(10);" + ) node2.query( """ CREATE TABLE attach_partition_t2 @@ -101,7 +118,9 @@ def test_check_attach_with_different_indices(start_cluster): ) # serverError 36 with pytest.raises(QueryRuntimeException) as exc: - node2.query("ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;") + node2.query( + "ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;" + ) assert "Tables have different secondary indices" in str(exc.value) node2.query( """ @@ -117,13 +136,17 @@ def test_check_attach_with_different_indices(start_cluster): ORDER BY a """ ) - node2.query("ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;") + node2.query( + "ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;" + ) assert node2.query("SELECT COUNT() FROM attach_partition_t3") == "10\n" assert node2.query("SELECT `a` FROM attach_partition_t3 WHERE `b` = '1'") == "1\n" assert node2.query("SELECT `a` FROM attach_partition_t3 WHERE `c` = '1'") == "1\n" node2.query("DROP TABLE attach_partition_t1") node2.query("DROP TABLE attach_partition_t2") node2.query("DROP TABLE attach_partition_t3") + + def test_check_completely_attach_with_different_projections(start_cluster): node1.query( """ @@ -142,7 +165,9 @@ def test_check_completely_attach_with_different_projections(start_cluster): ORDER BY a """ ) - node1.query("INSERT INTO attach_partition_t1 SELECT number, toString(number) FROM numbers(10);") + node1.query( + "INSERT INTO attach_partition_t1 SELECT number, toString(number) FROM numbers(10);" + ) node1.query( """ CREATE TABLE attach_partition_t2 @@ -162,7 +187,9 @@ def test_check_completely_attach_with_different_projections(start_cluster): ) # serverError 36 with pytest.raises(QueryRuntimeException) as exc: - node1.query("ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;") + node1.query( + "ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;" + ) assert "Tables have different projections" in str(exc.value) node1.query( """ @@ -189,11 +216,15 @@ def test_check_completely_attach_with_different_projections(start_cluster): ) # serverError 36 with pytest.raises(QueryRuntimeException) as exc: - node1.query("ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;") + node1.query( + "ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;" + ) assert "Tables have different projections" in str(exc.value) node1.query("DROP TABLE attach_partition_t1") node1.query("DROP TABLE attach_partition_t2") node1.query("DROP TABLE attach_partition_t3") + + def test_check_attach_with_different_projections(start_cluster): node2.query( """ @@ -212,7 +243,9 @@ def test_check_attach_with_different_projections(start_cluster): ORDER BY a """ ) - node2.query("INSERT INTO attach_partition_t1 SELECT number, toString(number) FROM numbers(10);") + node2.query( + "INSERT INTO attach_partition_t1 SELECT number, toString(number) FROM numbers(10);" + ) node2.query( """ CREATE TABLE attach_partition_t2 @@ -232,7 +265,9 @@ def test_check_attach_with_different_projections(start_cluster): ) # serverError 36 with pytest.raises(QueryRuntimeException) as exc: - node2.query("ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;") + node2.query( + "ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;" + ) assert "Tables have different projections" in str(exc.value) node2.query( """ @@ -257,11 +292,15 @@ def test_check_attach_with_different_projections(start_cluster): ORDER BY a """ ) - node2.query("ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;") + node2.query( + "ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;" + ) assert node2.query("SELECT COUNT() FROM attach_partition_t3") == "10\n" node2.query("DROP TABLE attach_partition_t1") node2.query("DROP TABLE attach_partition_t2") node2.query("DROP TABLE attach_partition_t3") + + def test_check_completely_attach_with_different_indices_and_projections(start_cluster): node1.query( """ @@ -282,7 +321,9 @@ def test_check_completely_attach_with_different_indices_and_projections(start_cl ORDER BY a """ ) - node1.query("INSERT INTO attach_partition_t1 SELECT number, toString(number), toString(number) FROM numbers(10);") + node1.query( + "INSERT INTO attach_partition_t1 SELECT number, toString(number), toString(number) FROM numbers(10);" + ) node1.query( """ CREATE TABLE attach_partition_t2 @@ -305,7 +346,9 @@ def test_check_completely_attach_with_different_indices_and_projections(start_cl ) # serverError 36 with pytest.raises(QueryRuntimeException) as exc: - node1.query("ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;") + node1.query( + "ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;" + ) assert "Tables have different secondary indices" in str(exc.value) node1.query( """ @@ -335,11 +378,15 @@ def test_check_completely_attach_with_different_indices_and_projections(start_cl ) # serverError 36 with pytest.raises(QueryRuntimeException) as exc: - node1.query("ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;") + node1.query( + "ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;" + ) assert "Tables have different secondary indices" in str(exc.value) node1.query("DROP TABLE attach_partition_t1") node1.query("DROP TABLE attach_partition_t2") node1.query("DROP TABLE attach_partition_t3") + + def test_check_attach_with_different_indices_and_projections(start_cluster): node2.query( """ @@ -360,7 +407,9 @@ def test_check_attach_with_different_indices_and_projections(start_cluster): ORDER BY a """ ) - node2.query("INSERT INTO attach_partition_t1 SELECT number, toString(number), toString(number) FROM numbers(10);") + node2.query( + "INSERT INTO attach_partition_t1 SELECT number, toString(number), toString(number) FROM numbers(10);" + ) node2.query( """ CREATE TABLE attach_partition_t2 @@ -383,7 +432,9 @@ def test_check_attach_with_different_indices_and_projections(start_cluster): ) # serverError 36 with pytest.raises(QueryRuntimeException) as exc: - node2.query("ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;") + node2.query( + "ALTER TABLE attach_partition_t2 ATTACH PARTITION tuple() FROM attach_partition_t1;" + ) assert "Tables have different secondary indices" in str(exc.value) node2.query( """ @@ -411,7 +462,9 @@ def test_check_attach_with_different_indices_and_projections(start_cluster): ORDER BY a """ ) - node2.query("ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;") + node2.query( + "ALTER TABLE attach_partition_t3 ATTACH PARTITION tuple() FROM attach_partition_t1;" + ) assert node2.query("SELECT COUNT() FROM attach_partition_t3") == "10\n" assert node2.query("SELECT `a` FROM attach_partition_t3 WHERE `b` = '1'") == "1\n" assert node2.query("SELECT `a` FROM attach_partition_t3 WHERE `c` = '1'") == "1\n" From ac2140d3af01fd95ebfd89131be1f467dc6a42e3 Mon Sep 17 00:00:00 2001 From: zhangwanyun1 <643044969@qq.com> Date: Sat, 19 Oct 2024 20:12:16 +0800 Subject: [PATCH 4/8] fix the location of the new setting --- src/Core/SettingsChangesHistory.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/SettingsChangesHistory.cpp b/src/Core/SettingsChangesHistory.cpp index bfb78b36a2d..3604137fd35 100644 --- a/src/Core/SettingsChangesHistory.cpp +++ b/src/Core/SettingsChangesHistory.cpp @@ -68,7 +68,6 @@ static std::initializer_list Date: Sun, 20 Oct 2024 00:54:29 +0800 Subject: [PATCH 5/8] retry ci From aa58fde1d70591ad13d02cfe63b376ef11925816 Mon Sep 17 00:00:00 2001 From: Vladimir Cherkasov Date: Mon, 21 Oct 2024 12:55:53 +0200 Subject: [PATCH 6/8] Cosmetic changes, upd setting name --- docs/en/sql-reference/statements/alter/partition.md | 6 +++--- src/Core/SettingsChangesHistory.cpp | 2 +- src/Interpreters/InterpreterCreateQuery.cpp | 8 ++++---- src/Storages/MergeTree/MergeTreeData.cpp | 7 ++++--- src/Storages/MergeTree/MergeTreeSettings.cpp | 2 +- .../config_with_check_table_structure_completely.xml | 2 +- .../config_without_check_table_structure_completely.xml | 2 +- .../test.py | 2 +- 8 files changed, 16 insertions(+), 15 deletions(-) diff --git a/docs/en/sql-reference/statements/alter/partition.md b/docs/en/sql-reference/statements/alter/partition.md index cbf83499a0f..087458a6646 100644 --- a/docs/en/sql-reference/statements/alter/partition.md +++ b/docs/en/sql-reference/statements/alter/partition.md @@ -132,7 +132,7 @@ For the query to run successfully, the following conditions must be met: - Both tables must have the same structure. - Both tables must have the same partition key, the same order by key and the same primary key. - Both tables must have the same storage policy. -- The dest table should have at least the same definitions as the source table, and can have more projections and secondary indices(If check_table_structure_completely is set to true, both tables must have the same indices and projections). +- The destination table must include all indices and projections from the source table. If the `enforce_index_structure_match_on_partition_manipulation` setting is enabled in destination table, the indices and projections must be identical. Otherwise, the destination table can have a superset of the source table’s indices and projections. ## REPLACE PARTITION @@ -152,7 +152,7 @@ For the query to run successfully, the following conditions must be met: - Both tables must have the same structure. - Both tables must have the same partition key, the same order by key and the same primary key. - Both tables must have the same storage policy. -- The dest table should have at least the same definitions as the source table, and can have more projections and secondary indices(If check_table_structure_completely is set to true, both tables must have the same indices and projections). +- The destination table must include all indices and projections from the source table. If the `enforce_index_structure_match_on_partition_manipulation` setting is enabled in destination table, the indices and projections must be identical. Otherwise, the destination table can have a superset of the source table’s indices and projections. ## MOVE PARTITION TO TABLE @@ -168,7 +168,7 @@ For the query to run successfully, the following conditions must be met: - Both tables must have the same partition key, the same order by key and the same primary key. - Both tables must have the same storage policy. - Both tables must be the same engine family (replicated or non-replicated). -- The dest table should have at least the same definitions as the source table, and can have more projections and secondary indices(If check_table_structure_completely is set to true, both tables must have the same indices and projections). +- The destination table must include all indices and projections from the source table. If the `enforce_index_structure_match_on_partition_manipulation` setting is enabled in destination table, the indices and projections must be identical. Otherwise, the destination table can have a superset of the source table’s indices and projections. ## CLEAR COLUMN IN PARTITION diff --git a/src/Core/SettingsChangesHistory.cpp b/src/Core/SettingsChangesHistory.cpp index 3604137fd35..3edbc682810 100644 --- a/src/Core/SettingsChangesHistory.cpp +++ b/src/Core/SettingsChangesHistory.cpp @@ -580,7 +580,7 @@ static std::initializer_listclone(), properties.columns, getContext()); if (properties.indices.has(index_desc.name)) - throw Exception(ErrorCodes::ILLEGAL_INDEX, "Duplicated index name {} is not allowed. Please use different index names.", backQuoteIfNeed(index_desc.name)); + throw Exception(ErrorCodes::ILLEGAL_INDEX, "Duplicated index name {} is not allowed. Please use a different index name", backQuoteIfNeed(index_desc.name)); const auto & settings = getContext()->getSettingsRef(); if (index_desc.type == FULL_TEXT_INDEX_NAME && !settings[Setting::allow_experimental_full_text_index]) - throw Exception(ErrorCodes::SUPPORT_IS_DISABLED, "Experimental full-text index feature is disabled. Turn on setting 'allow_experimental_full_text_index'"); + throw Exception(ErrorCodes::SUPPORT_IS_DISABLED, "The experimental full-text index feature is disabled. Enable the setting 'allow_experimental_full_text_index' to use it"); /// ---- /// Temporary check during a transition period. Please remove at the end of 2024. if (index_desc.type == INVERTED_INDEX_NAME && !settings[Setting::allow_experimental_inverted_index]) - throw Exception(ErrorCodes::ILLEGAL_INDEX, "Please use index type 'full_text' instead of 'inverted'"); + throw Exception(ErrorCodes::ILLEGAL_INDEX, "The 'inverted' index type is deprecated. Please use the 'full_text' index type instead"); /// ---- if (index_desc.type == "vector_similarity" && !settings[Setting::allow_experimental_vector_similarity_index]) - throw Exception(ErrorCodes::SUPPORT_IS_DISABLED, "Experimental vector similarity index is disabled. Turn on setting 'allow_experimental_vector_similarity_index'"); + throw Exception(ErrorCodes::SUPPORT_IS_DISABLED, "The experimental vector similarity index feature is disabled. Enable the setting 'allow_experimental_vector_similarity_index' to use it"); properties.indices.push_back(index_desc); } diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index 87e5ae21a03..44363b53540 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -229,7 +229,7 @@ namespace MergeTreeSetting extern const MergeTreeSettingsString storage_policy; extern const MergeTreeSettingsFloat zero_copy_concurrent_part_removal_max_postpone_ratio; extern const MergeTreeSettingsUInt64 zero_copy_concurrent_part_removal_max_split_times; - extern const MergeTreeSettingsBool check_table_structure_completely; + extern const MergeTreeSettingsBool enforce_index_structure_match_on_partition_manipulation; } @@ -7414,8 +7414,9 @@ MergeTreeData & MergeTreeData::checkStructureAndGetMergeTreeData(IStorage & sour throw Exception(ErrorCodes::BAD_ARGUMENTS, "Tables have different primary key"); const auto check_definitions = [this](const auto & my_descriptions, const auto & src_descriptions) { - if (((*getSettings())[MergeTreeSetting::check_table_structure_completely] && my_descriptions.size() != src_descriptions.size()) - || (!(*getSettings())[MergeTreeSetting::check_table_structure_completely] && my_descriptions.size() < src_descriptions.size())) + bool strict_match = (*getSettings())[MergeTreeSetting::enforce_index_structure_match_on_partition_manipulation]; + if ((my_descriptions.size() < src_descriptions.size()) || + (strict_match && my_descriptions.size() != src_descriptions.size())) return false; std::unordered_set my_query_strings; diff --git a/src/Storages/MergeTree/MergeTreeSettings.cpp b/src/Storages/MergeTree/MergeTreeSettings.cpp index b1e1a431ff9..753853ab2f9 100644 --- a/src/Storages/MergeTree/MergeTreeSettings.cpp +++ b/src/Storages/MergeTree/MergeTreeSettings.cpp @@ -97,7 +97,7 @@ namespace ErrorCodes M(String, merge_workload, "", "Name of workload to be used to access resources for merges", 0) \ M(String, mutation_workload, "", "Name of workload to be used to access resources for mutations", 0) \ M(Milliseconds, background_task_preferred_step_execution_time_ms, 50, "Target time to execution of one step of merge or mutation. Can be exceeded if one step takes longer time", 0) \ - M(Bool, check_table_structure_completely, false, "Whether to check table structure completely when manipulate partitions. If true, the source and target tables must have identical definitions including projections and secondary indices. Otherwise, the source table's projections and secondary indices must be a subset of those in the target table.", 0) \ + M(Bool, enforce_index_structure_match_on_partition_manipulation, false, "If this setting is enabled for destination table of a partition manipulation query (`ATTACH/MOVE/REPLACE PARTITION`), the indices and projections must be identical between the source and destination tables. Otherwise, the destination table can have a superset of the source table's indices and projections.", 0) \ M(MergeSelectorAlgorithm, merge_selector_algorithm, MergeSelectorAlgorithm::SIMPLE, "The algorithm to select parts for merges assignment", 0) \ \ /** Inserts settings. */ \ diff --git a/tests/integration/test_attach_with_different_projections_or_indices/configs/config_with_check_table_structure_completely.xml b/tests/integration/test_attach_with_different_projections_or_indices/configs/config_with_check_table_structure_completely.xml index dd9ae5e83d5..06a360847e4 100644 --- a/tests/integration/test_attach_with_different_projections_or_indices/configs/config_with_check_table_structure_completely.xml +++ b/tests/integration/test_attach_with_different_projections_or_indices/configs/config_with_check_table_structure_completely.xml @@ -1,5 +1,5 @@ - true + true diff --git a/tests/integration/test_attach_with_different_projections_or_indices/configs/config_without_check_table_structure_completely.xml b/tests/integration/test_attach_with_different_projections_or_indices/configs/config_without_check_table_structure_completely.xml index 734294478ba..fd78e9d1954 100644 --- a/tests/integration/test_attach_with_different_projections_or_indices/configs/config_without_check_table_structure_completely.xml +++ b/tests/integration/test_attach_with_different_projections_or_indices/configs/config_without_check_table_structure_completely.xml @@ -1,5 +1,5 @@ - false + false \ No newline at end of file diff --git a/tests/integration/test_attach_with_different_projections_or_indices/test.py b/tests/integration/test_attach_with_different_projections_or_indices/test.py index 75ce524b006..2db2dc7a7f4 100644 --- a/tests/integration/test_attach_with_different_projections_or_indices/test.py +++ b/tests/integration/test_attach_with_different_projections_or_indices/test.py @@ -24,7 +24,7 @@ def start_cluster(): # def test_setting_check_table_structure_completely(start_cluster): -# assert node1.query("""select value from system.merge_tree_settings where name='check_table_structure_completely';""") == "0\n" +# assert node1.query("""select value from system.merge_tree_settings where name='enforce_index_structure_match_on_partition_manipulation';""") == "0\n" def test_check_completely_attach_with_different_indices(start_cluster): node1.query( """ From eab454f3b9bac9d82b8f36f019cb923cf52063b9 Mon Sep 17 00:00:00 2001 From: Vladimir Cherkasov Date: Mon, 21 Oct 2024 15:13:58 +0200 Subject: [PATCH 7/8] upd aspell-dict.txt --- utils/check-style/aspell-ignore/en/aspell-dict.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/utils/check-style/aspell-ignore/en/aspell-dict.txt b/utils/check-style/aspell-ignore/en/aspell-dict.txt index 616ad4a800c..3226bf6d6fd 100644 --- a/utils/check-style/aspell-ignore/en/aspell-dict.txt +++ b/utils/check-style/aspell-ignore/en/aspell-dict.txt @@ -1,4 +1,4 @@ -personal_ws-1.1 en 2984 +personal_ws-1.1 en 2985 AArch ACLs ALTERs @@ -2722,6 +2722,7 @@ summapwithoverflow summingmergetree sumwithoverflow superaggregates +superset supertype supremum symlink From b8c70bf059e65771ac39adf4ec3dc5d55cc7b6f9 Mon Sep 17 00:00:00 2001 From: zhangwanyun1 Date: Tue, 19 Nov 2024 11:19:23 +0800 Subject: [PATCH 8/8] Re run pipeline