fix(fuse): isolate partition and cluster layouts - #20221
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6ea3e0da0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
396bffa to
d6ea3e0
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6ea3e0da0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
d6ea3e0 to
50895ad
Compare
There was a problem hiding this comment.
I found two issues that need to be addressed:
-
[P1] The full workspace test suite does not compile. After adding
BlockMeta.partition_stats, theBlockMetastruct literal atsrc/query/storages/common/cache/src/manager.rs:1294was not updated with the new field. Thelinux / check,linux / test_unit, andmac_checkjobs all fail with E0063. I can also reproduce it locally withcargo test -p databend-storages-common-cache --lib --no-run. -
[P2] Clustering introspection can still reuse legacy physical cluster statistics for partitioned tables. Historical
cluster_stats.min/maxvectors contain(partition keys..., cluster keys...), whileclustering_informationandclustering_statisticsnow prepare expressions for only the logical cluster key and still pass the samecluster_key_idtoget_min_max_stats. Since that function returns the cached vectors whenever the key ID matches without validating their dimensions, clustering information may be displayed or calculated using the partition prefix. Please recompute fromcol_statswhen the cached statistics dimensions do not match the logical expression count, or otherwise explicitly detect the legacy layout.
The focused Fuse test suite passes (cargo test -p databend-common-storages-fuse --lib: 103 tests), but it does not cover these two issues.
50895ad to
fe00398
Compare
This feature has not been deployed in the actual production environment, so I do not believe compatibility is required. |
fe00398 to
a805179
Compare
a805179 to
37802fd
Compare
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
PARTITION BYexpressions toCLUSTER BYkeysTransformPartitionByand only split blocks at adjacent partition boundariesTests
Type of change
This change is