ClickHouse icon indicating copy to clipboard operation
ClickHouse copied to clipboard

Fix IN with Iceberg table

Open ianton-ru opened this issue 1 week ago • 4 comments

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 to CHANGELOG.md):

Fix IN with Iceberg table

Documentation entry for user-facing changes

What is going on. Query like 'SELECT x FROM iceberg.table WHERE y IN (SELECT z FROM local.table)' During query planning planner created IcebergIterator. IcebergIterator takes filters (filter_dag) to make partition pruning, min/max pruning, etc. And IcebergIterator in constructor creates a background thread to create list of objects in parallel with other work. https://github.com/Altinity/ClickHouse/blob/antalya-25.8/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergIterator.cpp#L280 But filters are not ready yet, this separate thread founds that need to complete build and tries to do it. (SingleThreadIcebergKeysIterator::next -> ManifestFilesPruner::ManifestFilesPruner -> KeyCondition::KeyCondition -> KeyCondition::extractAtomFromTree -> FutureSetFromSubquery::buildOrderedSetInplace) Meanwhile main thread continues to build plan, and in some moment also try to complete it. (QueryPlanOptimizations::addStepsToBuildSets -> DelayedCreatingSetsStep::makePlansForSets -> FutureSetFromSubquery::build) When both build runs in the same moment, we get race condition. This PR adds mutex to prevent build from different threads.

More correct ways to fix it is to start IcebergIterator thread when plan is fully built, but planner blows my mind. so I made this simple workaround.

CI/CD Options

Exclude tests:

  • [ ] Fast test
  • [ ] Integration Tests
  • [ ] Stateless tests
  • [ ] Stateful tests
  • [ ] Performance tests
  • [ ] All with ASAN
  • [ ] All with TSAN
  • [ ] All with MSAN
  • [ ] All with UBSAN
  • [x] All with Coverage
  • [ ] All with Aarch64
  • [x] All Regression
  • [ ] Disable CI Cache

Regression jobs to run:

  • [ ] Fast suites (mostly <1h)
  • [ ] Aggregate Functions (2h)
  • [ ] Alter (1.5h)
  • [ ] Benchmark (30m)
  • [ ] ClickHouse Keeper (1h)
  • [ ] Iceberg (2h)
  • [ ] LDAP (1h)
  • [ ] Parquet (1.5h)
  • [ ] RBAC (1.5h)
  • [ ] SSL Server (1h)
  • [ ] S3 (2h)
  • [ ] Tiered Storage (2h)

ianton-ru avatar Nov 25 '25 19:11 ianton-ru

Workflow [PR], commit [02ebff1c]

github-actions[bot] avatar Nov 25 '25 19:11 github-actions[bot]

@codex review

mkmkme avatar Nov 26 '25 11:11 mkmkme

Codex Review: Didn't find any major issues. Keep them coming!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Failed tests looks unrelated to this PR

ianton-ru avatar Nov 26 '25 16:11 ianton-ru