coldata: use Equivalent() instead of Identical() in ReplaceCol - #172874
Open
shivamshaw23 wants to merge 1 commit into
Open
coldata: use Equivalent() instead of Identical() in ReplaceCol#172874shivamshaw23 wants to merge 1 commit into
shivamshaw23 wants to merge 1 commit into
Conversation
|
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
|
Previously, ReplaceCol and Reset in MemBatch used Type().Identical()
to validate column vector replacements. Identical() requires exact
match of all type metadata including Width, Precision, and Oid. This
caused a panic when two vectors had the same physical representation
but different type metadata - for example, VARBIT(1) (Width=1) vs
VARBIT (Width=0), which are both backed by DatumVec.
This manifested as a vectorized runtime crash for valid queries
involving BIT/VARBIT expressions with width constraints followed by
GROUP BY and UNION ALL:
SELECT IF(false, ARRAY[]::VARBIT(1)[], NULL)
GROUP BY 1 UNION ALL SELECT NULL;
ERROR: unexpected replacement: original vector is varbit[]
whereas the replacement is varbit[]
The fix changes both ReplaceCol and Reset to use Type().Equivalent()
instead. Equivalent() checks type family compatibility without
requiring exact width/precision/OID match, which is the correct
semantic for physical-level column vector operations. This also
resolves a long-standing TODO noting that Identical() was overkill.
Fixes: cockroachdb#172870
Release note (bug fix): Fixed an internal error in the vectorized
execution engine that occurred when running queries with BIT or VARBIT
types (with width constraints) combined with GROUP BY and UNION ALL.
The error message was: 'unexpected replacement: original vector is
varbit[] whereas the replacement is varbit[]'.
shivamshaw23
force-pushed
the
fix-172870-varbit-vectorized-replace-col
branch
from
July 28, 2026 07:29
dc192c0 to
827669f
Compare
|
Thank you for updating your pull request. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Author
|
@cla-bot check |
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
1 participant
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.
Summary
Fixed a vectorized execution internal error for valid queries involving
BIT/VARBIT types (with width constraints) combined with GROUP BY and
UNION ALL.
Error:
unexpected replacement: original vector is varbit[] whereas the replacement is varbit[]Root Cause
ReplaceColinMemBatchusedType().Identical()to validate columnvector replacements.
Identical()requires exact match of all typemetadata including Width, Precision, and Oid. When the
SerialUnorderedSynchronizer(used for UNION ALL) swapped column vectorsfrom the second input into the output batch, two VARBIT vectors with
different widths (e.g.,
VARBIT(1)with Width=1 vsVARBITwithWidth=0) caused a panic — despite having the same physical
representation (both backed by
DatumVec).Fix
Changed
ReplaceColandResetinMemBatchto useType().Equivalent()instead ofType().Identical().Equivalent()checks type family compatibility without requiringexact width/precision/OID match, which is the correct semantic for
physical-level column vector operations. This also resolves a
long-standing TODO in the code that noted
Identical()was likelyoverkill.
Changes
pkg/col/coldata/batch.go: ChangedReplaceColandResetto use
Equivalent()instead ofIdentical(). Removed stale TODOcomment that explicitly called out this issue.
pkg/sql/logictest/testdata/logic_test/union: Added 4regression test cases covering all reproducing variants (VARBIT[],
BIT[], VARBIT scalar, BIT scalar — all with width constraints).
Fixes: #172870
Release note (bug fix): Fixed an internal error in the vectorized
execution engine that occurred when running queries with BIT or VARBIT
types (with width constraints) combined with GROUP BY and UNION ALL.
The error message was:
unexpected replacement: original vector is varbit[] whereas the replacement is varbit[].