[4.2-dev] fix(hashmap): accept broadcast const iterator vectors - #26843
Merged
mergify[bot] merged 2 commits intoAug 9, 2026
Merged
Conversation
(cherry picked from commit f3cda59)
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
XuPeng-SH
approved these changes
Aug 9, 2026
XuPeng-SH
left a comment
XuPeng-SH
left a comment
Contributor
There was a problem hiding this comment.
Deep-reviewed exact head 917f6bc. No blocking correctness, performance, or unhappy-path findings. The cherry-pick has the same stable patch-id as source PR #26836. Const and const-null broadcast semantics are restored while malformed flat and empty const reads remain rejected. Focused tests, race x100, full hashmap tests and race, direct group consumer tests, build, vet, diff-check, and base/head benchmarks all pass. Flat Find remains 0 B/op and 0 allocs/op. Review submitted without waiting for remaining CI.
Contributor
Merge Queue Status
This pull request spent 11 seconds in the queue, including 1 second running CI. Required conditions to merge
|
7 tasks
XuPeng-SH
added a commit
that referenced
this pull request
Aug 9, 2026
## What type of PR is this? - [ ] API-change - [x] BUG - [ ] Improvement - [ ] Documentation - [ ] Feature - [ ] Test and CI - [ ] Code Refactoring ## Which issue(s) this PR fixes: issue #25992 Follow-up to #26843 (the `4.2-dev` backport of #26836). ## What this PR does / why we need it: After #26843 accepts a physical length-1 const vector as a broadcast input, a prepared grouped query can correctly produce a projection batch whose logical row count is greater than its first vector's physical length: ```sql SELECT ? AS projection_value, SUM(value_col) AS total FROM metric_rows GROUP BY bucket ORDER BY total; ``` The frontend still derived result cardinality from `bat.Vecs[0].Length()`. Consequently, it emitted only one MySQL result row for a three-row batch and also undercounted `sentRows`. Saved query results had a second inconsistency: their metadata reported three rows, but object storage and `result_scan` preserved only the first physical row. This change uses `batch.RowCount()`, the logical cardinality propagated by the projection operator, for protocol output and row accounting. At the query-result persistence boundary, it validates vector shapes and creates an owned writer view only when compact const vectors need their logical lengths normalized. Aligned vectors are borrowed, mismatched const/const-null vectors remain physically scalar, and the executor-owned input batch is never mutated. Regression coverage includes flat, const, broadcast const, const-null, malformed and zero-row batches; allocation/write failure cleanup; real object writer plus `result_scan`; SQL-level prepared statement reuse; and the original `GROUP BY ?` path. Validation on the exact `4.2-dev` base `9301a23cc8`: - The direct-output BVT on the unfixed base: 19/21 statements pass; both parameter-first executions incorrectly return one row. - On the direct-output-only candidate, `EXECUTE` returns 3 rows but the immediate `result_scan(last_query_id())` returns 1 row. - Focused frontend regressions, `-count=10`: pass. - Full `pkg/frontend` test suite: pass. - `go build -mod=readonly ./pkg/frontend`: pass. - `go vet -mod=readonly ./pkg/frontend`: pass. - `make build`: pass. - Isolated single-node fixed candidate BVT, including saved scans for first, NULL-reuse, and non-first parameters: 26/26 statements pass. - MySQL binary `COM_STMT_PREPARE/EXECUTE` probe: parameter-first returns 3 rows, NULL statement reuse returns 3 rows, and the #26843 grouped-parameter control returns 1 row. Direct output keeps an O(1) cardinality lookup. Saved aligned batches allocate nothing beyond the existing path. A mismatched const adds an O(column count) validation and copies only scalar physical storage; no per-row materialization or global object-writer contract change is introduced.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Which issue(s) this PR fixes:
issue #25992
What this PR does / why we need it:
Backport #26836 to
4.2-dev.#26042 fixed the original prepared-parameter identity problem. Later, #26531
added strict HashMap iterator row-range validation and incorrectly treated a
physical length-1 const vector as a flat vector. Prepared
GROUP BY ?thereforefails whenever the input batch contains more than one logical row.
This change preserves strict bounds for flat vectors while accepting non-empty
const and const-null vectors as broadcast inputs, matching the existing IntHashMap
and StrHashMap encoders. Zero-length const vectors are still rejected whenever
rows are requested.
Validation on the
4.2-devbase (4066fffaf7):-count=10pkg/common/hashmaptest and race testgo buildandgo vetforpkg/common/hashmapvalidateIteratorVectorscoverage:100%The cherry-picked commit retains
-xprovenance for #26836.