Adding support for ALTER RENAME COLUMN query to Distributed table engine #10727

vzakaznikov · 2020-05-07T12:56:42Z

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Adding support for ALTER RENAME COLUMN query to Distributed table engine.

Detailed description / Documentation draft:
Adding support for ALTER RENAME COLUMN query to Distributed table engine.

…ine.

alexey-milovidov

Ok. And could you please also add a simple functional test (that will use test_shard_localhost or something similar)?

This is recommended because this functional test will fail more quickly and in more obvious way (than integration test) if RENAME COLUMN suddenly disappear.

vzakaznikov · 2020-05-07T14:27:05Z

Ok, I will add a stateless test as well.

vzakaznikov · 2020-05-08T10:42:18Z

@alesapin, is this exception normal?

2020.05.08 03:31:24.543230 [ 20 ] {} <Error> default.test_rename_with_parallel_merges: void DB::StorageReplicatedMergeTree::queueUpdatingTask(): Code: 236, e.displayText() = DB::Exception: Log pulling is cancelled, Stack trace (when copying this message, always include the lines below):

0. Poco::Exception::Exception(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int) @ 0x1fa68ca9 in /usr/bin/clickhouse
1. DB::Exception::Exception(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int) @ 0xabf5c2e in /usr/bin/clickhouse
2. DB::ReplicatedMergeTreeQueue::pullLogsToQueue(std::__1::shared_ptr<zkutil::ZooKeeper>, std::__1::function<void (Coordination::WatchResponse const&)>) @ 0x19069521 in /usr/bin/clickhouse
3. DB::StorageReplicatedMergeTree::queueUpdatingTask() @ 0x18a324df in /usr/bin/clickhouse
4. DB::BackgroundSchedulePoolTaskInfo::execute() @ 0x175c4487 in /usr/bin/clickhouse
5. DB::BackgroundSchedulePool::threadFunction() @ 0x175c9aa0 in /usr/bin/clickhouse
6. ? @ 0x175cb52b in /usr/bin/clickhouse
7. ThreadPoolImpl<std::__1::thread>::worker(std::__1::__list_iterator<std::__1::thread, void*>) @ 0xac3efda in /usr/bin/clickhouse
8. ? @ 0xac4a138 in /usr/bin/clickhouse
9. start_thread @ 0x9669 in /usr/lib/x86_64-linux-gnu/libpthread-2.30.so
10. clone @ 0x122323 in /usr/lib/x86_64-linux-gnu/libc-2.30.so
 (version 20.5.1.3280)

vzakaznikov · 2020-05-08T10:48:00Z

Also, there is a couple of these exceptions.

2020.05.08 03:32:36.156681 [ 71 ] {a7595df6-dfc0-43ad-b63d-000e5cb0a3a7} <Error> executeQuery: Code: 48, e.displayText() = DB::Exception: There was an error on [node2:9000]: Cannot execute replicated DDL query on leader (version 20.5.1.3280) (from 172.19.0.1:56838) (in query: ALTER TABLE test_rename_distributed_parallel_insert_and_select_replicated ON CLUSTER test_cluster RENAME COLUMN num2 TO foo2 ), Stack trace (when copying this message, always include the lines below):

0. Poco::Exception::Exception(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int) @ 0x1fa68ca9 in /usr/bin/clickhouse
1. DB::Exception::Exception(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int) @ 0xabf5c2e in /usr/bin/clickhouse
2. DB::DDLQueryStatusInputStream::readImpl() @ 0x176189f8 in /usr/bin/clickhouse
3. DB::IBlockInputStream::read() @ 0x171e93ab in /usr/bin/clickhouse
4. DB::AsynchronousBlockInputStream::calculate() @ 0x171de4d5 in /usr/bin/clickhouse
5. ? @ 0x171df274 in /usr/bin/clickhouse
6. ThreadPoolImpl<ThreadFromGlobalPool>::worker(std::__1::__list_iterator<ThreadFromGlobalPool, void*>) @ 0xac45539 in /usr/bin/clickhouse
7. ThreadFromGlobalPool::ThreadFromGlobalPool<void ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::'lambda1'()>(void&&, void ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::'lambda1'()&&...)::'lambda'()::operator()() const @ 0xac4ec1d in /usr/bin/clickhouse
8. ThreadPoolImpl<std::__1::thread>::worker(std::__1::__list_iterator<std::__1::thread, void*>) @ 0xac3efda in /usr/bin/clickhouse
9. ? @ 0xac4a138 in /usr/bin/clickhouse
10. start_thread @ 0x9669 in /usr/lib/x86_64-linux-gnu/libpthread-2.30.so
11. clone @ 0x122323 in /usr/lib/x86_64-linux-gnu/libc-2.30.so

Adding renaming of the time column used inside the TTL expression after insert.

vzakaznikov · 2020-05-08T11:44:15Z

Updated test test_rename_with_parallel_ttl_move shows that renaming column used inside the TTL expression after the data is inserted breaks TTL move. Opened an issue #10747.

alexey-milovidov · 2020-05-09T01:39:31Z

But it still does not.

vzakaznikov · 2020-05-10T22:50:09Z

@alexey-milovidov, I am looking into the failing tests. Any ideas why there are so many fails related to zookeeper (https://clickhouse-test-reports.s3.yandex.net/10727/089371ddfb596a7b052ac709a5c2778e76f349c0/integration_tests_(release)/integration_run.7.txt.out.log)?

vzakaznikov · 2020-05-10T22:51:46Z

But it still does not.

Actually test test_rename_with_parallel_ttl_move now does pass.

alexey-milovidov · 2020-05-10T22:57:41Z

@vzakaznikov It means that one of ZooKeeper nodes refusing our connections, but I don't know why it is happening.

vzakaznikov · 2020-05-10T23:08:41Z

These fails also look very interesting https://clickhouse-test-reports.s3.yandex.net/10727/089371ddfb596a7b052ac709a5c2778e76f349c0/integration_tests_(asan)/integration_run.7.txt.out.log

vzakaznikov · 2020-05-10T23:10:17Z

Out of all the fails I can only see that test_rename_with_parallel_select has a problem and I will fix it. For the others I am not really sure what is going on.

vzakaznikov · 2020-05-11T17:04:36Z

@alesapin, please see a new test https://github.com/ClickHouse/ClickHouse/blob/9780da3e747726086cf705281775af98f38a3e0c/tests/queries/0_stateless/01277_alter_rename_column_constraint_expr.sql which shows that CONSTRAINT expressions are not being updated when one of the columns used inside the expression is renamed.

alesapin · 2020-05-12T13:57:42Z

tests/queries/0_stateless/01274_alter_rename_column_distributed.sql

@@ -0,0 +1,17 @@
+DROP TABLE IF EXISTS visits;


I think these tests are not related to changes. Let's make separate PR.

alesapin · 2020-05-12T13:58:32Z

tests/integration/test_rename_column/test.py

+    finally:
+        drop_distributed_table(node1, table_name)
+
+def test_rename_distributed_parallel_insert_and_select(started_cluster):


I think we need to simplify this test. Which scenario do we check here?

alesapin · 2020-05-12T13:59:23Z

tests/queries/0_stateless/01277_alter_rename_column_constraint_expr.sql

@@ -0,0 +1,43 @@
+DROP TABLE IF EXISTS table_for_rename;


I have stolen this test in my PR #10847 :)

Adding support for ALTER RENAME COLUMN query to Distributed table eng…

90e52e7

…ine.

filimonov added altinity labels May 7, 2020

blinkov added the pr-improvement Pull request with some product improvements label May 7, 2020

Adding tests.

abe2896

alexey-milovidov approved these changes May 7, 2020

View reviewed changes

Vitaliy Zakaznikov added 3 commits May 7, 2020 19:39

Adding stateless test for alter rename column on a distributed table.

6d071c4

Adding more integration tests for alter rename column.

65f139f

Adding configuration files.

eb3c6ca

Updating test_rename_with_parallel_ttl_move to check if parts got moves.

dd5dd5d

Adding renaming of the time column used inside the TTL expression after insert.

Reverting test so that it can pass.

089371d

alexey-milovidov self-assigned this May 9, 2020

alexey-milovidov added the st-waiting-for-fix We are waiting for fixes in this issue or pull request label May 9, 2020

Adding more tests to check renaming of columns.

9780da3

Vitaliy Zakaznikov added 2 commits May 11, 2020 20:09

Fixing test_rename_with_parallel_select.

1efc372

Increasing zookeeper_session_timeout setting to 15 sec.

3505ccc

alesapin self-assigned this May 12, 2020

alesapin requested changes May 12, 2020

View reviewed changes

Update test.py

35a9a20

alesapin mentioned this pull request May 13, 2020

Vzakaznikov add alter rename column to distributed #10887

Merged

alesapin merged commit 352c402 into ClickHouse:master May 16, 2020

qoega added doc-alert and removed st-waiting-for-fix We are waiting for fixes in this issue or pull request labels May 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for ALTER RENAME COLUMN query to Distributed table engine #10727

Adding support for ALTER RENAME COLUMN query to Distributed table engine #10727

vzakaznikov commented May 7, 2020

alexey-milovidov left a comment •

edited

vzakaznikov commented May 7, 2020

vzakaznikov commented May 8, 2020

vzakaznikov commented May 8, 2020

vzakaznikov commented May 8, 2020 •

edited

alexey-milovidov commented May 9, 2020

vzakaznikov commented May 10, 2020

vzakaznikov commented May 10, 2020

alexey-milovidov commented May 10, 2020

vzakaznikov commented May 10, 2020

vzakaznikov commented May 10, 2020

vzakaznikov commented May 11, 2020

alesapin May 12, 2020

alesapin May 12, 2020

alesapin May 12, 2020

Adding support for ALTER RENAME COLUMN query to Distributed table engine #10727

Adding support for ALTER RENAME COLUMN query to Distributed table engine #10727

Conversation

vzakaznikov commented May 7, 2020

alexey-milovidov left a comment • edited

Choose a reason for hiding this comment

vzakaznikov commented May 7, 2020

vzakaznikov commented May 8, 2020

vzakaznikov commented May 8, 2020

vzakaznikov commented May 8, 2020 • edited

alexey-milovidov commented May 9, 2020

vzakaznikov commented May 10, 2020

vzakaznikov commented May 10, 2020

alexey-milovidov commented May 10, 2020

vzakaznikov commented May 10, 2020

vzakaznikov commented May 10, 2020

vzakaznikov commented May 11, 2020

alesapin May 12, 2020

Choose a reason for hiding this comment

alesapin May 12, 2020

Choose a reason for hiding this comment

alesapin May 12, 2020

Choose a reason for hiding this comment

alexey-milovidov left a comment •

edited

vzakaznikov commented May 8, 2020 •

edited