MergeTreeData: force Wide part format when the table has deprecated Object columns#1416
MergeTreeData: force Wide part format when the table has deprecated Object columns#1416mkmkme wants to merge 3 commits intoreleases/25.8.16from
Conversation
…bject columns fixes #1412 Here's the analysis of #1412 by Claude: Root cause: The old Object('json') type converts JSON data to nested Tuple structures before storage. When parts have different JSON schemas, reading subcolumns from compact parts without per-substream marks fails — the compact reader's deserialization can't properly handle the complex nested type serialization, leading to tuple elements being deserialized with mismatched sizes. Crash: Logical error: 'Unexpected size of tuple element 1: 0. Expected size: 1' in SerializationTuple::deserializeBinaryBulkWithMultipleStreams Recommended Fix: Force wide parts for tables with deprecated Object columns In MergeTreeData::choosePartFormat, detect tables with deprecated Object columns and always choose Wide format. Wide parts work correctly with all ClickHouse versions and handle complex nested types properly. This is clean and targeted because: - Only affects tables using the deprecated Object('json') type (narrow scope) - The enum ordering (Wide=0 < Compact=1) means std::min in merge logic will pick Wide when choosePartFormat returns it, so even existing compact parts get rewritten to Wide during natural merges - No changes needed to the complex compact reader deserialization code
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afbc938189
ℹ️ 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".
tests/queries/0_stateless/03779_object_in_compact_mergetree_parts.sql
Outdated
Show resolved
Hide resolved
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cb21ed200
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac88a154f1
ℹ️ 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".
mkmkme
force-pushed
the
mkmkme/object-fix
branch
from
ac88a15 to
0cb21ed
Compare
QA VerificationIssue: #1412 Test Results
Verification
Verified. This fix resolves #1412. |
4 participants
fixes #1412
Here's the analysis of #1412 by Claude:
Changelog category (leave one):
CI/CD Options
Exclude tests:
Regression jobs to run: