ClickHouse
ClickHouse copied to clipboard
Fix IN with Iceberg table
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)
Workflow [PR], commit [02ebff1c]
@codex review
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