MDEV-10526: Add binary string support to bitwise operators by kjarir · Pull Request #5190 · MariaDB/server

kjarir · 2026-06-06T19:06:45Z

Draft for review. Implements byte-by-byte binary string mode for all scalar bitwise operators. Aggregate function
support (BIT_AND/BIT_OR/BIT_XOR) to follow.

Tested:

All 6 operators on VARBINARY columns
INET6 subnet masking (primary use case)
NULL handling
Mismatched length error
Integer mode backward compatibility
_binary introducer triggers binary mode correctly

gemini-code-assist

Code Review

This pull request introduces support for binary-to-binary bitwise operations (AND, OR, XOR, NOT, and bitwise shifts) on binary strings by implementing new handler classes and adding corresponding error messages for mismatched operand sizes. The review feedback highlights several critical issues: a potential integer overflow/wrap-around in the left-shift index calculation on 32-bit systems, incorrect ASCII string conversion of numeric arguments in mixed-type bitwise operations, and potential undefined behavior when handling empty binary strings due to passing a null pointer to memset.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-06T19:11:22Z

+        size_t src_idx= i + byte_shift;
+        if (src_idx < len)
+        {


The calculation size_t src_idx = i + byte_shift is susceptible to integer overflow/wrap-around on 32-bit systems if len is very large (e.g., close to size_t max). If i + byte_shift overflows, src_idx wraps around and can evaluate to a value less than len, leading to incorrect data copying from wrapped-around indices instead of zero-filling. To prevent this, perform the bounds check using subtraction (byte_shift < len - i) before computing src_idx.

if (byte_shift < len - i) { size_t src_idx= i + byte_shift;

gemini-code-assist · 2026-06-06T19:11:22Z

+    StringBuffer<128> b_buf;
+    String *b= item->arguments()[1]->val_str(&b_buf);


In mixed-type bitwise operations (e.g., binary_string & numeric), calling val_str on the numeric argument converts it to its decimal string representation (e.g., 255 becomes the 3-byte ASCII string "255" / 0x323535), rather than its binary representation (e.g., 0xFF). This causes mismatched length errors (e.g., VARBINARY(1) & 255 compares length 1 with length 3) or incorrect bitwise operations if the lengths happen to match. To align with standard bitwise behavior (and MySQL 8.0 compatibility), numeric arguments should be converted to a binary string representation of their integer value, padded or truncated to match the length of the other binary string operand.

gemini-code-assist · 2026-06-06T19:11:22Z

+    size_t len= a->length();
+
+    if (to->realloc(len))


When the input binary string a is empty (len == 0), calling to->realloc(0) can return a null pointer or do nothing. Subsequently, passing a null pointer to memset (e.g., memset(out_ptr, 0, len)) is technically undefined behavior in C/C++, even if the length is 0. Adding an early exit for len == 0 avoids this potential undefined behavior and improves efficiency by bypassing unnecessary allocation and loop overhead.

size_t len= a->length(); if (len == 0) { to->length(0); to->set_charset(&my_charset_bin); item->null_value= false; return to; } if (to->realloc(len))

grooverdan

Should be off the main branch as a new feature.

from @abarkov

"Would be nice to add tests that uuid, inet6, inet4, geometry are not allowed for bit operations. Later we can probably implement bit operations for uuid, inet6, inet4. But to avoid compatibility problems we need to make sure they return error now."

CLAassistant · 2026-06-11T07:24:54Z

All committers have signed the CLA.

gkodinov

Thank you for your contribution! This is a preliminary review.

Please squash your commits to 1 per feature. Maybe keep the spider part in a separate commit and the rest of it in another? Just a suggestion. But there definitely should not be 13 commits.

gkodinov · 2026-06-25T08:15:56Z

+# =========================================================================
+CREATE TABLE t_null (a VARBINARY(4), b VARBINARY(4));
+INSERT INTO t_null VALUES (x'FFFF0000', NULL);
+# Expected output: NULL, NULL, NULL, NULL, NULL, NULL


do you really need a table for that?

gkodinov

LGTM. Please stand by for the final review.

mariadb-YuchenPei · 2026-06-26T08:04:15Z

I'm reviewing the spider bit

grooverdan · 2026-06-30T03:49:06Z

+    String *a= item->arguments()[0]->val_str(&a_buf);
+    if (a == nullptr)
+    {
+      item->null_value= true;


normally this would just be null_value= true for the current object. Is null and invalid argument? Do we need to test if (item->arguments[0]->null_value) before val_str?

grooverdan · 2026-06-30T03:50:47Z

+    const uchar *a_ptr= (const uchar *) a->ptr();
+
+    if (shift_count_signed < 0 || (ulonglong)shift_count_signed >= (ulonglong)len * 8)
+    {


on style is ok to have a single line (only) in a if like condition without braces if its clear.

grooverdan · 2026-06-30T04:15:26Z

+    item->max_length= item->arguments()[0]->max_length;
+    item->collation.set(&my_charset_bin, DERIVATION_COERCIBLE);
+    return false;
+  }


^ common shared with shift type handers. Can the commonality be in the same class.

mariadb-YuchenPei

Some comments about the test. After these are addressed there may need to be another round about it.

Still reviewing spider C++ files.

mariadb-YuchenPei · 2026-07-01T05:08:49Z

@@ -0,0 +1,154 @@
+--disable_warnings


Why disable warnings?

mariadb-YuchenPei · 2026-07-01T08:00:41Z

/gemini review

gemini-code-assist

Code Review

This pull request implements binary string support for bitwise operators and aggregate functions (MDEV-10526), including window function and Spider pushdown support. The review identified several critical issues: heap buffer overflows in Item_sum_bit due to incorrect byte allocation sizes with thd->alloc and thd->calloc, an out-of-bounds read and incorrect parsing of 64-bit unsigned integers in Spider's spd_db_conn.cc using my_strtoll10, and potential null pointer dereferences in the binary accumulator reset functions due to unchecked allocation results.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-07-01T08:05:18Z

+  {
+    if (item->m_binary_bit_counters)
+    {
+      m_binary_bit_counters= thd->alloc<uint32>(m_binary_length * 8);


The call to thd->alloc<uint32>(m_binary_length * 8) allocates m_binary_length * 8 bytes rather than elements because alloc takes the size in bytes. This leads to a severe buffer overflow when memcpy copies m_binary_length * 8 * sizeof(uint32) bytes into it. It should be changed to allocate the correct size in bytes.

m_binary_bit_counters= (uint32*) thd->alloc(m_binary_length * 8 * sizeof(uint32));

gemini-code-assist · 2026-07-01T08:05:18Z

+{
+  if (m_binary_mode)
+  {
+    m_binary_bit_counters= thd->calloc<uint32>(m_binary_length * 8);


The call to thd->calloc<uint32>(m_binary_length * 8) allocates m_binary_length * 8 bytes instead of elements. This leads to a severe buffer overflow when writing up to m_binary_length * 8 elements of uint32. It should be changed to allocate the correct size in bytes.

m_binary_bit_counters= (uint32*) thd->calloc(m_binary_length * 8 * sizeof(uint32));

gemini-code-assist · 2026-07-01T08:05:18Z

+            int conv_error= 0;
+            ulonglong bits= (ulonglong) my_strtoll10(tmp_str.ptr(),
+                                                     (char **) NULL,
+                                                     &conv_error);


Using my_strtoll10 to parse unsigned 64-bit integers is incorrect because it clamps values greater than LONGLONG_MAX to LONGLONG_MAX and sets an error. Additionally, my_strtoll10 does not take a length parameter and expects a null-terminated string, which can lead to an out-of-bounds read if tmp_str is not null-terminated. We should use my_strntoull10 with tmp_str.length() instead.

int conv_error= 0; const char *endptr; ulonglong bits= my_strntoull10(tmp_str.ptr(), tmp_str.length(), &conv_error, &endptr);

mariadb-YuchenPei · 2026-07-01T08:06:14Z

+  if (m_binary_mode)
+  {
+    if (unlikely(direct_added))
+    {
+      direct_added= FALSE;
+      direct_reseted_field= TRUE;
+      null_value= direct_sum_is_null;
+      if (null_value)
+      {
+        result_field->set_null();
+        result_field->reset();
+      }
+      else
+      {
+        result_field->set_notnull();
+        result_field->store(direct_str_value.ptr(), direct_str_value.length(), &my_charset_bin);
+      }
+      return;
+    }


This part is not covered in the spider test, despite this bit added in the spider commit. Can you add test coverage?

mariadb-YuchenPei · 2026-07-01T08:06:48Z

+  if (unlikely(direct_added))
+  {
+    direct_added= FALSE;
+    direct_reseted_field= TRUE;
+    null_value= direct_sum_is_null;
+    bits= null_value ? reset_bits : direct_bits;
+    if (null_value)
+      result_field->set_null();
+    else
+      result_field->set_notnull();
+    int8store(result_field->ptr, bits);
+    return;
+  }


This part is not covered in the spider test, despite this bit added in the spider commit. Can you add test coverage?

mariadb-YuchenPei · 2026-07-01T08:09:35Z

+      if (unlikely(direct_added || direct_reseted_field))
+      {
+        direct_added= TRUE;
+        direct_reseted_field= FALSE;
+      }


This part is not covered in the spider test, despite this bit added in the spider commit. Can you add test coverage? Same for other bits added in Item_sum_bit::update_field in the spider commit - the method itself is not reached

mariadb-YuchenPei · 2026-07-01T08:10:20Z

+          if (row->is_null())
+          {
+            item_sum_bit->direct_add((const String *) NULL, TRUE);
+          }


Please add test coverage for this branch, same below

…ates Extends all six scalar bitwise operators (&, |, ^, ~, <<, >>) and aggregate functions (BIT_AND, BIT_OR, BIT_XOR) to work byte-by-byte on BINARY/VARBINARY columns, returning VARBINARY of the same length. Previously all operators silently cast binary arguments to BIGINT (64-bit), truncating values wider than 64 bits. This broke operations on INET6 addresses, UUIDs, and any binary column wider than 8 bytes. Binary mode activates when both operands have a true non-hybrid binary string type_handler (Type_handler_longstr with my_charset_bin collation, excluding hex hybrids like 0xFF). Uses the existing Item_handled_func pluggable Handler pattern, adding new Handler_str subclasses per operator alongside existing Handler_ulonglong handlers. Aggregate functions gain a parallel String accumulator with correct neutral elements (0xFF for AND, 0x00 for OR/XOR). Window function support added via a dynamically-sized uint32 bit-counter array (same sliding-window technique as the existing 64-bit bit_counters[], extended to arbitrary lengths). Key fixes included: - reset_field()/update_field() binary mode (crash fix: raw int8store on string-typed temp table field caused SIGSEGV) - return_type_handler() varies by max_length following the blob_type_handler/type_handler_varchar/type_handler_string pattern from Item_char_typecast_func_handler_fbt_to_binary - DERIVATION_COERCIBLE (not IMPLICIT) for binary results - MY_MAX() overflow pattern replaced with plain increment in window function counters - Mismatched lengths push warning (not hard error), escalating to error under strict SQL mode via THD::raise_condition() - Regressions in main.gis (geometry entering binary mode via STRING_RESULT+my_charset_bin) and main.func_json (JSON over-rejected by stricter check_arguments() override) fixed - GEOMETRY and hex hybrids excluded from binary mode detection New error codes: ER_INVALID_BITWISE_OPERANDS_SIZE ER_INVALID_BITWISE_AGGREGATE_OPERANDS_SIZE Closes: MDEV-10526 Review feedback addressed (gkodinov, mariadb-YuchenPei): - Remove unnecessary expected-output comments from test file - Add my_error(ER_OUTOFMEMORY) to all realloc/alloc failure paths - Add DBUG_ASSERT(argument_count() == 2) in 2-operand val_str() - Fix NULL check style: check null_value before val_str return - Add algorithm comments to shift handlers - Refactor shift left/right into base class Func_handler_shift_bin_to_bin with virtual do_shift() - eliminates duplication - Group same-type struct members together in Item_sum_bit - Rewrite Spider test as single-server test (spider_same_server_link=1) - Add GROUP BY test cases for reset_field/update_field coverage - Refactor SUM_BIT_FUNC in spd_db_conn.cc to reduce duplication

Implements Spider direct_add() support for BIT_AND/BIT_OR/BIT_XOR, allowing each remote shard to compute its own partial aggregate merged locally, mirroring the existing SUM/COUNT/MIN/MAX pattern. BIT_AND/OR/XOR are associative and commutative so partial results from shards can be merged correctly via direct_add(). Also fixes a pre-existing Spider bug: row->val_int() uses atoi() which silently overflows for BIGINT UNSIGNED values above INT_MAX. Fixed for this code path using my_strtoll10() instead. Closes: MDEV-10526 Review feedback addressed (mariadb-YuchenPei): - Rewrite Spider test as single-server test (spider_same_server_link=1) - Add GROUP BY test cases for reset_field/update_field coverage - Add NULL row test coverage for direct_add() NULL branch - Refactor SUM_BIT_FUNC in spd_db_conn.cc to reduce duplication

gemini-code-assist Bot reviewed Jun 6, 2026

View reviewed changes

kjarir force-pushed the feature/MDEV-10526-bitwise-binary branch from fa5cd7c to 862a606 Compare June 6, 2026 19:44

gkodinov added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Jun 8, 2026

grooverdan added the GSoC label Jun 10, 2026

grooverdan reviewed Jun 10, 2026

View reviewed changes

Comment thread sql/item_func.cc Outdated

Comment thread sql/item_func.cc

Comment thread sql/item_func.cc Outdated

Comment thread sql/item_func.cc Outdated

kjarir force-pushed the feature/MDEV-10526-bitwise-binary branch from 221664d to 6b9dd9e Compare June 11, 2026 07:24

grooverdan changed the base branch from 10.11 to main June 11, 2026 07:28

kjarir marked this pull request as ready for review June 25, 2026 07:07

kjarir requested a review from grooverdan June 25, 2026 07:07

gkodinov requested changes Jun 25, 2026

View reviewed changes

gkodinov self-assigned this Jun 25, 2026

kjarir force-pushed the feature/MDEV-10526-bitwise-binary branch from 934bb95 to 75cfdf6 Compare June 25, 2026 19:14

mariadb-YuchenPei self-requested a review June 26, 2026 05:44

gkodinov approved these changes Jun 26, 2026

View reviewed changes

Comment thread mysql-test/main/func_bitops_binary.test Outdated

gkodinov assigned grooverdan and unassigned gkodinov Jun 26, 2026

grooverdan reviewed Jun 30, 2026

View reviewed changes

mariadb-YuchenPei requested changes Jul 1, 2026

View reviewed changes

gemini-code-assist Bot reviewed Jul 1, 2026

View reviewed changes

mariadb-YuchenPei requested changes Jul 1, 2026

View reviewed changes

kjarir force-pushed the feature/MDEV-10526-bitwise-binary branch from 75cfdf6 to 7b07819 Compare July 15, 2026 06:52

kjarir force-pushed the feature/MDEV-10526-bitwise-binary branch from 7b07819 to 719f5a4 Compare July 17, 2026 18:07

		StringBuffer<128> b_buf;
		String *b= item->arguments()[1]->val_str(&b_buf);

Uh oh!

Uh oh!

Conversation

kjarir commented Jun 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 6, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 6, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 6, 2026

Choose a reason for hiding this comment

Uh oh!

grooverdan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CLAassistant commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gkodinov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mariadb-YuchenPei commented Jun 26, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mariadb-YuchenPei left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mariadb-YuchenPei commented Jul 1, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

kjarir commented Jun 6, 2026 •

edited

Loading

CLAassistant commented Jun 11, 2026 •

edited

Loading

mariadb-YuchenPei left a comment •

edited

Loading