Skip to content

Fix skip indexes on data read with patch parts#88702

Closed
CurtizJ wants to merge 2 commits intoClickHouse:masterfrom
CurtizJ:skip-indexes-patches
Closed

Fix skip indexes on data read with patch parts#88702
CurtizJ wants to merge 2 commits intoClickHouse:masterfrom
CurtizJ:skip-indexes-patches

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 usage of skip indexes on data read (with enabled setting use_skip_indexes_on_data_read) and existing patch parts created by lightweight updates.

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Oct 16, 2025

Workflow [PR], commit [26f6a4f]

Summary:

job_name test_name status info comment
Stateless tests (amd_binary, old analyzer, s3 storage, DatabaseReplicated, parallel) failure
00705_drop_create_merge_tree FAIL cidb
02841_check_table_progress FAIL cidb
02567_native_type_conversions FAIL cidb
03093_reading_bug_with_parallel_replicas FAIL cidb
02845_parquet_odd_decimals FAIL cidb
01475_read_subcolumns_3 FAIL cidb
01516_drop_table_stress_long FAIL cidb
03100_lwu_29_concurrent_merges FAIL cidb
02372_analyzer_join FAIL cidb
01825_new_type_json_in_other_types FAIL cidb
164 more test cases not shown
Stateless tests (amd_binary, old analyzer, s3 storage, DatabaseReplicated, sequential) failure
03100_lwu_21_on_fly_mutations FAIL cidb
03100_lwu_25_update_alters FAIL cidb
03096_http_interface_role_query_param FAIL cidb
03047_on_fly_mutations_alters FAIL cidb
03047_on_fly_mutations_basics_rmt FAIL cidb
03047_on_fly_mutations_basics FAIL cidb
03047_on_fly_mutations_multiple_updates_rmt FAIL cidb
03047_on_fly_mutations_multiple_updates FAIL cidb
03033_index_definition_sql_udf_bug FAIL cidb
03031_read_in_order_optimization_with_virtual_row_special FAIL cidb
14 more test cases not shown
Bugfix validation (functional tests) failure
Integration tests (amd_binary, 2/5) failure
test_merge_tree_s3/test.py::test_merge_canceled_by_s3_errors[node-broken_s3] FAIL cidb

@clickhouse-gh clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Oct 16, 2025
@rschu1ze rschu1ze assigned rschu1ze and unassigned rschu1ze Oct 17, 2025
@rschu1ze rschu1ze added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Oct 21, 2025
alexey-milovidov added a commit that referenced this pull request Mar 16, 2026
Continuation of #88702 - rebased
on current master and fixed tests.

The changes:
- Extract `canUseIndex`/`canUseMergedIndex` as static methods on
  `MergeTreeDataSelectExecutor` so they can be reused in multiple places.
- Add `canUseIndex` checks in `MergeTreeSkipIndexReader::read` to skip
  indexes that depend on columns being updated on the fly by lightweight
  updates.
- Add `canUseIndex` check in `collectTextIndexReadInfos` to prevent
  direct reading from text index when the indexed column is updated.
- Fix `MergeTreeReadersChain` to only read and apply patches at the step
  that actually has columns needing patches, not always at step 0.
- Fix `addPatchPartsColumns` to find the correct first step with patches
  instead of assuming it is always the first prewhere step.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@alexey-milovidov
Copy link
Copy Markdown
Member

#99543

@alexey-milovidov alexey-milovidov self-assigned this Mar 16, 2026
alexey-milovidov added a commit that referenced this pull request Mar 26, 2026
Continuation of #88702 - rebased
on current master and fixed tests.

The changes:
- Extract `canUseIndex`/`canUseMergedIndex` as static methods on
  `MergeTreeDataSelectExecutor` so they can be reused in multiple places.
- Add `canUseIndex` checks in `MergeTreeSkipIndexReader::read` to skip
  indexes that depend on columns being updated on the fly by lightweight
  updates.
- Add `canUseIndex` check in `collectTextIndexReadInfos` to prevent
  direct reading from text index when the indexed column is updated.
- Fix `MergeTreeReadersChain` to only read and apply patches at the step
  that actually has columns needing patches, not always at step 0.
- Fix `addPatchPartsColumns` to find the correct first step with patches
  instead of assuming it is always the first prewhere step.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default pr-must-backport Pull request should be backported intentionally. Use this label with great care!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants