You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/05/01 05:34:10 UTC
(datafusion) branch gh-readonly-queue/main/pr-21883-37dbdafc414d4e14470401f5351ba19797886875 created (now ea0928c19c)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-21883-37dbdafc414d4e14470401f5351ba19797886875
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at ea0928c19c feat: support binary arguments for StringConcat operator (#21883)
This branch includes the following new commits:
new ea0928c19c feat: support binary arguments for StringConcat operator (#21883)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: feat: support binary arguments for StringConcat operator (#21883)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-21883-37dbdafc414d4e14470401f5351ba19797886875
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit ea0928c19ccfff9444ef3e5affd8c90e2db89e90
Author: theirix <th...@gmail.com>
AuthorDate: Fri May 1 06:33:46 2026 +0100
feat: support binary arguments for StringConcat operator (#21883)
## Which issue does this PR close?
- Refers #12709.
## Rationale for this change
Binary arguments are supported for concat UDFs, but not for the pipe
operator (`||`), which supports only text.
## What changes are included in this PR?
- Support binary concat by providing specialised kernels for pure binary
operations. Avoid support of mixed string/binary arguments as it doesn't
match the behaviour of major DBs, except for Postgres (see the table in
the linked ticket).
- Add `concat_elements_binary_view_array` kernel
- Refactor private `binary_coercion` to support symmetric BinaryLike +
BinaryLike - required for the new codeflow
Concat UDFs are out of scope and supported separately.
## Are these changes tested?
- Existing SLTs
- Moved a few tests to a more appropriate `binary.slt`
- Added new unit tests
## Are there any user-facing changes?
Concatenation `||` operator now allows binary+binary concatenation
(`SELECT x'636166c3a9' || x'68656c6c6f'`), but denies mixed
string+binary concatenation `SELECT x'636166c3a9' || 'hello'`
---------
Co-authored-by: Jeffrey Vo <je...@gmail.com>
---
datafusion/expr-common/src/type_coercion/binary.rs | 37 ++++++-
.../src/type_coercion/binary/tests/comparison.rs | 106 +++++++++++++++++++++
datafusion/physical-expr/src/expressions/binary.rs | 31 +++---
.../src/expressions/binary/kernels.rs | 50 +++++++++-
datafusion/sqllogictest/test_files/binary.slt | 37 +++++++
datafusion/sqllogictest/test_files/scalar.slt | 18 +---
6 files changed, 244 insertions(+), 35 deletions(-)
diff --git a/datafusion/expr-common/src/type_coercion/binary.rs b/datafusion/expr-common/src/type_coercion/binary.rs
index 02bfdd9b34..aec87ec5ff 100644
--- a/datafusion/expr-common/src/type_coercion/binary.rs
+++ b/datafusion/expr-common/src/type_coercion/binary.rs
@@ -1611,9 +1611,37 @@ fn ree_coercion(
/// This is a union of string coercion rules and specified rules:
/// 1. At least one side of lhs and rhs should be string type (Utf8 / LargeUtf8)
/// 2. Data type of the other side should be able to cast to string type
+/// 3. Binary and string types cannot be mixed
fn string_concat_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
use arrow::datatypes::DataType::*;
string_coercion(lhs_type, rhs_type).or_else(|| match (lhs_type, rhs_type) {
+ // Allow pure binary + binary
+ (
+ Binary | LargeBinary | BinaryView | FixedSizeBinary(_),
+ Binary | LargeBinary | BinaryView | FixedSizeBinary(_),
+ ) => {
+ // Coerce fixed-sized binary to variable-sized `Binary` to make uniform signature
+ // with the `Binary` result
+ let lhs_type = match lhs_type {
+ FixedSizeBinary(_) => &Binary,
+ val => val,
+ };
+ let rhs_type = match rhs_type {
+ FixedSizeBinary(_) => &Binary,
+ val => val,
+ };
+ binary_coercion(lhs_type, rhs_type)
+ }
+ // Deny other mixed binary + string combinations
+ (
+ Binary | LargeBinary | BinaryView | FixedSizeBinary(_),
+ Utf8 | LargeUtf8 | Utf8View,
+ ) => None,
+ (
+ Utf8 | LargeUtf8 | Utf8View,
+ Binary | LargeBinary | BinaryView | FixedSizeBinary(_),
+ ) => None,
+ // Predicate-based coercion rules are following
(Utf8View, from_type) | (from_type, Utf8View) => {
string_concat_internal_coercion(from_type, &Utf8View)
}
@@ -1626,7 +1654,6 @@ fn string_concat_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<Da
(Dictionary(_, lhs_value_type), Dictionary(_, rhs_value_type)) => {
string_coercion(lhs_value_type, rhs_value_type).or(None)
}
- (Binary, Binary) => Some(Utf8),
_ => None,
})
}
@@ -1756,13 +1783,17 @@ pub fn binary_to_string_coercion(
fn binary_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
use arrow::datatypes::DataType::*;
match (lhs_type, rhs_type) {
+ // Prefer symmetric coercion (in case the function is called directly)
+ (Binary, Binary) => Some(Binary),
+ (LargeBinary, LargeBinary) => Some(LargeBinary),
+ (BinaryView, BinaryView) => Some(BinaryView),
// If BinaryView is in any side, we coerce to BinaryView.
- (BinaryView, BinaryView | Binary | LargeBinary | Utf8 | LargeUtf8 | Utf8View)
+ (BinaryView, Binary | LargeBinary | Utf8 | LargeUtf8 | Utf8View)
| (LargeBinary | Binary | Utf8 | LargeUtf8 | Utf8View, BinaryView) => {
Some(BinaryView)
}
// Prefer LargeBinary over Binary
- (LargeBinary | Binary | Utf8 | LargeUtf8 | Utf8View, LargeBinary)
+ (Binary | Utf8 | LargeUtf8 | Utf8View, LargeBinary)
| (LargeBinary, Binary | Utf8 | LargeUtf8 | Utf8View) => Some(LargeBinary),
// If Utf8View/LargeUtf8 presents need to be large Binary
diff --git a/datafusion/expr-common/src/type_coercion/binary/tests/comparison.rs b/datafusion/expr-common/src/type_coercion/binary/tests/comparison.rs
index 317b022238..f8bff3ca90 100644
--- a/datafusion/expr-common/src/type_coercion/binary/tests/comparison.rs
+++ b/datafusion/expr-common/src/type_coercion/binary/tests/comparison.rs
@@ -897,3 +897,109 @@ fn test_binary_comparison_string_numeric_coercion() -> Result<()> {
}
Ok(())
}
+
+#[test]
+fn test_string_concat_coercion() -> Result<()> {
+ // Binary
+ test_coercion_binary_rule!(
+ DataType::Binary,
+ DataType::Binary,
+ Operator::StringConcat,
+ DataType::Binary
+ );
+ test_coercion_binary_rule!(
+ DataType::LargeBinary,
+ DataType::LargeBinary,
+ Operator::StringConcat,
+ DataType::LargeBinary
+ );
+ test_coercion_binary_rule!(
+ DataType::BinaryView,
+ DataType::BinaryView,
+ Operator::StringConcat,
+ DataType::BinaryView
+ );
+ test_coercion_binary_rule!(
+ DataType::Binary,
+ DataType::LargeBinary,
+ Operator::StringConcat,
+ DataType::LargeBinary
+ );
+ test_coercion_binary_rule!(
+ DataType::BinaryView,
+ DataType::Binary,
+ Operator::StringConcat,
+ DataType::BinaryView
+ );
+ test_coercion_binary_rule!(
+ DataType::FixedSizeBinary(4),
+ DataType::FixedSizeBinary(16),
+ Operator::StringConcat,
+ DataType::Binary
+ );
+ test_coercion_binary_rule!(
+ DataType::FixedSizeBinary(4),
+ DataType::LargeBinary,
+ Operator::StringConcat,
+ DataType::LargeBinary
+ );
+ test_coercion_binary_rule!(
+ DataType::FixedSizeBinary(4),
+ DataType::BinaryView,
+ Operator::StringConcat,
+ DataType::BinaryView
+ );
+
+ // String
+ test_coercion_binary_rule!(
+ DataType::Utf8,
+ DataType::Utf8,
+ Operator::StringConcat,
+ DataType::Utf8
+ );
+ test_coercion_binary_rule!(
+ DataType::LargeUtf8,
+ DataType::LargeUtf8,
+ Operator::StringConcat,
+ DataType::LargeUtf8
+ );
+ test_coercion_binary_rule!(
+ DataType::Utf8View,
+ DataType::Utf8View,
+ Operator::StringConcat,
+ DataType::Utf8View
+ );
+
+ // Mixed string-binary
+ for string_dt in [DataType::Utf8, DataType::LargeUtf8, DataType::Utf8View] {
+ for binary_dt in [
+ DataType::Binary,
+ DataType::LargeBinary,
+ DataType::BinaryView,
+ DataType::FixedSizeBinary(8),
+ ] {
+ assert!(
+ BinaryTypeCoercer::new(&binary_dt, &Operator::StringConcat, &string_dt,)
+ .get_input_types()
+ .is_err(),
+ "{binary_dt} || {string_dt}"
+ );
+ assert!(
+ BinaryTypeCoercer::new(&string_dt, &Operator::StringConcat, &binary_dt,)
+ .get_input_types()
+ .is_err(),
+ "{string_dt} || {binary_dt}"
+ );
+ }
+ }
+
+ // Mixed string-other
+ test_coercion_binary_rule!(
+ DataType::Utf8,
+ DataType::Timestamp(Second, None),
+ Operator::StringConcat,
+ DataType::Utf8
+ );
+
+ Ok(())
+}
diff --git a/datafusion/physical-expr/src/expressions/binary.rs b/datafusion/physical-expr/src/expressions/binary.rs
index ae93b1d03a..396c0b87c4 100644
--- a/datafusion/physical-expr/src/expressions/binary.rs
+++ b/datafusion/physical-expr/src/expressions/binary.rs
@@ -24,7 +24,9 @@ use std::sync::Arc;
use arrow::array::*;
use arrow::compute::kernels::boolean::{and_kleene, or_kleene};
-use arrow::compute::kernels::concat_elements::concat_elements_utf8;
+use arrow::compute::kernels::concat_elements::{
+ concat_element_binary, concat_elements_utf8,
+};
use arrow::compute::{SlicesIterator, cast, filter_record_batch};
use arrow::datatypes::*;
use arrow::error::ArrowError;
@@ -46,7 +48,8 @@ use kernels::{
bitwise_and_dyn, bitwise_and_dyn_scalar, bitwise_or_dyn, bitwise_or_dyn_scalar,
bitwise_shift_left_dyn, bitwise_shift_left_dyn_scalar, bitwise_shift_right_dyn,
bitwise_shift_right_dyn_scalar, bitwise_xor_dyn, bitwise_xor_dyn_scalar,
- concat_elements_utf8view, regex_match_dyn, regex_match_dyn_scalar,
+ concat_elements_binary_view_array, concat_elements_utf8view, regex_match_dyn,
+ regex_match_dyn_scalar,
};
/// Binary expression
@@ -928,18 +931,6 @@ fn pre_selection_scatter(
}
fn concat_elements(left: &ArrayRef, right: &ArrayRef) -> Result<ArrayRef> {
- if *left.data_type() == DataType::Binary && *right.data_type() == DataType::Binary {
- // Cast Binary to Utf8 to validate UTF-8 encoding before concatenation
- // Follow widespread approach of PostgreSQL, sqlite, DuckDB, Snowflake
- // Spark does it in a different way by making a binary-to-binary concatenation
- let left = cast(left.as_ref(), &DataType::Utf8)?;
- let right = cast(right.as_ref(), &DataType::Utf8)?;
- return Ok(Arc::new(concat_elements_utf8(
- left.as_string::<i32>(),
- right.as_string::<i32>(),
- )?));
- }
-
Ok(match left.data_type() {
DataType::Utf8 => Arc::new(concat_elements_utf8(
left.as_string::<i32>(),
@@ -953,6 +944,18 @@ fn concat_elements(left: &ArrayRef, right: &ArrayRef) -> Result<ArrayRef> {
left.as_string_view(),
right.as_string_view(),
)?),
+ DataType::Binary => Arc::new(concat_element_binary::<i32>(
+ left.as_binary(),
+ right.as_binary(),
+ )?),
+ DataType::LargeBinary => Arc::new(concat_element_binary::<i64>(
+ left.as_binary(),
+ right.as_binary(),
+ )?),
+ DataType::BinaryView => Arc::new(concat_elements_binary_view_array(
+ left.as_binary_view(),
+ right.as_binary_view(),
+ )?),
other => {
return internal_err!(
"Data type {other:?} not supported for binary operation 'concat_elements' on string arrays"
diff --git a/datafusion/physical-expr/src/expressions/binary/kernels.rs b/datafusion/physical-expr/src/expressions/binary/kernels.rs
index 9818861c2f..e573d7ece2 100644
--- a/datafusion/physical-expr/src/expressions/binary/kernels.rs
+++ b/datafusion/physical-expr/src/expressions/binary/kernels.rs
@@ -18,7 +18,7 @@
//! This module contains computation kernels that are specific to
//! datafusion and not (yet) targeted to port upstream to arrow
use arrow::array::*;
-use arrow::buffer::NullBuffer;
+use arrow::buffer::{MutableBuffer, NullBuffer};
use arrow::compute::kernels::bitwise::{
bitwise_and, bitwise_and_scalar, bitwise_or, bitwise_or_scalar, bitwise_shift_left,
bitwise_shift_left_scalar, bitwise_shift_right, bitwise_shift_right_scalar,
@@ -161,11 +161,11 @@ create_left_integral_dyn_scalar_kernel!(
bitwise_shift_left_scalar
);
-/// Concatenates two `StringViewArray`s element-wise.
+/// Concatenates two `StringViewArray`s element-wise.
/// If either element is `Null`, the result element is also `Null`.
///
/// # Errors
-/// - Returns an error if the input arrays have different lengths.
+/// - Returns an error if the input arrays have different lengths.
/// - Returns an error if any concatenated string exceeds `u32::MAX` (≈4 GB) in length.
pub fn concat_elements_utf8view(
left: &StringViewArray,
@@ -204,6 +204,50 @@ pub fn concat_elements_utf8view(
Ok(result.finish())
}
+/// Concatenates two `BinaryViewArray`s element-wise.
+/// If either element is `Null`, the result element is also `Null`.
+///
+/// # Errors
+/// - Returns an error if the input arrays have different lengths.
+/// - Returns an error if any concatenated string exceeds `u32::MAX` in length.
+pub fn concat_elements_binary_view_array(
+ left: &BinaryViewArray,
+ right: &BinaryViewArray,
+) -> std::result::Result<BinaryViewArray, ArrowError> {
+ if left.len() != right.len() {
+ return Err(ArrowError::ComputeError(format!(
+ "Arrays must have the same length: {} != {}",
+ left.len(),
+ right.len()
+ )));
+ }
+ let mut result = BinaryViewBuilder::with_capacity(left.len());
+
+ // Avoid reallocations by writing to a reused buffer (note we could be even
+ // more efficient by creating the view directly here and avoid the buffer
+ // but that would be more complex)
+ let mut buffer = MutableBuffer::new(0);
+
+ // Pre-compute combined null bitmap, so the per-row NULL check is more
+ // efficient
+ let nulls = NullBuffer::union(left.nulls(), right.nulls());
+
+ for i in 0..left.len() {
+ if nulls.as_ref().is_some_and(|n| n.is_null(i)) {
+ result.append_null();
+ } else {
+ let l = left.value(i);
+ let r = right.value(i);
+ buffer.clear();
+ buffer.extend_from_slice(l);
+ buffer.extend_from_slice(r);
+ // No try-version of append_value
+ result.try_append_value(&buffer)?;
+ }
+ }
+ Ok(result.finish())
+}
+
/// Invoke a compute kernel on a pair of binary data arrays with flags
macro_rules! regexp_is_match_flag {
($LEFT:expr, $RIGHT:expr, $ARRAYTYPE:ident, $NOT:expr, $FLAG:expr) => {{
diff --git a/datafusion/sqllogictest/test_files/binary.slt b/datafusion/sqllogictest/test_files/binary.slt
index 54ac51d9e7..a57c31547f 100644
--- a/datafusion/sqllogictest/test_files/binary.slt
+++ b/datafusion/sqllogictest/test_files/binary.slt
@@ -321,3 +321,40 @@ query T
SELECT split_part(CAST(binary AS VARCHAR), 'o', 2) FROM t WHERE binary = X'466f6f';
----
(empty)
+
+# Pipe concatenation of binaries always provides a binary
+query ?
+SELECT x'636166c3a9' || x'68656c6c6f';
+----
+636166c3a968656c6c6f
+
+# Pipe concatenation of binary and other kind of binary also provides a binary
+query ?
+SELECT x'636166c3a9' || arrow_cast(x'68656c6c6f', 'LargeBinary');
+----
+636166c3a968656c6c6f
+
+query ?
+SELECT x'636166c3a9' || arrow_cast(arrow_cast(x'68656c6c6f', 'LargeBinary'), 'BinaryView');
+----
+636166c3a968656c6c6f
+
+query ?T
+SELECT x'636166c3a9' || arrow_cast(x'68656c6c6f', 'FixedSizeBinary(5)'), arrow_typeof(x'636166c3a9' || arrow_cast(x'68656c6c6f', 'FixedSizeBinary(5)'));
+----
+636166c3a968656c6c6f Binary
+
+query ?T
+SELECT arrow_cast(x'6361', 'FixedSizeBinary(2)') || arrow_cast(x'68656c6c6f', 'FixedSizeBinary(5)'), arrow_typeof(arrow_cast(x'6361', 'FixedSizeBinary(2)') || arrow_cast(x'68656c6c6f', 'FixedSizeBinary(5)'));
+----
+636168656c6c6f Binary
+
+# Byte pipe operator is forbidden for mixed binary and text
+query error DataFusion error: Error during planning: Cannot infer common string type for string concat operation Binary || Utf8
+SELECT x'c3a9' || 'hello';
+
+query error DataFusion error: Error during planning: Cannot infer common string type for string concat operation Utf8 || LargeBinary
+SELECT 'hello' || arrow_cast(arrow_cast('hello', 'Binary'), 'LargeBinary');
+
+query error DataFusion error: Error during planning: Cannot infer common string type for string concat operation Utf8 || BinaryView
+SELECT 'hello' || arrow_cast(arrow_cast('hello', 'Binary'), 'BinaryView');
diff --git a/datafusion/sqllogictest/test_files/scalar.slt b/datafusion/sqllogictest/test_files/scalar.slt
index aac6abfe2f..89ae30e3c0 100644
--- a/datafusion/sqllogictest/test_files/scalar.slt
+++ b/datafusion/sqllogictest/test_files/scalar.slt
@@ -1739,23 +1739,11 @@ SELECT 'a' || 42 || 23.3
----
a4223.3
-# concat of binary and text provides a text output
-query T
-select arrow_cast('Café', 'Utf8') || arrow_cast('Foobar', 'Binary');
-----
-CaféFoobar
-
-query T
-select arrow_cast('Café', 'Binary') || arrow_cast('Foobar', 'Utf8');
-----
-CaféFoobar
-
-# Concat of two binaries should cast arguments to text and produce a text output,
-# following common behaviour of PostreSQL. However, Spark is providing binary
-query T
+# Concat operator of two binaries uses their binary representation without text at all
+query ?
select arrow_cast('Café', 'Binary') || arrow_cast('Foobar', 'Binary');
----
-CaféFoobar
+436166c3a9466f6f626172
# test_not_expressions()
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org