Skip to content

add sync to the test 03352_concurrent_rename_alter.sh#83830

Merged
CheSema merged 2 commits intomasterfrom
chesema-fix-03352
Jul 17, 2025
Merged

add sync to the test 03352_concurrent_rename_alter.sh#83830
CheSema merged 2 commits intomasterfrom
chesema-fix-03352

Conversation

@CheSema
Copy link
Copy Markdown
Member

@CheSema CheSema commented Jul 16, 2025

The test is bad actually.

https://fiddle.clickhouse.com/9973867d-2aa6-4061-b4c0-8761e5579b84
It exploit a loophole that allows to alter column type Tuple in a way which extends number of elements. Explicit MODIFY COLUMN would throw an error here.

This 2 commands
ALTER TABLE t_rename_alter RENAME COLUMN arr TO arr_tmp;
ALTER TABLE t_rename_alter RENAME COLUMN arr_v2 TO arr;
are converted to 2 alters and 2 mutations
alters:
DROP COLUMN arr
ADD COLUMN arr Array(Tuple(DateTime, UInt64, String, String, String)) (5 elements)
mutations:
DROP COLUMN arr
MOVE arr_v2 to arr (5 elements)

Alter and mutations could be delayed. They are not synchonized.

When we read from the table we could see metadata with column type arr as Array(Tuple(DateTime, UInt64, String, String, String)) but have the files on the disk where column arr as Array(Tuple(DateTime, UInt64, String, String)).
It tries to apply CAST, but it throws the error.

I have added a synchronization. Now all alters and mutations are done before the reading.
When we apply mutations we apply DROP and MOVE to the files therefore we do not threat old column arr with 4 elements in tuple as a new column with 5 elements.

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

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

...

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Jul 16, 2025

Workflow [PR], commit [4a6959a]

Summary:

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Jul 16, 2025
@Michicosun Michicosun self-assigned this Jul 16, 2025
@tavplubix
Copy link
Copy Markdown
Member

Do we have an issue about the root cause of the problem?

@CheSema
Copy link
Copy Markdown
Member Author

CheSema commented Jul 17, 2025

Do we have an issue about the root cause of the problem?

I need to create it. It is not written.

@CheSema CheSema added this pull request to the merge queue Jul 17, 2025
Merged via the queue into master with commit 8ccd1e6 Jul 17, 2025
121 checks passed
@CheSema CheSema deleted the chesema-fix-03352 branch July 17, 2025 16:27
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jul 17, 2025
@CheSema
Copy link
Copy Markdown
Member Author

CheSema commented Jul 23, 2025

Do we have an issue about the root cause of the problem?

#84295

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants