From 5ffb61f556ca6aadfe7ec6d4807d761abc8ba241 Mon Sep 17 00:00:00 2001 From: Cao Kai Date: Wed, 1 Jul 2026 16:17:10 +0800 Subject: [PATCH 01/11] fix: BIT_AND/BIT_OR/BIT_XOR return neutral values for empty or all-NULL groups BIT_AND(), BIT_OR(), BIT_XOR() now return MySQL-compatible neutral values for groups with no non-NULL input values: - BIT_AND: all bits set (max uint64 / 8 bytes of 0xFF) - BIT_OR: 0 - BIT_XOR: 0 Previously, these returned NULL for empty or all-NULL groups. Fixes #25285 Co-Authored-By: Claude --- pkg/sql/colexec/aggexec/bitops2.go | 36 +++ pkg/sql/colexec/aggexec/bitops2_test.go | 330 ++++++++++++++++++++++++ 2 files changed, 366 insertions(+) create mode 100644 pkg/sql/colexec/aggexec/bitops2_test.go diff --git a/pkg/sql/colexec/aggexec/bitops2.go b/pkg/sql/colexec/aggexec/bitops2.go index f5b9db84f1bfd..93cd9d5b9e8aa 100644 --- a/pkg/sql/colexec/aggexec/bitops2.go +++ b/pkg/sql/colexec/aggexec/bitops2.go @@ -15,6 +15,7 @@ package aggexec import ( + "math" "slices" "github.com/matrixorigin/matrixone/pkg/common/moerr" @@ -151,6 +152,27 @@ func (exec *bitOpExecFixed[T]) Flush() ([]*vector.Vector, error) { exec.state[i].vecs[0] = nil exec.state[i].length = 0 exec.state[i].capacity = 0 + + // Replace NULL entries with neutral values per MySQL semantics: + // BIT_AND: neutral is all bits set (max uint64) + // BIT_OR, BIT_XOR: neutral is 0 + if exec.op == bitAnd { + aggs := vector.MustFixedColNoTypeCheck[uint64](vecs[i]) + for j := 0; j < vecs[i].Length(); j++ { + if vecs[i].IsNull(uint64(j)) { + vecs[i].UnsetNull(uint64(j)) + aggs[j] = math.MaxUint64 + } + } + } else { + aggs := vector.MustFixedColNoTypeCheck[uint64](vecs[i]) + for j := 0; j < vecs[i].Length(); j++ { + if vecs[i].IsNull(uint64(j)) { + vecs[i].UnsetNull(uint64(j)) + aggs[j] = 0 + } + } + } } return vecs, nil } @@ -237,6 +259,20 @@ func (exec *bitOpExecBytes) Flush() ([]*vector.Vector, error) { exec.state[i].vecs[0] = nil exec.state[i].length = 0 exec.state[i].capacity = 0 + + // Replace NULL entries with neutral values per MySQL semantics: + // BIT_AND: neutral is all bits set (8 bytes of 0xFF) + // BIT_OR, BIT_XOR: neutral is empty bytes + var neutralBytesForAnd = []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF} + for j := 0; j < vecs[i].Length(); j++ { + if vecs[i].IsNull(uint64(j)) { + vecs[i].UnsetNull(uint64(j)) + if exec.op == bitAnd { + _ = vector.SetBytesAt(vecs[i], j, neutralBytesForAnd, exec.mp) + } + // For bitOr and bitXor, the default zero bytes (empty) is already the neutral value + } + } } return vecs, nil } diff --git a/pkg/sql/colexec/aggexec/bitops2_test.go b/pkg/sql/colexec/aggexec/bitops2_test.go new file mode 100644 index 0000000000000..75efc5e353218 --- /dev/null +++ b/pkg/sql/colexec/aggexec/bitops2_test.go @@ -0,0 +1,330 @@ +// Copyright 2026 Matrix Origin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package aggexec + +import ( + "math" + "testing" + + "github.com/matrixorigin/matrixone/pkg/common/mpool" + "github.com/matrixorigin/matrixone/pkg/container/types" + "github.com/matrixorigin/matrixone/pkg/container/vector" + "github.com/stretchr/testify/require" +) + +// makeBitAggExec creates a bit aggregate exec directly without going through +// the function registry (which is not set up in unit tests). +func makeBitAggExec(mp *mpool.MPool, aggID int64, param types.Type) AggFuncExec { + switch aggID { + case AggIdOfBitAnd: + return makeBitAndExec(mp, aggID, false, param) + case AggIdOfBitOr: + return makeBitOrExec(mp, aggID, false, param) + case AggIdOfBitXor: + return makeBitXorExec(mp, aggID, false, param) + default: + panic("unknown bit agg id") + } +} + +func TestBitOpsInt64(t *testing.T) { + mp := mpool.MustNewZero() + typ := types.T_int64.ToType() + + makeInputVec := func(values []int64, nulls []bool) *vector.Vector { + vec := vector.NewVec(typ) + for i, v := range values { + isNull := false + if nulls != nil && nulls[i] { + isNull = true + } + require.NoError(t, vector.AppendFixed(vec, v, isNull, mp)) + } + return vec + } + + t.Run("normal values", func(t *testing.T) { + // BIT_AND: 3 & 5 & 7 = 1 + // BIT_OR: 3 | 5 | 7 = 7 + // BIT_XOR: 3 ^ 5 ^ 7 = 1 (3^5=6, 6^7=1) + vec := makeInputVec([]int64{3, 5, 7}, nil) + defer vec.Free(mp) + + for _, tc := range []struct { + name string + aggID int64 + expect uint64 + }{ + {"bit_and", AggIdOfBitAnd, 1}, + {"bit_or", AggIdOfBitOr, 7}, + {"bit_xor", AggIdOfBitXor, 1}, + } { + t.Run(tc.name, func(t *testing.T) { + exec := makeBitAggExec(mp, tc.aggID, typ) + require.NoError(t, exec.GroupGrow(1)) + require.NoError(t, exec.BulkFill(0, []*vector.Vector{vec})) + + results, err := exec.Flush() + require.NoError(t, err) + require.Len(t, results, 1) + require.Equal(t, tc.expect, vector.MustFixedColNoTypeCheck[uint64](results[0])[0]) + + exec.Free() + for _, r := range results { + r.Free(mp) + } + }) + } + }) + + t.Run("empty group returns neutral values", func(t *testing.T) { + for _, tc := range []struct { + name string + aggID int64 + expect uint64 + }{ + {"bit_and empty", AggIdOfBitAnd, math.MaxUint64}, + {"bit_or empty", AggIdOfBitOr, 0}, + {"bit_xor empty", AggIdOfBitXor, 0}, + } { + t.Run(tc.name, func(t *testing.T) { + exec := makeBitAggExec(mp, tc.aggID, typ) + require.NoError(t, exec.GroupGrow(1)) + // no Fill/BulkFill/BatchFill called — simulating empty group + + results, err := exec.Flush() + require.NoError(t, err) + require.Len(t, results, 1) + require.Equal(t, tc.expect, vector.MustFixedColNoTypeCheck[uint64](results[0])[0]) + + exec.Free() + for _, r := range results { + r.Free(mp) + } + }) + } + }) + + t.Run("all-NULL group returns neutral values", func(t *testing.T) { + vec := makeInputVec([]int64{0, 0, 0}, []bool{true, true, true}) + defer vec.Free(mp) + + for _, tc := range []struct { + name string + aggID int64 + expect uint64 + }{ + {"bit_and all null", AggIdOfBitAnd, math.MaxUint64}, + {"bit_or all null", AggIdOfBitOr, 0}, + {"bit_xor all null", AggIdOfBitXor, 0}, + } { + t.Run(tc.name, func(t *testing.T) { + exec := makeBitAggExec(mp, tc.aggID, typ) + require.NoError(t, exec.GroupGrow(1)) + require.NoError(t, exec.BulkFill(0, []*vector.Vector{vec})) + + results, err := exec.Flush() + require.NoError(t, err) + require.Len(t, results, 1) + require.Equal(t, tc.expect, vector.MustFixedColNoTypeCheck[uint64](results[0])[0]) + + exec.Free() + for _, r := range results { + r.Free(mp) + } + }) + } + }) + + t.Run("mixed null and non-null", func(t *testing.T) { + // BIT_AND: 6 & NULL & 7 = 6 & 7 = 6 + // BIT_OR: 6 | NULL | 7 = 6 | 7 = 7 + // BIT_XOR: 6 ^ NULL ^ 7 = 6 ^ 7 = 1 + vec := makeInputVec([]int64{6, 0, 7}, []bool{false, true, false}) + defer vec.Free(mp) + + for _, tc := range []struct { + name string + aggID int64 + expect uint64 + }{ + {"bit_and mixed", AggIdOfBitAnd, 6}, + {"bit_or mixed", AggIdOfBitOr, 7}, + {"bit_xor mixed", AggIdOfBitXor, 1}, + } { + t.Run(tc.name, func(t *testing.T) { + exec := makeBitAggExec(mp, tc.aggID, typ) + require.NoError(t, exec.GroupGrow(1)) + require.NoError(t, exec.BulkFill(0, []*vector.Vector{vec})) + + results, err := exec.Flush() + require.NoError(t, err) + require.Len(t, results, 1) + require.Equal(t, tc.expect, vector.MustFixedColNoTypeCheck[uint64](results[0])[0]) + + exec.Free() + for _, r := range results { + r.Free(mp) + } + }) + } + }) +} + +func TestBitOpsMultipleGroups(t *testing.T) { + mp := mpool.MustNewZero() + typ := types.T_int64.ToType() + + makeInputVec := func(values []int64) *vector.Vector { + vec := vector.NewVec(typ) + for _, v := range values { + require.NoError(t, vector.AppendFixed(vec, v, false, mp)) + } + return vec + } + + t.Run("multiple groups with mixed data", func(t *testing.T) { + // Group 1: 3 & 5 & 7 = 1 + // Group 2: empty (no values) → neutral + // Group 3: 6 & 2 = 2 for AND, 6|2=6 for OR, 6^2=4 for XOR + vec := makeInputVec([]int64{3, 5, 7, 6, 2}) + defer vec.Free(mp) + // Row 0 → group 1, Row 1 → group 1, Row 2 → group 1 + // Row 3 → group 3, Row 4 → group 3 + // Group 2 intentionally left empty + groups := []uint64{1, 1, 1, 3, 3} + + for _, tc := range []struct { + name string + aggID int64 + expectGroup1 uint64 + expectGroup2 uint64 + expectGroup3 uint64 + }{ + {"bit_and multi-group", AggIdOfBitAnd, 1, math.MaxUint64, 2}, + {"bit_or multi-group", AggIdOfBitOr, 7, 0, 6}, + {"bit_xor multi-group", AggIdOfBitXor, 1, 0, 4}, + } { + t.Run(tc.name, func(t *testing.T) { + exec := makeBitAggExec(mp, tc.aggID, typ) + require.NoError(t, exec.GroupGrow(3)) + require.NoError(t, exec.BatchFill(0, groups, []*vector.Vector{vec})) + + results, err := exec.Flush() + require.NoError(t, err) + require.Len(t, results, 1) + + aggs := vector.MustFixedColNoTypeCheck[uint64](results[0]) + require.Equal(t, tc.expectGroup1, aggs[0]) + require.Equal(t, tc.expectGroup2, aggs[1]) + require.Equal(t, tc.expectGroup3, aggs[2]) + + exec.Free() + for _, r := range results { + r.Free(mp) + } + }) + } + }) +} + +func TestBitOpsBytes(t *testing.T) { + mp := mpool.MustNewZero() + typ := types.T_varbinary.ToType() + + makeInputVec := func(values [][]byte, nulls []bool) *vector.Vector { + vec := vector.NewVec(typ) + for i, v := range values { + isNull := false + if nulls != nil && nulls[i] { + isNull = true + } + require.NoError(t, vector.AppendBytes(vec, v, isNull, mp)) + } + return vec + } + + t.Run("normal values byte bit_or", func(t *testing.T) { + // BIT_OR: 0x01 | 0x02 | 0x04 = 0x07 + vec := makeInputVec([][]byte{{0x01}, {0x02}, {0x04}}, nil) + defer vec.Free(mp) + + exec := makeBitAggExec(mp, AggIdOfBitOr, typ) + require.NoError(t, exec.GroupGrow(1)) + require.NoError(t, exec.BulkFill(0, []*vector.Vector{vec})) + + results, err := exec.Flush() + require.NoError(t, err) + require.Len(t, results, 1) + require.Equal(t, []byte{0x07}, results[0].GetBytesAt(0)) + + exec.Free() + for _, r := range results { + r.Free(mp) + } + }) + + t.Run("empty group byte bit_or returns empty", func(t *testing.T) { + exec := makeBitAggExec(mp, AggIdOfBitOr, typ) + require.NoError(t, exec.GroupGrow(1)) + + results, err := exec.Flush() + require.NoError(t, err) + require.Len(t, results, 1) + require.False(t, results[0].IsNull(0)) + require.Equal(t, []byte{}, results[0].GetBytesAt(0)) + + exec.Free() + for _, r := range results { + r.Free(mp) + } + }) + + t.Run("empty group byte bit_and returns all-ones", func(t *testing.T) { + exec := makeBitAggExec(mp, AggIdOfBitAnd, typ) + require.NoError(t, exec.GroupGrow(1)) + + results, err := exec.Flush() + require.NoError(t, err) + require.Len(t, results, 1) + require.False(t, results[0].IsNull(0)) + require.Equal(t, []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}, results[0].GetBytesAt(0)) + + exec.Free() + for _, r := range results { + r.Free(mp) + } + }) + + t.Run("all-NULL byte bit_xor returns empty", func(t *testing.T) { + vec := makeInputVec([][]byte{{0x01}, {0x02}}, []bool{true, true}) + defer vec.Free(mp) + + exec := makeBitAggExec(mp, AggIdOfBitXor, typ) + require.NoError(t, exec.GroupGrow(1)) + require.NoError(t, exec.BulkFill(0, []*vector.Vector{vec})) + + results, err := exec.Flush() + require.NoError(t, err) + require.Len(t, results, 1) + require.False(t, results[0].IsNull(0)) + require.Equal(t, []byte{}, results[0].GetBytesAt(0)) + + exec.Free() + for _, r := range results { + r.Free(mp) + } + }) +} From d75b0c27ebdd47e5cdc4922f185a541c7578ca0a Mon Sep 17 00:00:00 2001 From: Cao Kai Date: Wed, 1 Jul 2026 16:41:33 +0800 Subject: [PATCH 02/11] fix: address review feedback for BIT_AND/BIT_OR/BIT_XOR neutral values - Move neutralBytesForAnd allocation out of inner loop - Propagate SetBytesAt error instead of silently discarding - Fix gofmt alignment in test file Co-Authored-By: Claude --- pkg/sql/colexec/aggexec/bitops2.go | 6 ++++-- pkg/sql/colexec/aggexec/bitops2_test.go | 10 +++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pkg/sql/colexec/aggexec/bitops2.go b/pkg/sql/colexec/aggexec/bitops2.go index 93cd9d5b9e8aa..926a19e2decfa 100644 --- a/pkg/sql/colexec/aggexec/bitops2.go +++ b/pkg/sql/colexec/aggexec/bitops2.go @@ -254,6 +254,7 @@ func (exec *bitOpExecBytes) SetExtraInformation(partialResult any, _ int) error func (exec *bitOpExecBytes) Flush() ([]*vector.Vector, error) { // transfer vector to result vecs := make([]*vector.Vector, len(exec.state)) + neutralBytesForAnd := []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF} for i := range vecs { vecs[i] = exec.state[i].vecs[0] exec.state[i].vecs[0] = nil @@ -263,12 +264,13 @@ func (exec *bitOpExecBytes) Flush() ([]*vector.Vector, error) { // Replace NULL entries with neutral values per MySQL semantics: // BIT_AND: neutral is all bits set (8 bytes of 0xFF) // BIT_OR, BIT_XOR: neutral is empty bytes - var neutralBytesForAnd = []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF} for j := 0; j < vecs[i].Length(); j++ { if vecs[i].IsNull(uint64(j)) { vecs[i].UnsetNull(uint64(j)) if exec.op == bitAnd { - _ = vector.SetBytesAt(vecs[i], j, neutralBytesForAnd, exec.mp) + if err := vector.SetBytesAt(vecs[i], j, neutralBytesForAnd, exec.mp); err != nil { + return nil, err + } } // For bitOr and bitXor, the default zero bytes (empty) is already the neutral value } diff --git a/pkg/sql/colexec/aggexec/bitops2_test.go b/pkg/sql/colexec/aggexec/bitops2_test.go index 75efc5e353218..bce3f9218a49d 100644 --- a/pkg/sql/colexec/aggexec/bitops2_test.go +++ b/pkg/sql/colexec/aggexec/bitops2_test.go @@ -207,11 +207,11 @@ func TestBitOpsMultipleGroups(t *testing.T) { groups := []uint64{1, 1, 1, 3, 3} for _, tc := range []struct { - name string - aggID int64 - expectGroup1 uint64 - expectGroup2 uint64 - expectGroup3 uint64 + name string + aggID int64 + expectGroup1 uint64 + expectGroup2 uint64 + expectGroup3 uint64 }{ {"bit_and multi-group", AggIdOfBitAnd, 1, math.MaxUint64, 2}, {"bit_or multi-group", AggIdOfBitOr, 7, 0, 6}, From 40213109d6eb643d61eb042baeb738e79cb9a5d0 Mon Sep 17 00:00:00 2001 From: Cao Kai Date: Wed, 1 Jul 2026 17:16:02 +0800 Subject: [PATCH 03/11] fix: update BVT result for BIT_AND/BIT_OR/BIT_XOR neutral values Empty or all-NULL groups now return MySQL-compatible neutral values: - BIT_AND: 18446744073709551615 (all bits set) - BIT_OR/BIT_XOR: 0 Co-Authored-By: Claude --- .../cases/function/func_aggr_bitwise.result | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/test/distributed/cases/function/func_aggr_bitwise.result b/test/distributed/cases/function/func_aggr_bitwise.result index dbc610f86fe91..62bc025578196 100644 --- a/test/distributed/cases/function/func_aggr_bitwise.result +++ b/test/distributed/cases/function/func_aggr_bitwise.result @@ -1,37 +1,37 @@ SELECT bit_and(null), bit_or(null), bit_xor(null); bit_and(null) bit_or(null) bit_xor(null) -null null null +18446744073709551615 0 0 CREATE TABLE t1 (a int, b int); INSERT INTO t1 VALUES (1,null); INSERT INTO t1 VALUES (1,null); INSERT INTO t1 VALUES (2,null); select a, BIT_AND(b), BIT_OR(b), BIT_XOR(b) from t1 group by a; a BIT_AND(b) BIT_OR(b) BIT_XOR(b) -1 null null null -2 null null null +1 18446744073709551615 0 0 +2 18446744073709551615 0 0 SELECT a, OCT(BIT_AND(b)), OCT(BIT_OR(b)), OCT(BIT_XOR(b)) FROM t1 GROUP BY a; a OCT(BIT_AND(b)) OCT(BIT_OR(b)) OCT(BIT_XOR(b)) -1 null null null -2 null null null +1 1777777777777777777777 0 0 +2 1777777777777777777777 0 0 SELECT OCT(BIT_AND(b)), OCT(BIT_OR(b)), OCT(BIT_XOR(b)) FROM t1; OCT(BIT_AND(b)) OCT(BIT_OR(b)) OCT(BIT_XOR(b)) -null null null +1777777777777777777777 0 0 INSERT INTO t1 VALUES (3,123); SELECT a, OCT(BIT_AND(b)), OCT(BIT_OR(b)), OCT(BIT_XOR(b)) FROM t1 GROUP BY a; a OCT(BIT_AND(b)) OCT(BIT_OR(b)) OCT(BIT_XOR(b)) -1 null null null -2 null null null +1 1777777777777777777777 0 0 +2 1777777777777777777777 0 0 3 173 173 173 INSERT INTO t1 VALUES (2,124124), (3, 4951312); SELECT a, OCT(BIT_AND(b)), OCT(BIT_OR(b)), OCT(BIT_XOR(b)) FROM t1 GROUP BY a; a OCT(BIT_AND(b)) OCT(BIT_OR(b)) OCT(BIT_XOR(b)) -1 null null null +1 1777777777777777777777 0 0 2 362334 362334 362334 3 20 22706573 22706553 INSERT INTO t1 VALUES (4,-4124124); SELECT a, OCT(BIT_AND(b)), OCT(BIT_OR(b)), OCT(BIT_XOR(b)) FROM t1 GROUP BY a; a OCT(BIT_AND(b)) OCT(BIT_OR(b)) OCT(BIT_XOR(b)) -1 null null null +1 1777777777777777777777 0 0 2 362334 362334 362334 3 20 22706573 22706553 4 1777777777777760211044 1777777777777760211044 1777777777777760211044 @@ -162,11 +162,11 @@ invalid argument aggregate function bit_and, bad value [DOUBLE] create table t1(a bigint); select BIT_AND(a),BIT_OR(a), BIT_XOR(a) from t1; BIT_AND(a) BIT_OR(a) BIT_XOR(a) -null null null +18446744073709551615 0 0 insert into t1 values(null),(null),(null),(null); select BIT_AND(a),BIT_OR(a), BIT_XOR(a) from t1; BIT_AND(a) BIT_OR(a) BIT_XOR(a) -null null null +18446744073709551615 0 0 insert into t1 values(12417249128419),(124124125124151),(5124125151415),(124125152651515); select BIT_AND(a),BIT_OR(a), BIT_XOR(a) from t1; BIT_AND(a) BIT_OR(a) BIT_XOR(a) @@ -206,18 +206,22 @@ INSERT INTO t1 VALUES (1,10),(1,20),(2,NULL),(2,NULL),(3,50); select Fld1, BIT_AND(Fld2),BIT_OR(Fld2), BIT_XOR(Fld2) as q from t1 group by Fld1 having q is not null; Fld1 BIT_AND(Fld2) BIT_OR(Fld2) q 1 0 30 30 +2 18446744073709551615 0 0 3 50 50 50 select Fld1, BIT_AND(Fld2),BIT_OR(Fld2), BIT_XOR(Fld2) from t1 group by Fld1 having BIT_AND(Fld2) is not null; Fld1 BIT_AND(Fld2) BIT_OR(Fld2) BIT_XOR(Fld2) 1 0 30 30 +2 18446744073709551615 0 0 3 50 50 50 select Fld1, BIT_AND(Fld2),BIT_OR(Fld2), BIT_XOR(Fld2) from t1 group by Fld1 having BIT_OR(Fld2) is not null; Fld1 BIT_AND(Fld2) BIT_OR(Fld2) BIT_XOR(Fld2) 1 0 30 30 +2 18446744073709551615 0 0 3 50 50 50 select Fld1, BIT_AND(Fld2),BIT_OR(Fld2), BIT_XOR(Fld2) from t1 group by Fld1 having BIT_XOR(Fld2) is not null; Fld1 BIT_AND(Fld2) BIT_OR(Fld2) BIT_XOR(Fld2) 1 0 30 30 +2 18446744073709551615 0 0 3 50 50 50 drop table t1; SELECT BIT_AND(1)BIT_OR(3), BIT_AND(3)>BIT_XOR(5); From a0c41ce557961d1636ab58a1bb9c9cd8c88ceabd Mon Sep 17 00:00:00 2001 From: Cao Kai Date: Wed, 1 Jul 2026 22:31:48 +0800 Subject: [PATCH 04/11] fix: update all BVT results for BIT_AND/BIT_OR neutral values Empty or all-NULL groups now return MySQL-compatible neutral values: - BIT_AND: 18446744073709551615 (all bits set for uint64) - BIT_OR/BIT_XOR: 0 Updated result files: - func_aggr_bitwise.result - func_aggr_avg.result - func_aggr_count.result - func_aggr_max.result - dtype/binary.result Co-Authored-By: Claude --- test/distributed/cases/dtype/binary.result | Bin 43006 -> 43006 bytes .../cases/function/func_aggr_avg.result | 12 ++++++------ .../cases/function/func_aggr_count.result | 12 ++++++------ .../cases/function/func_aggr_max.result | 12 ++++++------ 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/test/distributed/cases/dtype/binary.result b/test/distributed/cases/dtype/binary.result index 9857b49bb6875731908e77755a18a2b3c0f3e60b..f75c6bf205eb3affcc0ab760885e28bdf0b3e07e 100644 GIT binary patch delta 59 zcmex&p6TCtrVYzw1rF~=g_Bpy7Ee}^E1kSiNp$lDx%(DO3=Eqmhu`-A(Paxd*a7Uf B9ee-) delta 62 zcmex&p6TCtrVYzwC+o?IOg=7~45qE*Qh_wj Date: Thu, 2 Jul 2026 12:10:01 +0800 Subject: [PATCH 05/11] fix: preserve argument byte length for BIT_AND/BIT_OR/BIT_XOR neutral values on binary types Address XuPeng-SH review: bitOpExecBytes.Flush() hard-coded an 8-byte 0xFF neutral for BIT_AND and left BIT_OR/BIT_XOR empty, which does not match the binary-string semantics of these aggregates. The neutral value must preserve the argument byte length (BINARY(N)/VARBINARY(N)). Track the argument width on bitOpExecBytes and build the neutral value with bytes.Repeat: width bytes of 0xFF for BIT_AND, width bytes of 0x00 for BIT_OR/BIT_XOR. Add focused unit coverage for BINARY(3)/BINARY(10)/ VARBINARY(4) empty, all-NULL, multi-group and merge paths, and regenerate test/distributed/cases/dtype/binary.result for the binary(10) all-NULL group. Co-Authored-By: Claude Opus 4.8 --- pkg/sql/colexec/aggexec/bitops2.go | 31 +++-- pkg/sql/colexec/aggexec/bitops2_test.go | 132 ++++++++++++++++----- test/distributed/cases/dtype/binary.result | Bin 43006 -> 43030 bytes 3 files changed, 125 insertions(+), 38 deletions(-) diff --git a/pkg/sql/colexec/aggexec/bitops2.go b/pkg/sql/colexec/aggexec/bitops2.go index 926a19e2decfa..a753d6d788f06 100644 --- a/pkg/sql/colexec/aggexec/bitops2.go +++ b/pkg/sql/colexec/aggexec/bitops2.go @@ -15,6 +15,7 @@ package aggexec import ( + "bytes" "math" "slices" @@ -40,6 +41,11 @@ type bitOpExecFixed[T types.Ints | types.UInts] struct { type bitOpExecBytes struct { aggExec op bitOp + // width is the byte length of the argument type (BINARY(N)/VARBINARY(N)). + // It drives the length of the neutral value produced for empty or + // all-NULL groups so the result preserves the argument's byte length, + // matching MySQL binary-string aggregate semantics. + width int32 } func (op bitOp) compute(a, b uint64) uint64 { @@ -254,25 +260,31 @@ func (exec *bitOpExecBytes) SetExtraInformation(partialResult any, _ int) error func (exec *bitOpExecBytes) Flush() ([]*vector.Vector, error) { // transfer vector to result vecs := make([]*vector.Vector, len(exec.state)) - neutralBytesForAnd := []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF} + + // Neutral values for empty or all-NULL groups, per MySQL binary-string + // aggregate semantics. The neutral value has the same byte length as the + // argument type (BINARY(N)/VARBINARY(N)), driven by exec.width: + // BIT_AND: all bits set -> width bytes of 0xFF + // BIT_OR / BIT_XOR: all bits clear -> width bytes of 0x00 + var neutral []byte + if exec.op == bitAnd { + neutral = bytes.Repeat([]byte{0xFF}, int(exec.width)) + } else { + neutral = bytes.Repeat([]byte{0x00}, int(exec.width)) + } + for i := range vecs { vecs[i] = exec.state[i].vecs[0] exec.state[i].vecs[0] = nil exec.state[i].length = 0 exec.state[i].capacity = 0 - // Replace NULL entries with neutral values per MySQL semantics: - // BIT_AND: neutral is all bits set (8 bytes of 0xFF) - // BIT_OR, BIT_XOR: neutral is empty bytes for j := 0; j < vecs[i].Length(); j++ { if vecs[i].IsNull(uint64(j)) { vecs[i].UnsetNull(uint64(j)) - if exec.op == bitAnd { - if err := vector.SetBytesAt(vecs[i], j, neutralBytesForAnd, exec.mp); err != nil { - return nil, err - } + if err := vector.SetBytesAt(vecs[i], j, neutral, exec.mp); err != nil { + return nil, err } - // For bitOr and bitXor, the default zero bytes (empty) is already the neutral value } } } @@ -299,6 +311,7 @@ func makeBitOpExecBytes(mp *mpool.MPool, id int64, param types.Type, op bitOp) A var bitOp bitOpExecBytes bitOp.mp = mp bitOp.op = op + bitOp.width = param.Width bitOp.aggInfo = aggInfo{ aggId: id, isDistinct: false, diff --git a/pkg/sql/colexec/aggexec/bitops2_test.go b/pkg/sql/colexec/aggexec/bitops2_test.go index bce3f9218a49d..116389f448f76 100644 --- a/pkg/sql/colexec/aggexec/bitops2_test.go +++ b/pkg/sql/colexec/aggexec/bitops2_test.go @@ -242,9 +242,8 @@ func TestBitOpsMultipleGroups(t *testing.T) { func TestBitOpsBytes(t *testing.T) { mp := mpool.MustNewZero() - typ := types.T_varbinary.ToType() - makeInputVec := func(values [][]byte, nulls []bool) *vector.Vector { + makeInputVec := func(typ types.Type, values [][]byte, nulls []bool) *vector.Vector { vec := vector.NewVec(typ) for i, v := range values { isNull := false @@ -256,9 +255,19 @@ func TestBitOpsBytes(t *testing.T) { return vec } + // bytesN builds a slice of n bytes filled with b. + bytesN := func(b byte, n int) []byte { + out := make([]byte, n) + for i := range out { + out[i] = b + } + return out + } + t.Run("normal values byte bit_or", func(t *testing.T) { + typ := types.New(types.T_binary, 1, 0) // BIT_OR: 0x01 | 0x02 | 0x04 = 0x07 - vec := makeInputVec([][]byte{{0x01}, {0x02}, {0x04}}, nil) + vec := makeInputVec(typ, [][]byte{{0x01}, {0x02}, {0x04}}, nil) defer vec.Free(mp) exec := makeBitAggExec(mp, AggIdOfBitOr, typ) @@ -276,31 +285,86 @@ func TestBitOpsBytes(t *testing.T) { } }) - t.Run("empty group byte bit_or returns empty", func(t *testing.T) { - exec := makeBitAggExec(mp, AggIdOfBitOr, typ) - require.NoError(t, exec.GroupGrow(1)) + // Neutral values for empty or all-NULL groups must preserve the argument + // byte length: BIT_AND -> width bytes of 0xFF, BIT_OR/BIT_XOR -> width + // bytes of 0x00. Cover both BINARY(N) and VARBINARY(N) for several widths. + for _, dim := range []struct { + name string + oid types.T + width int32 + }{ + {"binary(3)", types.T_binary, 3}, + {"binary(10)", types.T_binary, 10}, + {"varbinary(4)", types.T_varbinary, 4}, + } { + typ := types.New(dim.oid, dim.width, 0) + w := int(dim.width) - results, err := exec.Flush() - require.NoError(t, err) - require.Len(t, results, 1) - require.False(t, results[0].IsNull(0)) - require.Equal(t, []byte{}, results[0].GetBytesAt(0)) + for _, tc := range []struct { + name string + aggID int64 + expect []byte + }{ + {"bit_and", AggIdOfBitAnd, bytesN(0xFF, w)}, + {"bit_or", AggIdOfBitOr, bytesN(0x00, w)}, + {"bit_xor", AggIdOfBitXor, bytesN(0x00, w)}, + } { + t.Run(dim.name+" empty group "+tc.name, func(t *testing.T) { + exec := makeBitAggExec(mp, tc.aggID, typ) + require.NoError(t, exec.GroupGrow(1)) - exec.Free() - for _, r := range results { - r.Free(mp) + results, err := exec.Flush() + require.NoError(t, err) + require.Len(t, results, 1) + require.False(t, results[0].IsNull(0)) + require.Equal(t, tc.expect, results[0].GetBytesAt(0)) + + exec.Free() + for _, r := range results { + r.Free(mp) + } + }) + + t.Run(dim.name+" all-NULL group "+tc.name, func(t *testing.T) { + vec := makeInputVec(typ, [][]byte{{0x01}, {0x02}}, []bool{true, true}) + defer vec.Free(mp) + + exec := makeBitAggExec(mp, tc.aggID, typ) + require.NoError(t, exec.GroupGrow(1)) + require.NoError(t, exec.BulkFill(0, []*vector.Vector{vec})) + + results, err := exec.Flush() + require.NoError(t, err) + require.Len(t, results, 1) + require.False(t, results[0].IsNull(0)) + require.Equal(t, tc.expect, results[0].GetBytesAt(0)) + + exec.Free() + for _, r := range results { + r.Free(mp) + } + }) } - }) + } - t.Run("empty group byte bit_and returns all-ones", func(t *testing.T) { - exec := makeBitAggExec(mp, AggIdOfBitAnd, typ) - require.NoError(t, exec.GroupGrow(1)) + t.Run("multiple groups one empty preserves width", func(t *testing.T) { + typ := types.New(types.T_binary, 3, 0) + // group 1 has values, group 2 is empty -> neutral, group 3 has values + vec := makeInputVec(typ, [][]byte{{0x0F, 0x0F, 0x0F}, {0xF0, 0xF0, 0xF0}}, nil) + defer vec.Free(mp) + groups := []uint64{1, 3} + + exec := makeBitAggExec(mp, AggIdOfBitOr, typ) + require.NoError(t, exec.GroupGrow(3)) + require.NoError(t, exec.BatchFill(0, groups, []*vector.Vector{vec})) results, err := exec.Flush() require.NoError(t, err) require.Len(t, results, 1) - require.False(t, results[0].IsNull(0)) - require.Equal(t, []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}, results[0].GetBytesAt(0)) + require.Equal(t, []byte{0x0F, 0x0F, 0x0F}, results[0].GetBytesAt(0)) + require.False(t, results[0].IsNull(1)) + require.Equal(t, bytesN(0x00, 3), results[0].GetBytesAt(1)) + require.Equal(t, []byte{0xF0, 0xF0, 0xF0}, results[0].GetBytesAt(2)) exec.Free() for _, r := range results { @@ -308,21 +372,31 @@ func TestBitOpsBytes(t *testing.T) { } }) - t.Run("all-NULL byte bit_xor returns empty", func(t *testing.T) { - vec := makeInputVec([][]byte{{0x01}, {0x02}}, []bool{true, true}) - defer vec.Free(mp) + // Merging an all-NULL group into an empty group and then flushing must + // still produce a width-length neutral value (covers the bytes Merge path). + t.Run("merge all-NULL groups preserves neutral width", func(t *testing.T) { + typ := types.New(types.T_binary, 4, 0) - exec := makeBitAggExec(mp, AggIdOfBitXor, typ) - require.NoError(t, exec.GroupGrow(1)) - require.NoError(t, exec.BulkFill(0, []*vector.Vector{vec})) + exec1 := makeBitAggExec(mp, AggIdOfBitAnd, typ) + require.NoError(t, exec1.GroupGrow(1)) + exec2 := makeBitAggExec(mp, AggIdOfBitAnd, typ) + require.NoError(t, exec2.GroupGrow(1)) + // feed exec2 an all-NULL row so its single group stays NULL + nullVec := makeInputVec(typ, [][]byte{{0x01}}, []bool{true}) + defer nullVec.Free(mp) + require.NoError(t, exec2.BulkFill(0, []*vector.Vector{nullVec})) - results, err := exec.Flush() + // merge exec2's group 0 into exec1's group 0 + require.NoError(t, exec1.Merge(exec2, 0, 0)) + + results, err := exec1.Flush() require.NoError(t, err) require.Len(t, results, 1) require.False(t, results[0].IsNull(0)) - require.Equal(t, []byte{}, results[0].GetBytesAt(0)) + require.Equal(t, bytesN(0xFF, 4), results[0].GetBytesAt(0)) - exec.Free() + exec1.Free() + exec2.Free() for _, r := range results { r.Free(mp) } diff --git a/test/distributed/cases/dtype/binary.result b/test/distributed/cases/dtype/binary.result index f75c6bf205eb3affcc0ab760885e28bdf0b3e07e..e8f817bfcd4276684f0e47c78e1108e768fd8e6b 100644 GIT binary patch delta 42 vcmex&o@v?zrVZO=CvTQ5XJ%kvnEX&)c(R#XCYZfWNoI1iJlE!Zat@sUS1}HN delta 26 icmbPsf$86QrVZO=C-0Umo@^mkIypc|bn_WGuTB7{n+t*f From 3397d677dfe0bb10c6a0e2918a29c1ea0360a876 Mon Sep 17 00:00:00 2001 From: Cao Kai Date: Thu, 2 Jul 2026 14:41:09 +0800 Subject: [PATCH 06/11] fix: free transferred vectors on SetBytesAt failure in bitOpExecBytes.Flush MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When vector.SetBytesAt fails midway (e.g. OOM on varlena allocation), the vectors already transferred from exec.state become orphaned — the caller receives nil, err and cannot free them. Free vecs[0..i] before returning the error. Co-Authored-By: Claude Opus 4.8 --- pkg/sql/colexec/aggexec/bitops2.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/sql/colexec/aggexec/bitops2.go b/pkg/sql/colexec/aggexec/bitops2.go index a753d6d788f06..d2f72ad6874d5 100644 --- a/pkg/sql/colexec/aggexec/bitops2.go +++ b/pkg/sql/colexec/aggexec/bitops2.go @@ -283,6 +283,13 @@ func (exec *bitOpExecBytes) Flush() ([]*vector.Vector, error) { if vecs[i].IsNull(uint64(j)) { vecs[i].UnsetNull(uint64(j)) if err := vector.SetBytesAt(vecs[i], j, neutral, exec.mp); err != nil { + // Ownership of vecs[0..i] has been transferred from + // exec.state — free them before returning to avoid leaks. + for k := 0; k <= i; k++ { + if vecs[k] != nil { + vecs[k].Free(exec.mp) + } + } return nil, err } } From 757599db98fffb1eb55224ec13475fc0b44d6e03 Mon Sep 17 00:00:00 2001 From: Cao Kai Date: Fri, 3 Jul 2026 14:46:48 +0800 Subject: [PATCH 07/11] fix: check SetBytesAt error before UnsetNull in BatchFill/BatchMerge In bitOpExecBytes.BatchFill and BatchMerge, the first non-NULL binary value was written by calling UnsetNull BEFORE SetBytesAt. If SetBytesAt fails (e.g. OOM on varlena off-heap allocation), the state slot is left marked non-NULL with an empty/default payload. Flush() then skips neutral replacement and returns a wrong result instead of propagating the error. Fix: call SetBytesAt first, check its error, and only UnsetNull after the write succeeds. Add regression coverage for both unhappy paths using a capped mpool to force SetBytesAt failure, verifying the state stays NULL. Co-Authored-By: Claude Opus 4.8 --- pkg/sql/colexec/aggexec/bitops2.go | 10 ++- pkg/sql/colexec/aggexec/bitops2_test.go | 88 +++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/pkg/sql/colexec/aggexec/bitops2.go b/pkg/sql/colexec/aggexec/bitops2.go index d2f72ad6874d5..1454b498bf64a 100644 --- a/pkg/sql/colexec/aggexec/bitops2.go +++ b/pkg/sql/colexec/aggexec/bitops2.go @@ -204,8 +204,10 @@ func (exec *bitOpExecBytes) BatchFill(offset int, groups []uint64, vectors []*ve x, y := exec.getXY(grp - 1) value := vectors[0].GetBytesAt(int(idx)) if exec.state[x].vecs[0].IsNull(uint64(y)) { + if err := vector.SetBytesAt(exec.state[x].vecs[0], int(y), value, exec.mp); err != nil { + return err + } exec.state[x].vecs[0].UnsetNull(uint64(y)) - vector.SetBytesAt(exec.state[x].vecs[0], int(y), value, exec.mp) } else { oldValue := exec.state[x].vecs[0].GetBytesAt(int(y)) // computeBytes will update oldValue in place @@ -237,9 +239,11 @@ func (exec *bitOpExecBytes) BatchMerge(next AggFuncExec, offset int, groups []ui continue } if exec.state[x1].vecs[0].IsNull(uint64(y1)) { - exec.state[x1].vecs[0].UnsetNull(uint64(y1)) otherValue := other.state[x2].vecs[0].GetBytesAt(int(y2)) - vector.SetBytesAt(exec.state[x1].vecs[0], int(y1), otherValue, exec.mp) + if err := vector.SetBytesAt(exec.state[x1].vecs[0], int(y1), otherValue, exec.mp); err != nil { + return err + } + exec.state[x1].vecs[0].UnsetNull(uint64(y1)) } else { oldValue := exec.state[x1].vecs[0].GetBytesAt(int(y1)) otherValue := other.state[x2].vecs[0].GetBytesAt(int(y2)) diff --git a/pkg/sql/colexec/aggexec/bitops2_test.go b/pkg/sql/colexec/aggexec/bitops2_test.go index 116389f448f76..82c5dc3eecfbb 100644 --- a/pkg/sql/colexec/aggexec/bitops2_test.go +++ b/pkg/sql/colexec/aggexec/bitops2_test.go @@ -401,4 +401,92 @@ func TestBitOpsBytes(t *testing.T) { r.Free(mp) } }) + + // When SetBytesAt fails during BulkFill (e.g. OOM on varlena allocation), + // the state must stay NULL and the error must propagate — never leave a + // non-NULL slot with a default/empty payload. + t.Run("BulkFill OOM leaves state NULL", func(t *testing.T) { + // Set a global cap large enough to create a capped mpool below. + mpool.InitCap(100 * 1024 * 1024) // 100 MiB + capped, err := mpool.NewMPool("test", 2*1024*1024, 0) // 2 MiB + require.NoError(t, err) + + // BINARY(64) > VarlenaInlineSize (23): forces non-inline varlena path. + typ := types.New(types.T_binary, 64, 0) + exec := makeBitAggExec(capped, AggIdOfBitAnd, typ) + require.NoError(t, exec.GroupGrow(1)) + + // Exhaust the off-heap capacity by repeatedly allocating + // until the pool is full. Any subsequent SetBytesAt must + // then fail. + for { + if _, e := capped.Alloc(4096, true); e != nil { + break + } + } + + // Input vector from a normal (unlimited) mpool. + inputVec := vector.NewVec(typ) + val := make([]byte, 64) + for i := range val { + val[i] = byte(i) + } + require.NoError(t, vector.AppendBytes(inputVec, val, false, mp)) + defer inputVec.Free(mp) + + err = exec.BulkFill(0, []*vector.Vector{inputVec}) + require.Error(t, err, "BulkFill should fail when SetBytesAt cannot allocate") + + // No Flush — the mpool is exhausted even for neutral replacement. + // Verify the state slot is still NULL directly (UnsetNull must + // not have been called before SetBytesAt). + require.True(t, exec.(*bitOpExecBytes).state[0].vecs[0].IsNull(0), + "state slot must remain NULL after failed SetBytesAt") + + exec.Free() + }) + + // Same contract for Merge: a failed SetBytesAt while copying the first + // non-NULL value into an empty target group must leave the target NULL. + t.Run("Merge OOM leaves state NULL", func(t *testing.T) { + mpool.InitCap(100 * 1024 * 1024) // 100 MiB + capped, err := mpool.NewMPool("test", 2*1024*1024, 0) // 2 MiB + require.NoError(t, err) + + typ := types.New(types.T_binary, 64, 0) + + // exec1: empty target (all-NULL), uses capped mpool. + exec1 := makeBitAggExec(capped, AggIdOfBitAnd, typ) + require.NoError(t, exec1.GroupGrow(1)) + + // exec2: has a real value, uses normal (unlimited) mpool. + exec2 := makeBitAggExec(mp, AggIdOfBitAnd, typ) + require.NoError(t, exec2.GroupGrow(1)) + inputVec := vector.NewVec(typ) + val := make([]byte, 64) + for i := range val { + val[i] = byte(i) + } + require.NoError(t, vector.AppendBytes(inputVec, val, false, mp)) + defer inputVec.Free(mp) + require.NoError(t, exec2.BulkFill(0, []*vector.Vector{inputVec})) + + // Exhaust the off-heap capacity by repeatedly allocating + // until the pool is full. + for { + if _, e := capped.Alloc(4096, true); e != nil { + break + } + } + + err = exec1.Merge(exec2, 0, 0) + require.Error(t, err, "Merge should fail when SetBytesAt cannot allocate") + + // Verify the state slot is still NULL directly. + require.True(t, exec1.(*bitOpExecBytes).state[0].vecs[0].IsNull(0), + "target slot must remain NULL after failed Merge SetBytesAt") + + exec1.Free() + exec2.Free() + }) } From af2bd0792a8bf1bd2f3d2c122b4e1e8f2cf3f3b8 Mon Sep 17 00:00:00 2001 From: Cao Kai Date: Sat, 4 Jul 2026 20:25:04 +0800 Subject: [PATCH 08/11] fix: address review feedback for OOM test resource leaks, VARBINARY OOM coverage, and hex/length BVT coverage - P2: Save/restore global mpool cap in OOM tests, free exhaustion allocations - P3b: Add VARBINARY(64) OOM coverage alongside BINARY(64) via table-driven tests - P3a: Add hex(bit_and(b))/length(bit_and(b)) queries to binary BVT for neutral value verification - P3c: Regenerate binary.result with ASCII-only hex output (no NUL byte pollution) Co-Authored-By: Claude Opus 4.8 --- pkg/sql/colexec/aggexec/bitops2_test.go | 164 +++++++++++++-------- test/distributed/cases/dtype/binary.result | Bin 43030 -> 43755 bytes test/distributed/cases/dtype/binary.test | 5 + 3 files changed, 104 insertions(+), 65 deletions(-) diff --git a/pkg/sql/colexec/aggexec/bitops2_test.go b/pkg/sql/colexec/aggexec/bitops2_test.go index 82c5dc3eecfbb..6dda15a26ccb3 100644 --- a/pkg/sql/colexec/aggexec/bitops2_test.go +++ b/pkg/sql/colexec/aggexec/bitops2_test.go @@ -406,87 +406,121 @@ func TestBitOpsBytes(t *testing.T) { // the state must stay NULL and the error must propagate — never leave a // non-NULL slot with a default/empty payload. t.Run("BulkFill OOM leaves state NULL", func(t *testing.T) { - // Set a global cap large enough to create a capped mpool below. - mpool.InitCap(100 * 1024 * 1024) // 100 MiB - capped, err := mpool.NewMPool("test", 2*1024*1024, 0) // 2 MiB - require.NoError(t, err) + for _, dim := range []struct { + name string + oid types.T + width int32 + }{ + {"binary(64)", types.T_binary, 64}, + {"varbinary(64)", types.T_varbinary, 64}, + } { + t.Run(dim.name, func(t *testing.T) { + origCap := mpool.GlobalCap() + mpool.InitCap(100 * 1024 * 1024) // 100 MiB + defer mpool.InitCap(origCap) + capped, err := mpool.NewMPool("test", 2*1024*1024, 0) // 2 MiB + require.NoError(t, err) - // BINARY(64) > VarlenaInlineSize (23): forces non-inline varlena path. - typ := types.New(types.T_binary, 64, 0) - exec := makeBitAggExec(capped, AggIdOfBitAnd, typ) - require.NoError(t, exec.GroupGrow(1)) + typ := types.New(dim.oid, dim.width, 0) + exec := makeBitAggExec(capped, AggIdOfBitAnd, typ) + require.NoError(t, exec.GroupGrow(1)) - // Exhaust the off-heap capacity by repeatedly allocating - // until the pool is full. Any subsequent SetBytesAt must - // then fail. - for { - if _, e := capped.Alloc(4096, true); e != nil { - break - } - } + // Exhaust the off-heap capacity. Track allocations so + // they can be freed after the test. + var exhaust [][]byte + for { + b, e := capped.Alloc(4096, true) + if e != nil { + break + } + exhaust = append(exhaust, b) + } - // Input vector from a normal (unlimited) mpool. - inputVec := vector.NewVec(typ) - val := make([]byte, 64) - for i := range val { - val[i] = byte(i) - } - require.NoError(t, vector.AppendBytes(inputVec, val, false, mp)) - defer inputVec.Free(mp) + // Input vector from a normal (unlimited) mpool. + inputVec := vector.NewVec(typ) + val := make([]byte, 64) + for i := range val { + val[i] = byte(i) + } + require.NoError(t, vector.AppendBytes(inputVec, val, false, mp)) + defer inputVec.Free(mp) - err = exec.BulkFill(0, []*vector.Vector{inputVec}) - require.Error(t, err, "BulkFill should fail when SetBytesAt cannot allocate") + err = exec.BulkFill(0, []*vector.Vector{inputVec}) + require.Error(t, err, "BulkFill should fail when SetBytesAt cannot allocate") - // No Flush — the mpool is exhausted even for neutral replacement. - // Verify the state slot is still NULL directly (UnsetNull must - // not have been called before SetBytesAt). - require.True(t, exec.(*bitOpExecBytes).state[0].vecs[0].IsNull(0), - "state slot must remain NULL after failed SetBytesAt") + // Verify the state slot is still NULL directly. + require.True(t, exec.(*bitOpExecBytes).state[0].vecs[0].IsNull(0), + "state slot must remain NULL after failed SetBytesAt") - exec.Free() + // Clean up exhausted allocations before freeing the exec. + for _, b := range exhaust { + capped.Free(b) + } + exec.Free() + }) + } }) // Same contract for Merge: a failed SetBytesAt while copying the first // non-NULL value into an empty target group must leave the target NULL. t.Run("Merge OOM leaves state NULL", func(t *testing.T) { - mpool.InitCap(100 * 1024 * 1024) // 100 MiB - capped, err := mpool.NewMPool("test", 2*1024*1024, 0) // 2 MiB - require.NoError(t, err) + for _, dim := range []struct { + name string + oid types.T + width int32 + }{ + {"binary(64)", types.T_binary, 64}, + {"varbinary(64)", types.T_varbinary, 64}, + } { + t.Run(dim.name, func(t *testing.T) { + origCap := mpool.GlobalCap() + mpool.InitCap(100 * 1024 * 1024) // 100 MiB + defer mpool.InitCap(origCap) + capped, err := mpool.NewMPool("test", 2*1024*1024, 0) // 2 MiB + require.NoError(t, err) - typ := types.New(types.T_binary, 64, 0) + typ := types.New(dim.oid, dim.width, 0) - // exec1: empty target (all-NULL), uses capped mpool. - exec1 := makeBitAggExec(capped, AggIdOfBitAnd, typ) - require.NoError(t, exec1.GroupGrow(1)) + // exec1: empty target (all-NULL), uses capped mpool. + exec1 := makeBitAggExec(capped, AggIdOfBitAnd, typ) + require.NoError(t, exec1.GroupGrow(1)) - // exec2: has a real value, uses normal (unlimited) mpool. - exec2 := makeBitAggExec(mp, AggIdOfBitAnd, typ) - require.NoError(t, exec2.GroupGrow(1)) - inputVec := vector.NewVec(typ) - val := make([]byte, 64) - for i := range val { - val[i] = byte(i) - } - require.NoError(t, vector.AppendBytes(inputVec, val, false, mp)) - defer inputVec.Free(mp) - require.NoError(t, exec2.BulkFill(0, []*vector.Vector{inputVec})) - - // Exhaust the off-heap capacity by repeatedly allocating - // until the pool is full. - for { - if _, e := capped.Alloc(4096, true); e != nil { - break - } - } + // exec2: has a real value, uses normal (unlimited) mpool. + exec2 := makeBitAggExec(mp, AggIdOfBitAnd, typ) + require.NoError(t, exec2.GroupGrow(1)) + inputVec := vector.NewVec(typ) + val := make([]byte, 64) + for i := range val { + val[i] = byte(i) + } + require.NoError(t, vector.AppendBytes(inputVec, val, false, mp)) + defer inputVec.Free(mp) + require.NoError(t, exec2.BulkFill(0, []*vector.Vector{inputVec})) + + // Exhaust the off-heap capacity. + var exhaust [][]byte + for { + b, e := capped.Alloc(4096, true) + if e != nil { + break + } + exhaust = append(exhaust, b) + } - err = exec1.Merge(exec2, 0, 0) - require.Error(t, err, "Merge should fail when SetBytesAt cannot allocate") + err = exec1.Merge(exec2, 0, 0) + require.Error(t, err, "Merge should fail when SetBytesAt cannot allocate") - // Verify the state slot is still NULL directly. - require.True(t, exec1.(*bitOpExecBytes).state[0].vecs[0].IsNull(0), - "target slot must remain NULL after failed Merge SetBytesAt") + // Verify the state slot is still NULL directly. + require.True(t, exec1.(*bitOpExecBytes).state[0].vecs[0].IsNull(0), + "target slot must remain NULL after failed Merge SetBytesAt") - exec1.Free() - exec2.Free() + // Clean up exhausted allocations. + for _, b := range exhaust { + capped.Free(b) + } + exec1.Free() + exec2.Free() + }) + } }) } diff --git a/test/distributed/cases/dtype/binary.result b/test/distributed/cases/dtype/binary.result index e8f817bfcd4276684f0e47c78e1108e768fd8e6b..798cdef3e23511532c91f2e535ef795f6e7e523c 100644 GIT binary patch delta 636 zcmbPsf$8;CrVab$COgQnC}pHpXe4Ep#3$yZXe4QB>L}!-=B1ZpAc;*rFRP%9rc40{ zFcfNXC4$5!^Q(x~18GA8t|SoO*Z>F(4Nw3`94Ma*l{Yj-ku@-Y%BO&6H!J`uU!0ng znp~ognWCcrb6|cE$Qwc^{s4=Cyy1ze3SdYF%3vOqJDyaW}6_{qS~z{J1^ p)ge$Pfc*rNL5n7MMBt1lRj2{rm_jug!UM}s_E!?yd`8Zz696P^pacK_ delta 18 acmaETm1)`qrVab$Ca+TE-MmoIs}lfN5eRPp diff --git a/test/distributed/cases/dtype/binary.test b/test/distributed/cases/dtype/binary.test index 3549b0a7166df..296f8044aa7d8 100644 --- a/test/distributed/cases/dtype/binary.test +++ b/test/distributed/cases/dtype/binary.test @@ -163,6 +163,11 @@ select id, bit_and(b) from bit01 group by id; select id, bit_or(b) from bit01 group by id; select id, bit_xor(b) from bit01 group by id; +-- hex/length for binary aggregate neutral values +select id, hex(bit_and(b)), length(bit_and(b)) from bit01 group by id; +select id, hex(bit_or(b)), length(bit_or(b)) from bit01 group by id; +select id, hex(bit_xor(b)), length(bit_xor(b)) from bit01 group by id; + -- order by select * from bit01 order by b desc; select * from bit01 order by b; From 26eeb8c70a3bca6a4b74968072389764fac3ab69 Mon Sep 17 00:00:00 2001 From: Cao Kai Date: Sun, 5 Jul 2026 15:57:17 +0800 Subject: [PATCH 09/11] fix: use t.Cleanup for OOM test teardown and add empty-table/mixed-NULL BVT coverage - OOM tests now register all teardown via t.Cleanup (LIFO), so cleanup runs even when a require assertion aborts the subtest, and the capped mpool is released via mpool.DeleteMPool instead of leaking in globalPools - Add bit02 BVT cases covering zero-row aggregate without GROUP BY (returns neutral values) and same-group mixed NULL/non-NULL rows Co-Authored-By: Claude Opus 4.8 --- pkg/sql/colexec/aggexec/bitops2_test.go | 42 +++++++++++---------- test/distributed/cases/dtype/binary.result | Bin 43755 -> 45057 bytes test/distributed/cases/dtype/binary.test | 19 ++++++++++ 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/pkg/sql/colexec/aggexec/bitops2_test.go b/pkg/sql/colexec/aggexec/bitops2_test.go index 6dda15a26ccb3..074ff52da84bf 100644 --- a/pkg/sql/colexec/aggexec/bitops2_test.go +++ b/pkg/sql/colexec/aggexec/bitops2_test.go @@ -417,17 +417,25 @@ func TestBitOpsBytes(t *testing.T) { t.Run(dim.name, func(t *testing.T) { origCap := mpool.GlobalCap() mpool.InitCap(100 * 1024 * 1024) // 100 MiB - defer mpool.InitCap(origCap) + t.Cleanup(func() { mpool.InitCap(origCap) }) capped, err := mpool.NewMPool("test", 2*1024*1024, 0) // 2 MiB require.NoError(t, err) + t.Cleanup(func() { mpool.DeleteMPool(capped) }) typ := types.New(dim.oid, dim.width, 0) exec := makeBitAggExec(capped, AggIdOfBitAnd, typ) + t.Cleanup(exec.Free) require.NoError(t, exec.GroupGrow(1)) // Exhaust the off-heap capacity. Track allocations so - // they can be freed after the test. + // they can be freed after the test (t.Cleanup runs LIFO, + // so they are released before exec.Free and DeleteMPool). var exhaust [][]byte + t.Cleanup(func() { + for _, b := range exhaust { + capped.Free(b) + } + }) for { b, e := capped.Alloc(4096, true) if e != nil { @@ -438,12 +446,12 @@ func TestBitOpsBytes(t *testing.T) { // Input vector from a normal (unlimited) mpool. inputVec := vector.NewVec(typ) + t.Cleanup(func() { inputVec.Free(mp) }) val := make([]byte, 64) for i := range val { val[i] = byte(i) } require.NoError(t, vector.AppendBytes(inputVec, val, false, mp)) - defer inputVec.Free(mp) err = exec.BulkFill(0, []*vector.Vector{inputVec}) require.Error(t, err, "BulkFill should fail when SetBytesAt cannot allocate") @@ -451,12 +459,6 @@ func TestBitOpsBytes(t *testing.T) { // Verify the state slot is still NULL directly. require.True(t, exec.(*bitOpExecBytes).state[0].vecs[0].IsNull(0), "state slot must remain NULL after failed SetBytesAt") - - // Clean up exhausted allocations before freeing the exec. - for _, b := range exhaust { - capped.Free(b) - } - exec.Free() }) } }) @@ -475,30 +477,39 @@ func TestBitOpsBytes(t *testing.T) { t.Run(dim.name, func(t *testing.T) { origCap := mpool.GlobalCap() mpool.InitCap(100 * 1024 * 1024) // 100 MiB - defer mpool.InitCap(origCap) + t.Cleanup(func() { mpool.InitCap(origCap) }) capped, err := mpool.NewMPool("test", 2*1024*1024, 0) // 2 MiB require.NoError(t, err) + t.Cleanup(func() { mpool.DeleteMPool(capped) }) typ := types.New(dim.oid, dim.width, 0) // exec1: empty target (all-NULL), uses capped mpool. exec1 := makeBitAggExec(capped, AggIdOfBitAnd, typ) + t.Cleanup(exec1.Free) require.NoError(t, exec1.GroupGrow(1)) // exec2: has a real value, uses normal (unlimited) mpool. exec2 := makeBitAggExec(mp, AggIdOfBitAnd, typ) + t.Cleanup(exec2.Free) require.NoError(t, exec2.GroupGrow(1)) inputVec := vector.NewVec(typ) + t.Cleanup(func() { inputVec.Free(mp) }) val := make([]byte, 64) for i := range val { val[i] = byte(i) } require.NoError(t, vector.AppendBytes(inputVec, val, false, mp)) - defer inputVec.Free(mp) require.NoError(t, exec2.BulkFill(0, []*vector.Vector{inputVec})) - // Exhaust the off-heap capacity. + // Exhaust the off-heap capacity. Freed via t.Cleanup (LIFO) + // before the execs and the capped pool are torn down. var exhaust [][]byte + t.Cleanup(func() { + for _, b := range exhaust { + capped.Free(b) + } + }) for { b, e := capped.Alloc(4096, true) if e != nil { @@ -513,13 +524,6 @@ func TestBitOpsBytes(t *testing.T) { // Verify the state slot is still NULL directly. require.True(t, exec1.(*bitOpExecBytes).state[0].vecs[0].IsNull(0), "target slot must remain NULL after failed Merge SetBytesAt") - - // Clean up exhausted allocations. - for _, b := range exhaust { - capped.Free(b) - } - exec1.Free() - exec2.Free() }) } }) diff --git a/test/distributed/cases/dtype/binary.result b/test/distributed/cases/dtype/binary.result index 798cdef3e23511532c91f2e535ef795f6e7e523c..d9c7c90ada811c911b952cce27509b11c5e0a44b 100644 GIT binary patch delta 359 zcmaETm8tOo(}rHf$%~@I#Y+r~ththlQWHy36-pA5a#9tNGJ#@~1Epoyic@n^lS?Ka zlG9@bDw@nHuQqw2yfB}e0uUG)a6zTF%Ik0gr8811G=N&;Cx1{7WzR3tNYd2gnq04- zG1vMaFS}-%OIJKxmAv3QeA8J>fw5pnsLRn%?X=<^CdZN0HLSAW3j;1vi zrqU%SN|W#@6_ZgFL718hS2}sSoF${thbeeP1 delta 26 icmZpC!1Vem(}rHf$@^5 Date: Sun, 5 Jul 2026 17:56:19 +0800 Subject: [PATCH 10/11] fix: restore literal \0\0 in binary.result and add ORDER BY id to new GROUP BY cases - Revert two unrelated NUL drifts: the column headers for `c = 'a\0\0'` in the binary(3)/varbinary(3) tests were regenerated into real NUL bytes; restore them to the literal `\0\0` text - Add ORDER BY id to the six new hex/length GROUP BY queries (bit01 and bit02) so row order is deterministic Co-Authored-By: Claude Opus 4.8 --- test/distributed/cases/dtype/binary.result | Bin 45057 -> 45133 bytes test/distributed/cases/dtype/binary.test | 12 ++++++------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/distributed/cases/dtype/binary.result b/test/distributed/cases/dtype/binary.result index d9c7c90ada811c911b952cce27509b11c5e0a44b..31b3312422e4cd3cc53bb6f3c2b43c3342333c97 100644 GIT binary patch delta 117 zcmZpCz;yNj(}urtYzp~BDXB%1qgAXXKhhDN?4_nNxj?=SCaj}sHCahvFO2(F)oQb_ nq8~GiAFXm7#?(=TF*pBJooUGuV-RDoc}Dzw4+yJrMF%?oDi$lp delta 103 zcmX^6fT{5T(}urtljCJMCi5w9OqP^?2xY&7i$$xcY&KW)W1h^WAOKXLGWn>=Tp-(I qa;TQz Date: Tue, 7 Jul 2026 11:51:13 +0800 Subject: [PATCH 11/11] fix: regenerate binary.result to include main's binary_hex_padding/width results The merge resolution kept the branch's binary.result, dropping results for #25424 test cases (binary_hex_padding, binary_hex_width). Regenerate with mo-tester to include both the branch's neutral-value coverage and main's hex padding/width test results. Co-Authored-By: Claude Opus 4.8 --- test/distributed/cases/dtype/binary.result | Bin 45133 -> 46127 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/test/distributed/cases/dtype/binary.result b/test/distributed/cases/dtype/binary.result index 31b3312422e4cd3cc53bb6f3c2b43c3342333c97..20e140b235b258a32ab52b2d3a879a910275feb3 100644 GIT binary patch delta 777 zcmZ`%%TB^T6pe_6bfpUx#0S@nw5E%;ve2+XW8C@&&@ux|EG?N16%v(=e=uL+N)kW8 zwg2GKoqU4xC|HQhqI2imJLle>GoN3Dx9`I2sNHS#@wheg5h78qVRR_=83yJDA%=iN zKAq$Xz$6Gr#9&CtK;+;aPizRXO0#a+kOq+YR9{s{Q&!s4g0yy0w6MAc<_l==L{c)w_u?|uQN*8h_L delta 14 WcmZ4gg6Zr7rVX+QoBt;p1ONaz00#>I