Skip to content

Solve direct read issue with low cardinality text columns#87994

Merged
Ergus merged 14 commits intoClickHouse:masterfrom
Ergus:Fix_87887
Oct 21, 2025
Merged

Solve direct read issue with low cardinality text columns#87994
Ergus merged 14 commits intoClickHouse:masterfrom
Ergus:Fix_87887

Conversation

@Ergus
Copy link
Copy Markdown
Member

@Ergus Ergus commented Oct 1, 2025

When the input column for is low cardinality, it seems like hasToken and similar functions are reported also as low cardinality result type in the dag.

This is strictly true, but for direct read the column will contain only 0 and 1. So it is simpler and easier just set it as UInt8 explicitly in all the cases.

Maybe one day we will have some boolean support in columns (storing internally some bitmap) and we can use that specialization.

Resolves: #87887
Resolves: #88119

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

When the input column is low cardinality, it seems like hasToken and similar functions are reported also as low cardinality result type in
the dag.

This is strictly true, but for direct read the column will contain only 0 and 1. So it is simpler and easier just set it as UInt8
explicitly in all the cases.

Maybe one day we will have some boolean support in columns (storing internally some bitmap) and we can use that specialization.
@Ergus Ergus requested a review from rschu1ze October 1, 2025 18:07
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Oct 1, 2025

Workflow [PR], commit [4ad1a78]

Summary:

job_name test_name status info comment
Stateless tests (amd_tsan, s3 storage, sequential, 1/2) error
Stateless tests (arm_binary, sequential) error

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Oct 1, 2025
@rschu1ze rschu1ze self-assigned this Oct 1, 2025
@rschu1ze

This comment was marked as resolved.

@rschu1ze rschu1ze enabled auto-merge October 5, 2025 20:08
@rschu1ze rschu1ze added this pull request to the merge queue Oct 5, 2025
@rschu1ze rschu1ze removed this pull request from the merge queue due to a manual request Oct 5, 2025
@rschu1ze

This comment was marked as resolved.

@rschu1ze

This comment was marked as resolved.

@Ergus

This comment was marked as resolved.

@Ergus Ergus enabled auto-merge October 15, 2025 12:00
@Ergus Ergus added this pull request to the merge queue Oct 21, 2025
Merged via the queue into ClickHouse:master with commit db4efe2 Oct 21, 2025
230 of 361 checks passed
@Ergus Ergus deleted the Fix_87887 branch October 21, 2025 09:53
@robot-clickhouse robot-clickhouse added the pr-synced-to-cloud The PR is synced to the cloud repo label Oct 21, 2025
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

4 participants