Skip to content

Fix reading from text index with query condition cache#88660

Merged
CurtizJ merged 4 commits intoClickHouse:masterfrom
CurtizJ:fix-text-index-condition-cache
Oct 17, 2025
Merged

Fix reading from text index with query condition cache#88660
CurtizJ merged 4 commits intoClickHouse:masterfrom
CurtizJ:fix-text-index-condition-cache

Conversation

@CurtizJ
Copy link
Copy Markdown
Member

@CurtizJ CurtizJ commented Oct 16, 2025

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

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

Fixed reading from the text index and enabled query condition cache (with enabled settings use_skip_indexes_on_data_read and use_query_condition_cache)

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Oct 16, 2025 •
edited
Loading

Workflow [PR], commit [ae4cbc6]

Summary: ❌

job_name test_name status info comment
Integration tests (amd_asan, old analyzer, 4/6) failure
test_scheduler_cpu_preemptive/test.py::test_downscaling[cpu-slot-preemption-timeout-1ms] FAIL cidb
test_backup_restore_on_cluster/test_slow_rmt.py::test_replicated_database_async FAIL cidb
test_replicated_merge_tree_compatibility/test.py::test_replicated_merge_tree_defaults_compatibility FAIL cidb
test_backup_restore_on_cluster/test_huge_concurrent_restore.py::test_backup_restore_huge_cluster FAIL cidb
OOM in dmesg FAIL cidb
Stress test (amd_ubsan) failure
Server died FAIL cidb
Hung check failed, possible deadlock found (see hung_check.log) FAIL cidb
Killed by signal (in clickhouse-server.log) FAIL cidb
Fatal message in clickhouse-server.log (see fatal_messages.txt) FAIL cidb
Killed by signal (output files) FAIL cidb
Found signal in gdb.log FAIL cidb

@clickhouse-gh clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Oct 16, 2025
@CurtizJ CurtizJ force-pushed the fix-text-index-condition-cache branch from 765b963 to 1ae9201 Compare October 16, 2025 12:49
@CurtizJ CurtizJ force-pushed the fix-text-index-condition-cache branch from 1ae9201 to 4479f15 Compare October 16, 2025 12:53
@pamarcos pamarcos self-assigned this Oct 16, 2025
@pamarcos pamarcos self-requested a review October 16, 2025 13:08
@pamarcos
Copy link
Copy Markdown
Member

pamarcos commented Oct 16, 2025

Code-wise LGTM.

  • Is there any bug issue created that this one is addressing, or how did you figure out there was a bug?
  • Does it need to be backported to some other version?

Ergus
Ergus approved these changes Oct 16, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@Ergus Ergus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that using a hash could make debugging a bit harder, but I understand that otherwise name size will be too big.

LGTM

@rschu1ze rschu1ze mentioned this pull request Oct 16, 2025
Merged
@CurtizJ
Copy link
Copy Markdown
Member Author

CurtizJ commented Oct 16, 2025

Is there any bug issue created that this one is addressing, or how did you figure out there was a bug?

It was found in #88504.

Does it need to be backported to some other version?

Yes, we need to backport it to 25.9.

@CurtizJ CurtizJ added this pull request to the merge queue Oct 17, 2025
Merged via the queue into ClickHouse:master with commit b3ec031 Oct 17, 2025
120 of 123 checks passed
@CurtizJ CurtizJ deleted the fix-text-index-condition-cache branch October 17, 2025 01:38
@robot-clickhouse robot-clickhouse added the pr-synced-to-cloud The PR is synced to the cloud repo label Oct 17, 2025
@robot-ch-test-poll robot-ch-test-poll mentioned this pull request Oct 17, 2025
Closed
@shankar-iyer shankar-iyer mentioned this pull request Oct 17, 2025
Merged
@Felixoid Felixoid mentioned this pull request Dec 16, 2025
Merged
1 task
@den-crane den-crane mentioned this pull request Jan 9, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rschu1ze rschu1ze rschu1ze left review comments

@pamarcos pamarcos pamarcos approved these changes

@Ergus Ergus Ergus approved these changes

Assignees

@pamarcos pamarcos

Labels

pr-bugfix Pull request with bugfix, not backported by default 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

@CurtizJ @pamarcos @Ergus @rschu1ze @robot-clickhouse