Skip to content

Move 03631_hive_columns_not_in_format_header.sql to the proper file location#87621

Merged
kssenii merged 4 commits intoClickHouse:masterfrom
arthurpassos:do_not_put_hive_columns_in_format_header
Sep 26, 2025
Merged

Move 03631_hive_columns_not_in_format_header.sql to the proper file location#87621
kssenii merged 4 commits intoClickHouse:masterfrom
arthurpassos:do_not_put_hive_columns_in_format_header

Conversation

@arthurpassos
Copy link
Contributor

@arthurpassos arthurpassos commented Sep 25, 2025

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):

Move test files to their proper locationn

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@arthurpassos
Copy link
Contributor Author

arthurpassos commented Sep 25, 2025

@al13n321 sorry for that :D

@arthurpassos
Copy link
Contributor Author

arthurpassos commented Sep 25, 2025 •
edited
Loading

#87528

@Blargian Blargian added the can be tested Allows running workflows for external contributors label Sep 25, 2025
@clickhouse-gh
Copy link
Contributor

clickhouse-gh bot commented Sep 25, 2025 •
edited
Loading

Workflow [PR], commit [2721b91]

Summary: ❌

job_name test_name status info comment
Stateless tests (amd_binary, old analyzer, s3 storage, DatabaseReplicated, sequential) failure
01079_parallel_alter_detach_table_zookeeper FAIL
Stateless tests (amd_binary, ParallelReplicas, s3 storage, parallel) failure
03274_squashing_transform_sparse_bug FAIL
Exception in test runner FAIL
Killed by signal (in clickhouse-server.log or clickhouse-server.err.log) FAIL
Fatal messages (in clickhouse-server.log or clickhouse-server.err.log) FAIL

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Sep 25, 2025
@kssenii kssenii enabled auto-merge September 25, 2025 13:38
@kssenii kssenii disabled auto-merge September 25, 2025 13:38
@kssenii
Copy link
Member

kssenii commented Sep 25, 2025

@arthurpassos please check https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=87621&sha=e7712b27b4c984edf5f58ddbc57e60fa1556c9a4&name_0=PR&name_1=Stateless%20tests%20%28amd_binary%2C%20ParallelReplicas%2C%20s3%20storage%2C%20sequential%29

@arthurpassos
Copy link
Contributor Author

arthurpassos commented Sep 25, 2025

@arthurpassos please check https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=87621&sha=e7712b27b4c984edf5f58ddbc57e60fa1556c9a4&name_0=PR&name_1=Stateless%20tests%20%28amd_binary%2C%20ParallelReplicas%2C%20s3%20storage%2C%20sequential%29

This is somewhat related to #87586.

Historically, hive columns have been parsed into LC whenever possible. Even before my very first contribution to hive: https://github.com/ClickHouse/ClickHouse/pull/76802/files#diff-fcfbcff9317d0ec5f3ff12be38831fa59608735ad53e1a1d0c43ed8f921abe98L227.

Instead of simply inferred_type->canBeInsideLowCardinality() { wrapWithLc()}, we can maybe put it in LC only if it is parsed as string type.

Is it ok if I put this logic in this PR?

kssenii
kssenii approved these changes Sep 25, 2025
View reviewed changes
@kssenii kssenii self-assigned this Sep 25, 2025
@kssenii kssenii enabled auto-merge September 25, 2025 14:47
auto-merge was automatically disabled September 25, 2025 15:51

Head branch was pushed to by a user without write access

@arthurpassos arthurpassos mentioned this pull request Sep 25, 2025
Closed
@arthurpassos
Copy link
Contributor Author

arthurpassos commented Sep 26, 2025 •
edited
Loading

@kssenii you might need to enable auto-merge again, I had to push another small change

@kssenii kssenii enabled auto-merge September 26, 2025 12:22
@kssenii kssenii added this pull request to the merge queue Sep 26, 2025
Merged via the queue into ClickHouse:master with commit 99e90e2 Sep 26, 2025
121 of 124 checks passed
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Sep 26, 2025
arthurpassos pushed a commit to Altinity/ClickHouse that referenced this pull request Sep 26, 2025
@kssenii @arthurpassos
Merge pull request ClickHouse#87621 from arthurpassos/do_not_put_hive…
f5182ca 
…_columns_in_format_header

Move 03631_hive_columns_not_in_format_header.sql to the proper file location
@arthurpassos arthurpassos mentioned this pull request Sep 26, 2025
Merged
13 tasks
Enmk added a commit to Altinity/ClickHouse that referenced this pull request Sep 30, 2025
@Enmk
Merge pull request #1037 from Altinity/backports/25.6.5/87515_and_876…
1ee7da1 
…21_do_not_put_hive_partition_columns_in_format_header

Antalya 25.6.5 - Backport of ClickHouse#87515 and ClickHouse#87621 - Never not put hive columns in format_header / Do not read hive columns as LC by default
arthurpassos pushed a commit to Altinity/ClickHouse that referenced this pull request Dec 16, 2025
@kssenii @arthurpassos
Merge pull request ClickHouse#87621 from arthurpassos/do_not_put_hive…
be0ea1b 
…_columns_in_format_header

Move 03631_hive_columns_not_in_format_header.sql to the proper file location
@arthurpassos arthurpassos mentioned this pull request Dec 16, 2025
Merged
25 tasks
zvonand added a commit to Altinity/ClickHouse that referenced this pull request Dec 17, 2025
@zvonand
Merge pull request #1231 from Altinity/backports/25.8.12/87621_move_t…
66a6bf5 
…est_file_and_fix_lc

25.8.12 Backport of ClickHouse#87621: Move 03631_hive_columns_not_in_format_header.sql to the proper file location
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Blargian Blargian Blargian approved these changes

@kssenii kssenii kssenii approved these changes

Assignees

@Blargian Blargian

@kssenii kssenii

Labels

can be tested Allows running workflows for external contributors 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@arthurpassos @kssenii @Blargian @robot-ch-test-poll2 @svb-alt