Support rename table without keyword TABLE #55373

lingtaolf · 2023-10-09T09:10:29Z

Original issue : #55017

Changelog category (leave one):

Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Support rename table without keyword TABLE like RENAME db.t1 to db.t2

Documentation entry for user-facing changes

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

robot-clickhouse-ci-1 · 2023-10-09T10:49:42Z

This is an automated comment for commit e646fb7 with description of existing statuses. It's updated for the latest CI running

❌ Click here to open a full report in a separate page

Successful checks

Check name	Description	Status
AST fuzzer	Runs randomly generated queries to catch program errors. The build type is optionally given in parenthesis. If it fails, ask a maintainer for help	✅ success
CI running	A meta-check that indicates the running CI. Normally, it's in success or pending state. The failed status indicates some problems with the PR	✅ success
ClickHouse build check	Builds ClickHouse in various configurations for use in further steps. You have to fix the builds that fail. Build logs often has enough information to fix the error, but you might have to reproduce the failure locally. The cmake options can be found in the build log, grepping for cmake. Use these options and follow the general build process	✅ success
Compatibility check	Checks that clickhouse binary runs on distributions with old libc versions. If it fails, ask a maintainer for help	✅ success
Docker image for servers	The check to build and optionally push the mentioned image to docker hub	✅ success
Docs Check	Builds and tests the documentation	✅ success
Fast test	Normally this is the first check that is ran for a PR. It builds ClickHouse and runs most of stateless functional tests, omitting some. If it fails, further checks are not started until it is fixed. Look at the report to see which tests fail, then reproduce the failure locally as described here	✅ success
Flaky tests	Checks if new added or modified tests are flaky by running them repeatedly, in parallel, with more randomization. Functional tests are run 100 times with address sanitizer, and additional randomization of thread scheduling. Integrational tests are run up to 10 times. If at least once a new test has failed, or was too long, this check will be red. We don't allow flaky tests, read the doc	✅ success
Install packages	Checks that the built packages are installable in a clear environment	✅ success
Integration tests	The integration tests report. In parenthesis the package type is given, and in square brackets are the optional part/total tests	✅ success
Mergeable Check	Checks if all other necessary checks are successful	✅ success
Performance Comparison	Measure changes in query performance. The performance test report is described in detail here. In square brackets are the optional part/total tests	✅ success
Push to Dockerhub	The check for building and pushing the CI related docker images to docker hub	✅ success
SQLTest	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
SQLancer	Fuzzing tests that detect logical bugs with SQLancer tool	✅ success
Sqllogic	Run clickhouse on the sqllogic test set against sqlite and checks that all statements are passed	✅ success
Stateful tests	Runs stateful functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	✅ success
Stress test	Runs stateless functional tests concurrently from several clients to detect concurrency-related errors	✅ success
Style Check	Runs a set of checks to keep the code style clean. If some of tests failed, see the related log from the report	✅ success
Unit tests	Runs the unit tests for different release types	✅ success
Upgrade check	Runs stress tests on server version from last release and then tries to upgrade it to the version from the PR. It checks if the new server can successfully startup without any errors, crashes or sanitizer asserts	✅ success

Check name	Description	Status
Stateless tests	Runs stateless functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	❌ failure

tests/queries/0_stateless/02891_rename_table_without_keyword.sql

tests/queries/0_stateless/02891_rename_table_without_keyword.reference

antaljanosbenjamin · 2023-10-10T10:51:45Z

So it seems it also renames dictionaries. I am not sure if that is good or not. Let me check.

lingtaolf · 2023-10-10T11:18:02Z

So it seems it also renames dictionaries. I am not sure if that is good or not. Let me check.

I've tried rename dictionary with statement rename table dictionary_name to dictionary_name_2 on version 22.3.3.44 and I found this sql can be exeuted without exception. So It will not throw an exception when you rename dictionary using sql rename dictionary_name to dictionary_name_2 in this PR. Is that ok?

lingtaolf · 2023-10-10T12:01:54Z

So it seems it also renames dictionaries. I am not sure if that is good or not. Let me check.

I've tried rename dictionary with statement rename table dictionary_name to dictionary_name_2 on version 22.3.3.44 and I found this sql can be exeuted without exception. So It will not throw an exception when you rename dictionary using sql rename dictionary_name to dictionary_name_2 in this PR. Is that ok?

Master branch can also rename dictionary via using RENAME TABLE statement.

antaljanosbenjamin

Last request from me: could you please change the docs to mention that RENAME TABLE now should also work just as RENAME in docs/en/sql-reference/statements/rename.md? Don't forget to update the syntax of the RENAME command on the top of the page: instead of RENAME DATABASE|TABLE|DICTIONARY it should be RENAME [DATABASE|TABLE|DICTIONARY] I think.

tests/queries/0_stateless/02891_rename_table_without_keyword.reference

tests/queries/0_stateless/02891_rename_table_without_keyword.sql

lingtaolf · 2023-10-11T09:29:26Z

The failed test does not appear to be related to this change, should I ignore them ?
@antaljanosbenjamin

docs/en/sql-reference/statements/rename.md

antaljanosbenjamin · 2023-10-11T12:05:46Z

tests/queries/0_stateless/02891_rename_table_without_keyword.sh

+$CLICKHOUSE_CLIENT --param_db="$db_name" --multiquery \
+    --query="CREATE DATABASE {db:Identifier};
+             CREATE TABLE IF NOT EXISTS {db:Identifier}.r1 (name String) Engine=Memory();
+             SHOW TABLES FROM {db:Identifier}"


I think what @tavplubix was referring to (thanks for catching this!) is to use {CLICKHOUSE_DATABASE:Identifier} as a param for database name. It is appended by clickhouse-test (see here), so you don't need to manually generate anything.

Conveniently we also append CLICKHOUSE_DATABASE_1 identifier, so you can use that to rename the database to, so the last query can be written like

RENAME {CLICKHOUSE_DATABASE:Identifier} TO {CLICKHOUSE_DATABASE_1:Identifier}; -- { serverError UNKNOWN_TABLE }

As a result as we don't need anything generated, we can revert back the tests to the simpler sql file instead of running a shell script in my opinion.

Yes, it doesn't make sense for .sh tests because in .sh tests we have $CLICKHOUSE_DATABASE env var

Ok, I undertand what you mean and I've update the test script.

I've tried use {CLICKHOUSE_DATABASE:Identifier}, but there were some strange problems. So I think using the current test script should also be ok.

I pushed the test using SQL files. Usually we prefer to use SQL files. The reasons I am aware of:

They are simpler than the script based tests

The queries in SQL scripts are picked up by the fuzzer and used to fuzz ClickHouse.

I hope you don't mind that I pushed a commit, but I think this make the tests better and it was easier for me. On the other hand, if you have any questions regarding to the test file, I am more than happy to answer it. If you want to understand why it wasn't working for you, I am also happy to help you, just show the errors you faced.

thx for your helping.

antaljanosbenjamin · 2023-10-16T11:20:06Z

I think the failed test 00385_storage_file_and_clickhouse is not related, but let me make sure that we don't merge something that breaks master.

antaljanosbenjamin · 2023-10-16T11:34:41Z

The failure might be connected to #55035

support rename table without keyword TABLE

8f82d64

lingtaolf force-pushed the enhancement/rename_without_keyword branch from 60fe7c5 to 8f82d64 Compare October 9, 2023 10:10

ucasfl added the can be tested Allows running workflows for external contributors label Oct 9, 2023

robot-clickhouse-ci-1 added the pr-improvement Pull request with some product improvements label Oct 9, 2023

lingtaolf force-pushed the enhancement/rename_without_keyword branch from 413c190 to 2116377 Compare October 9, 2023 11:59

antaljanosbenjamin self-assigned this Oct 9, 2023

antaljanosbenjamin reviewed Oct 9, 2023

View reviewed changes

tests/queries/0_stateless/02891_rename_table_without_keyword.sql Outdated Show resolved Hide resolved

lingtaolf force-pushed the enhancement/rename_without_keyword branch 2 times, most recently from 408f570 to 6af3241 Compare October 10, 2023 02:54

antaljanosbenjamin requested changes Oct 10, 2023

View reviewed changes

tests/queries/0_stateless/02891_rename_table_without_keyword.sql Outdated Show resolved Hide resolved

tests/queries/0_stateless/02891_rename_table_without_keyword.reference Outdated Show resolved Hide resolved

antaljanosbenjamin requested changes Oct 10, 2023

View reviewed changes

tests/queries/0_stateless/02891_rename_table_without_keyword.reference Show resolved Hide resolved

antaljanosbenjamin linked an issue Oct 10, 2023 that may be closed by this pull request

RENAME without TABLE keyword #55017

Closed

tavplubix reviewed Oct 10, 2023

View reviewed changes

tests/queries/0_stateless/02891_rename_table_without_keyword.sql Outdated Show resolved Hide resolved

lingtaolf force-pushed the enhancement/rename_without_keyword branch from 82948f2 to 51a5703 Compare October 11, 2023 06:12

antaljanosbenjamin requested changes Oct 11, 2023

View reviewed changes

lingtaolf force-pushed the enhancement/rename_without_keyword branch from 1192180 to 1886be8 Compare October 12, 2023 06:20

modify style

4499dd3

lingtaolf force-pushed the enhancement/rename_without_keyword branch from 1886be8 to 4499dd3 Compare October 12, 2023 06:23

modify doc

89d123d

lingtaolf force-pushed the enhancement/rename_without_keyword branch from 84fc959 to 89d123d Compare October 12, 2023 06:25

lingtaolf and others added 3 commits October 12, 2023 14:25

Delete tests/queries/0_stateless/02891_rename_table_without_keyword.sql

ca6975a

Use SQL test

2fd2352

Remove executable flag

e646fb7

antaljanosbenjamin mentioned this pull request Oct 16, 2023

Flaky 00385_storage_file_and_clickhouse-local_app_long #55674

Closed

antaljanosbenjamin approved these changes Oct 16, 2023

View reviewed changes

antaljanosbenjamin merged commit eeff98f into ClickHouse:master Oct 16, 2023
285 of 286 checks passed

vdimir mentioned this pull request Nov 2, 2023

Fix link to failed check report in status commit #56243

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support rename table without keyword TABLE #55373

Support rename table without keyword TABLE #55373

lingtaolf commented Oct 9, 2023

robot-clickhouse-ci-1 commented Oct 9, 2023 •

edited by robot-ch-test-poll

antaljanosbenjamin commented Oct 10, 2023

lingtaolf commented Oct 10, 2023 •

edited

lingtaolf commented Oct 10, 2023

antaljanosbenjamin left a comment

lingtaolf commented Oct 11, 2023

antaljanosbenjamin Oct 11, 2023

tavplubix Oct 11, 2023

lingtaolf Oct 11, 2023

lingtaolf Oct 12, 2023

antaljanosbenjamin Oct 12, 2023

lingtaolf Oct 14, 2023

antaljanosbenjamin commented Oct 16, 2023

antaljanosbenjamin commented Oct 16, 2023

Support rename table without keyword TABLE #55373

Support rename table without keyword TABLE #55373

Conversation

lingtaolf commented Oct 9, 2023

Changelog category (leave one):

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Documentation entry for user-facing changes

robot-clickhouse-ci-1 commented Oct 9, 2023 • edited by robot-ch-test-poll

antaljanosbenjamin commented Oct 10, 2023

lingtaolf commented Oct 10, 2023 • edited

lingtaolf commented Oct 10, 2023

antaljanosbenjamin left a comment

Choose a reason for hiding this comment

lingtaolf commented Oct 11, 2023

antaljanosbenjamin Oct 11, 2023

Choose a reason for hiding this comment

tavplubix Oct 11, 2023

Choose a reason for hiding this comment

lingtaolf Oct 11, 2023

Choose a reason for hiding this comment

lingtaolf Oct 12, 2023

Choose a reason for hiding this comment

antaljanosbenjamin Oct 12, 2023

Choose a reason for hiding this comment

lingtaolf Oct 14, 2023

Choose a reason for hiding this comment

antaljanosbenjamin commented Oct 16, 2023

antaljanosbenjamin commented Oct 16, 2023

robot-clickhouse-ci-1 commented Oct 9, 2023 •

edited by robot-ch-test-poll

lingtaolf commented Oct 10, 2023 •

edited