Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions pkg/col/coldata/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,17 @@ func (m *MemBatch) AppendCol(col *Vec) {

// ReplaceCol implements the Batch interface.
func (m *MemBatch) ReplaceCol(col *Vec, colIdx int) {
if m.b[colIdx] != nil && m.b[colIdx].t != nil && !m.b[colIdx].Type().Identical(col.Type()) {
// NB: we use Equivalent() rather than Identical() here because
// ReplaceCol is a physical-level operation - it swaps column vector
// pointers. What matters for correctness is that the replacement
// vector has the same physical representation (i.e. the same canonical
// type family), not that the logical SQL types are exactly identical.
// For example, VARBIT(1) and VARBIT (no width limit) are both backed
// by DatumVec and are physically interchangeable, but Identical()
// rejects them because they differ in Width. Equivalent() checks type
// family compatibility which is the right semantic here.
// Fixes: https://github.com/cockroachdb/cockroach/issues/172870
if m.b[colIdx] != nil && m.b[colIdx].t != nil && !m.b[colIdx].Type().Equivalent(col.Type()) {
panic(fmt.Sprintf("unexpected replacement: original vector is %s "+
"whereas the replacement is %s", m.b[colIdx].Type(), col.Type()))
}
Expand All @@ -409,11 +419,11 @@ func (m *MemBatch) ReplaceCol(col *Vec, colIdx int) {
func (m *MemBatch) Reset(typs []*types.T, length int, factory ColumnFactory) {
cannotReuse := m == nil || m.Capacity() < length || m.Width() < len(typs)
for i := 0; i < len(typs) && !cannotReuse; i++ {
// TODO(yuzefovich): change this when DatumVec is introduced.
// TODO(yuzefovich): requiring that types are "identical" might be an
// overkill - the vectors could have the same physical representation
// but non-identical types. Think through this more.
if v := m.ColVec(i); !v.Type().Identical(typs[i]) {
// NB: we use Equivalent() rather than Identical() to determine batch
// reusability. Vectors with the same physical representation but
// non-identical logical types (e.g. VARBIT(1) vs VARBIT) should
// allow the batch to be reused.
if v := m.ColVec(i); !v.Type().Equivalent(typs[i]) {
cannotReuse = true
break
}
Expand Down
30 changes: 30 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/union
Original file line number Diff line number Diff line change
Expand Up @@ -799,3 +799,33 @@ TABLE t130591_1 INTERSECT TABLE t130591_1;

statement count 1
TABLE t130591_1 UNION TABLE t130591_1;

# Regression test for #172870. UNION ALL with BIT/VARBIT expressions that have
# width constraints should not cause vectorized execution internal errors. The
# bug was that ReplaceCol used Type().Identical() which rejected types like
# VARBIT(1) vs VARBIT (different Width) even though they have the same physical
# representation in the vectorized engine.

query T
SELECT IF(false, ARRAY[]::VARBIT(1)[], NULL) GROUP BY 1 UNION ALL SELECT NULL;
----
NULL
NULL

query T
SELECT IF(false, ARRAY[]::BIT(1)[], NULL) GROUP BY 1 UNION ALL SELECT NULL;
----
NULL
NULL

query T
SELECT IF(false, B''::VARBIT(1), NULL) GROUP BY 1 UNION ALL SELECT NULL;
----
NULL
NULL

query T
SELECT IF(false, B''::BIT(1), NULL) GROUP BY 1 UNION ALL SELECT NULL;
----
NULL
NULL
Loading