fix: BIT_AND/BIT_OR/BIT_XOR return neutral values for empty or all-NULL groups

[ck89119]

What type of PR is this?

• Bug fix

Which issue(s) this PR fixes:

Fixes #25285

What this PR does / why we need it:

BIT_AND(), BIT_OR(), BIT_XOR() now return MySQL-compatible neutral values for groups with no non-NULL input values:

Function	Empty/All-NULL group result
BIT_AND	all bits set (max uint64)
BIT_OR	0
BIT_XOR	0

Previously these returned NULL for empty or all-NULL groups, which differs from MySQL behavior.

The fix modifies the Flush() methods to replace NULL entries with the appropriate neutral value (identity element for each bitwise operation).

Special notes for your reviewer:

• Modified files: pkg/sql/colexec/aggexec/bitops2.go, pkg/sql/colexec/aggexec/bitops2_test.go
• Test coverage: 12 test cases covering normal values, empty groups, all-NULL groups, mixed NULL/non-NULL, multiple groups, and bytes types
• All existing aggexec tests continue to pass with no regressions

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[XuPeng-SH]

Requesting changes for one correctness blocker in the binary-string path.

bitOpExecBytes.Flush() currently converts NULL aggregate slots by using a hard-coded 8-byte 0xff value for BIT_AND and leaving BIT_OR / BIT_XOR as empty bytes. That does not match the binary-string semantics of these aggregates. This executor is used when the argument type is BINARY / VARBINARY and the return type is the original argument type, so the neutral value has to preserve the argument byte length. MySQL documents the binary-string result as the same length as the argument values, and the all-NULL/no-row neutral value follows that same length rule.

Concrete failure I verified locally with a temporary unit repro:

typ := types.New(types.T_binary, 3, 0)
// two NULL input rows, then Flush()

Expected neutral values for BINARY(3) are:

• BIT_AND: []byte{0xff, 0xff, 0xff}
• BIT_OR: []byte{0x00, 0x00, 0x00}
• BIT_XOR: []byte{0x00, 0x00, 0x00}

Actual PR output is:

• BIT_AND: 8 bytes of 0xff
• BIT_OR: empty bytes
• BIT_XOR: empty bytes

The updated test/distributed/cases/dtype/binary.result also exposes this for the existing binary(10) case: group d is all NULL, but bit_and(b) is emitted as 8 bytes and bit_or(b) / bit_xor(b) as empty values instead of 10-byte neutral values.

Suggested fix direction:

1. Do not hard-code byte neutral values in Flush().
2. Track/derive the neutral width for bytes aggregates. For BINARY(N), param.Width should drive the neutral byte length. For VARBINARY, please verify the intended MySQL-compatible behavior for all-NULL / empty input and encode it explicitly instead of relying on the zero varlena default.
3. Fill NULL slots with bytes.Repeat(0xff, width) for BIT_AND and bytes.Repeat(0x00, width) for BIT_OR / BIT_XOR.
4. Add focused coverage for hex() and length() on BINARY(N) all-NULL groups, empty input without GROUP BY, mixed NULL/non-NULL groups, and the direct aggexec unit path. This will avoid invisible NUL bytes hiding result-length regressions in BVT output.

Unhappy-path pass: I did not find a hang/leak/unbounded-accumulation issue in this PR. The blocker is deterministic result corruption for the byte aggregate path.

[XuPeng-SH]

I still see one blocker on the latest head (3397d677). The earlier binary-width neutral-value issue looks fixed: the bytes flush path now tracks param.Width, and the added BINARY(10)/VARBINARY tests cover empty and all-NULL groups.

Blocking finding:

bitOpExecBytes.BatchFill and bitOpExecBytes.BatchMerge still ignore vector.SetBytesAt errors when copying the first non-NULL binary value into an empty aggregate state (pkg/sql/colexec/aggexec/bitops2.go, around the SetBytesAt calls in both methods). This is not just theoretical: for non-inline binary values (len > types.VarlenaInlineSize), SetBytesAt can fail through BuildVarlenaFromByteSlice -> BuildVarlenaNoInline -> mp.Grow2. Because the code clears the NULL marker before the unchecked write, a failed allocation leaves the group marked non-NULL with an empty/default payload. Flush() then skips neutral replacement and returns a silent wrong result instead of propagating OOM/internal error.

I verified this locally with a constrained mpool probe:

• BINARY(64) input vector allocated from a normal mpool.
• aggregate state allocated from a 1MiB capped mpool.
• after GroupGrow(1), fill the remaining mpool capacity so the state SetBytesAt area allocation fails.
• current BulkFill()/Merge() return nil, and Flush() returns a non-NULL value with length 0 instead of the 64-byte value or an error.

Please close the write contract for both paths:

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))
}

Do the same in BatchMerge. The important part is: return the SetBytesAt error and only clear NULL after the write succeeds, so failure leaves the state unchanged.

Please also add regression coverage for both unhappy paths:

• BulkFill first non-NULL BINARY(64)/VARBINARY(64) with constrained exec mpool returns an error and does not produce a non-NULL empty payload.
• Merge copying a first non-NULL binary partial result into an empty target group under the same constrained mpool returns an error.

Local verification on the PR as-is:

• go test ... ./pkg/sql/colexec/aggexec -run 'TestBitOps' passes.
• go test ... ./pkg/sql/colexec/aggexec passes.
• The constrained-mpool probe above fails on current code for both fill and merge, reproducing the state corruption.

[aunjgr]

Straightforward bug: BINARY(N) neutral values hard-coded to 8 bytes instead of N. Should use bytes.Repeat with param.Width. Narrow fix.