Skip to content

Fix flattened Dynamic type serialization with binary encoded data types#102692

Merged
Avogar merged 1 commit intomasterfrom
fix-dynamic-flat-serialization
Apr 17, 2026
Merged

Fix flattened Dynamic type serialization with binary encoded data types#102692
Avogar merged 1 commit intomasterfrom
fix-dynamic-flat-serialization

Conversation

@Avogar
Copy link
Copy Markdown
Member

@Avogar Avogar commented Apr 14, 2026

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):

Fix flattened Dynamic type serialization with binary encoded data types. Closes #101911

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Apr 14, 2026

Workflow [PR], commit [9a89987]

Summary:

job_name test_name status info comment
Unit tests (asan_ubsan) failure
FunctionsStress.stress failure cidb IGNORED
AllTests failure cidb IGNORED

AI Review

Summary

This PR fixes flattened Dynamic serialization when output_format_native_encode_types_in_binary_format=1 by switching to the stream-writing encodeDataType overload in SerializationDynamic, and adds a focused stateless roundtrip test. I did not find correctness, safety, performance, or compatibility issues in the diff. High-level verdict: approve from code-review perspective.

Missing context
  • ⚠️ Full CI is still in progress (only partial statuses are currently available), so final merge readiness still depends on CI completion.
ClickHouse Rules
Item Status Notes
Deletion logging
Serialization versioning
Core-area scrutiny
No test removal
Experimental gate
No magic constants
Backward compatibility
SettingsChangesHistory.cpp
PR metadata quality
Safe rollout
Compilation time
No large/binary files
Final Verdict
  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Apr 14, 2026
@yakov-olkhovskiy yakov-olkhovskiy self-assigned this Apr 14, 2026
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Apr 14, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.00% 84.10% +0.10%
Functions 90.90% 90.90% +0.00%
Branches 76.60% 76.60% +0.00%

Changed lines: 100.00% (4/4) · Uncovered code

Full report · Diff report

@Avogar
Copy link
Copy Markdown
Member Author

Avogar commented Apr 17, 2026

Unit tests (asan_ubsan) - fixed in master

@Avogar Avogar added this pull request to the merge queue Apr 17, 2026
Merged via the queue into master with commit 2892de4 Apr 17, 2026
159 of 161 checks passed
@Avogar Avogar deleted the fix-dynamic-flat-serialization branch April 17, 2026 14:39
@clickgapai
Copy link
Copy Markdown
Contributor

Hi — this PR may need backporting to 26.3 (LTS), 26.2, 26.1, 25.8 (LTS), but no backport label was found.

Affected code: SerializationDynamic::serializeBinaryBulkStatePrefix — FLATTENED path binary type encoding at line 172 — introduced in 25.8.

Why: The FLATTENED serialization path was introduced in PR #80499 (merged 2025-06-04), before the 25.8 release (2025-08-29). The buggy encodeDataType(type) call was present since that commit. The encode_types_in_binary_format setting interaction was introduced even earlier in PR #75832 (2025-02-14). All supported branches contain the affected code.

If this should be backported, consider adding pr-must-backport or a version-specific label (e.g. v26.3-must-backport). Ignore this if backporting is not applicable.

@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Apr 17, 2026
@CurtizJ CurtizJ added pr-must-backport Pull request should be backported intentionally. Use this label with great care! v25.12-must-backport labels Apr 21, 2026
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Apr 21, 2026
robot-ch-test-poll added a commit that referenced this pull request Apr 21, 2026
Cherry pick #102692 to 25.8: Fix flattened Dynamic type serialization with binary encoded data types
robot-clickhouse added a commit that referenced this pull request Apr 21, 2026
robot-ch-test-poll added a commit that referenced this pull request Apr 21, 2026
Cherry pick #102692 to 26.1: Fix flattened Dynamic type serialization with binary encoded data types
robot-clickhouse added a commit that referenced this pull request Apr 21, 2026
robot-ch-test-poll added a commit that referenced this pull request Apr 21, 2026
Cherry pick #102692 to 26.2: Fix flattened Dynamic type serialization with binary encoded data types
robot-clickhouse added a commit that referenced this pull request Apr 21, 2026
robot-ch-test-poll added a commit that referenced this pull request Apr 21, 2026
Cherry pick #102692 to 26.3: Fix flattened Dynamic type serialization with binary encoded data types
robot-clickhouse added a commit that referenced this pull request Apr 21, 2026
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Apr 21, 2026
Avogar added a commit that referenced this pull request Apr 22, 2026
Backport #102692 to 26.3: Fix flattened Dynamic type serialization with binary encoded data types
Avogar added a commit that referenced this pull request Apr 22, 2026
Backport #102692 to 26.2: Fix flattened Dynamic type serialization with binary encoded data types
Avogar added a commit that referenced this pull request Apr 22, 2026
Backport #102692 to 26.1: Fix flattened Dynamic type serialization with binary encoded data types
Avogar added a commit that referenced this pull request Apr 22, 2026
Backport #102692 to 25.8: Fix flattened Dynamic type serialization with binary encoded data types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore 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! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo v25.12-must-backport

Projects

None yet

Development

Successfully merging this pull request may close these issues.

encodeDataType called without stream argument in FLATTENED Dynamic serialization discards encoded type data

7 participants