Skip to content

feat: support binary arguments for StringConcat operator #21883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jefffrey merged 8 commits into from
May 1, 2026
Merged

feat: support binary arguments for StringConcat operator #21883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5d46932
Support binary arguments for StringConcat
theirix Apr 6, 2026
c539d00
Update tests
theirix Apr 28, 2026
905571c
Clarify binary coercion rules for binary concat
theirix Apr 28, 2026
5629996
More tests for coercer
theirix Apr 28, 2026
6dc7a22
Reuse arrow concat_element_binary
theirix Apr 28, 2026
46bfc74
Support FixedSizeBinary by coercing to Binary
theirix Apr 29, 2026
77768b9
Merge branch 'main' into binary-concat-fixes
Jefffrey Apr 30, 2026
5469c45
Merge branch 'main' into binary-concat-fixes
Jefffrey May 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 34 additions & 3 deletions datafusion/expr-common/src/type_coercion/binary.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1589,9 +1589,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> {
Copy link
Copy Markdown
Contributor

@kumarUjjawal kumarUjjawal Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before the change, Binary || Utf8 returned a Utf8 string, and Binary || Binary also returned Utf8. Now the first errors out, and the second returns Binary, wouldn't this be a breaking change for users who are hitting this in a query?

Copy link
Copy Markdown
Contributor Author

@theirix theirix Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is worth noting, as we decided to ban mixing according to this analysis.

I'll add api change label and put an upgrade notice shortly.

I intend to put a separate PR to harmonise concat UDF behaviour (it allows mixing) with the pipe operator's behaviour to avoid confusion

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)
}
Expand All @@ -1604,7 +1632,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,
})
}
Expand Down Expand Up @@ -1734,13 +1761,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
Expand Down
106 changes: 106 additions & 0 deletions datafusion/expr-common/src/type_coercion/binary/tests/comparison.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
31 changes: 17 additions & 14 deletions datafusion/physical-expr/src/expressions/binary.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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>(),
Expand All @@ -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"
Expand Down
50 changes: 47 additions & 3 deletions datafusion/physical-expr/src/expressions/binary/kernels.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) => {{
Expand Down
37 changes: 37 additions & 0 deletions datafusion/sqllogictest/test_files/binary.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,3 +321,40 @@ query T
SELECT split_part(CAST(binary AS VARCHAR), 'o', 2) FROM t WHERE binary = X'466f6f';
----
(empty)

Copy link
Copy Markdown
Contributor

@Jefffrey Jefffrey Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a test with fixedsizebinary too?

Copy link
Copy Markdown
Contributor Author

@theirix theirix Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, added support for this type as well

# 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
Copy link
Copy Markdown
Contributor

@Jefffrey Jefffrey Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about mixed binary?

1. query failed: DataFusion error: Error during planning: Cannot infer common string type for string concat operation Binary || LargeBinary
[SQL] SELECT x'636166c3a9' || arrow_cast(x'68656c6c6f', 'LargeBinary');
at /Users/jeffrey/Code/datafusion/datafusion/sqllogictest/test_files/binary.slt:331

Is this intentional?

Copy link
Copy Markdown
Contributor Author

@theirix theirix Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I slipped it, thanks!. Added unit and SLT tests

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');
18 changes: 3 additions & 15 deletions datafusion/sqllogictest/test_files/scalar.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading