Skip to content

Fix possible logical error during reading Map subcolumns#101641

Merged
alexey-milovidov merged 3 commits intomasterfrom
fix-map-subcolumn-bug
Apr 9, 2026
Merged

Fix possible logical error during reading Map subcolumns#101641
alexey-milovidov merged 3 commits intomasterfrom
fix-map-subcolumn-bug

Conversation

@Avogar
Copy link
Copy Markdown
Member

@Avogar Avogar commented Apr 2, 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 possible logical error during reading Map subcolumns. Closes #100769. Closes #101336

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 2, 2026

Workflow [PR], commit [6bae80a]

Summary:

job_name test_name status info comment
Stateless tests (arm_binary, parallel) failure
04092_tcp_settings_parse_error_closes_connection FAIL cidb
Integration tests (amd_asan_ubsan, db disk, old analyzer, 1/6) failure
test_database_iceberg_nessie_catalog/test.py::test_select FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_tables_with_same_location FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_timestamps FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_insert FAIL cidb
Integration tests (amd_asan_ubsan, db disk, old analyzer, 6/6) failure
test_database_iceberg_lakekeeper_catalog/test.py::test_select FAIL cidb
test_database_iceberg_lakekeeper_catalog/test.py::test_tables_with_same_location FAIL cidb
Integration tests (amd_binary, 3/5) failure
test_database_iceberg_lakekeeper_catalog/test.py::test_select FAIL cidb
test_database_iceberg_lakekeeper_catalog/test.py::test_tables_with_same_location FAIL cidb
Integration tests (amd_binary, 4/5) failure
test_database_iceberg_nessie_catalog/test.py::test_select FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_tables_with_same_location FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_timestamps FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_insert FAIL cidb
Integration tests (arm_binary, distributed plan, 1/4) failure
test_database_iceberg_nessie_catalog/test.py::test_select FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_tables_with_same_location FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_timestamps FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_insert FAIL cidb
Integration tests (arm_binary, distributed plan, 4/4) failure
test_database_iceberg_lakekeeper_catalog/test.py::test_select FAIL cidb
test_database_iceberg_lakekeeper_catalog/test.py::test_tables_with_same_location FAIL cidb
Integration tests (amd_tsan, 1/6) failure
test_database_iceberg_nessie_catalog/test.py::test_select FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_tables_with_same_location FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_timestamps FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_insert FAIL cidb
Integration tests (amd_tsan, 6/6) failure
test_database_iceberg_lakekeeper_catalog/test.py::test_select FAIL cidb
test_database_iceberg_lakekeeper_catalog/test.py::test_tables_with_same_location FAIL cidb
Integration tests (amd_msan, 1/6) failure
test_database_iceberg_nessie_catalog/test.py::test_select FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_tables_with_same_location FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_timestamps FAIL cidb
test_database_iceberg_nessie_catalog/test.py::test_insert FAIL cidb

AI Review

Summary

This PR fixes a real correctness issue in SerializationMapKeyValue::deserializeBinaryBulkWithMultipleStreams for Map subcolumn reads by always enabling insert_only_rows_in_current_range_from_substreams_cache in the temporary nested-column path, and adds a focused stateless regression test (04076_map_key_value_subcolumn_cache_bug.sql) that reproduces the multi-range read scenario. I did not find blocker- or major-level correctness, safety, concurrency, performance, or compliance issues in the proposed change.

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 2, 2026
@antaljanosbenjamin antaljanosbenjamin self-assigned this Apr 2, 2026
@alexey-milovidov
Copy link
Copy Markdown
Member

The Stress test (arm_msan) failure is fixed by #101239, which should be merged first. After it is merged, please update the branch to include the fix.

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Apr 9, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.00% 84.00% +0.00%
Functions 90.90% 90.90% +0.00%
Branches 76.50% 76.50% +0.00%

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

Full report · Diff report

@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Apr 9, 2026

The failures look related.

Upd: they aren't.

@alexey-milovidov
Copy link
Copy Markdown
Member

Except Iceberg failures - they are likely due to Docker.

@alexey-milovidov
Copy link
Copy Markdown
Member

Checked the test 04092_tcp_settings_parse_error_closes_connection it was reverted in master.

@alexey-milovidov alexey-milovidov self-assigned this Apr 9, 2026
@alexey-milovidov alexey-milovidov merged commit fb1aa42 into master Apr 9, 2026
146 of 163 checks passed
@alexey-milovidov alexey-milovidov deleted the fix-map-subcolumn-bug branch April 9, 2026 21:03
@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 9, 2026
@alexey-milovidov
Copy link
Copy Markdown
Member

The test_database_iceberg_nessie_catalog and test_database_iceberg_lakekeeper_catalog failures in this PR's CI are false positives caused by a bug in PR #100583 (merged to master on 2026-04-09 at 11:19 UTC). That PR broke the metadata file path resolution in getMetadataLocation for Iceberg catalog tests — it produced a doubled S3 key like table_path/warehouse-rest/table_path/metadata/... instead of table_path/metadata/..., causing MinIO to return 403.

The bug was reverted by PR #102247 (merged at ~14:48 UTC). All nessie/lakekeeper test failures between these two merges are unrelated to this PR.

robot-clickhouse added a commit that referenced this pull request Apr 9, 2026
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Apr 9, 2026
@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Apr 9, 2026
clickhouse-gh Bot added a commit that referenced this pull request Apr 10, 2026
Backport #101641 to 26.3: Fix possible logical error during reading Map subcolumns
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-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo v26.3-must-backport

Projects

None yet

5 participants