Skip to content

Commit e4e8598

Browse files
authored
fix: Add integer check for bitwise coercion (#20241)
## Which issue does this PR close? N/A ## Rationale for this change In the original codebase, bitwise_coercion was implemented as follows: ```rust if left_type == right_type { return Some(left_type.clone()); } ``` This causes any identical types—such as floats—to pass the check during the logical planning stage. The error only surfaces much later when the arrow kernel attempts execution. This appears to be a minor oversight by the original author. ## What changes are included in this PR? ```diff - if left_type == right_type { + if left_type == right_type && left_type.is_integer() { return Some(left_type.clone()); } ``` ## Are these changes tested? Yes, a new unit test is added, and all existing tests passed. ## Are there any user-facing changes? No.
1 parent 9fd84e7 commit e4e8598

3 files changed

Lines changed: 58 additions & 1 deletion

File tree

datafusion/expr-common/src/type_coercion/binary.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,9 @@ fn bitwise_coercion(left_type: &DataType, right_type: &DataType) -> Option<DataT
470470
return None;
471471
}
472472

473-
if left_type == right_type {
473+
let is_integer_dictionary =
474+
matches!(left_type, Dictionary(_, value_type) if value_type.is_integer());
475+
if left_type == right_type && (left_type.is_integer() || is_integer_dictionary) {
474476
return Some(left_type.clone());
475477
}
476478

datafusion/expr-common/src/type_coercion/binary/tests/arithmetic.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,53 @@ fn test_type_coercion_arithmetic() -> Result<()> {
228228
Ok(())
229229
}
230230

231+
#[test]
232+
fn test_bitwise_coercion_non_integer_types() -> Result<()> {
233+
let err = BinaryTypeCoercer::new(
234+
&DataType::Float32,
235+
&Operator::BitwiseAnd,
236+
&DataType::Float32,
237+
)
238+
.get_input_types()
239+
.unwrap_err()
240+
.to_string();
241+
assert_contains!(
242+
&err,
243+
"Cannot infer common type for bitwise operation Float32 & Float32"
244+
);
245+
246+
let err = BinaryTypeCoercer::new(
247+
&DataType::Float32,
248+
&Operator::BitwiseAnd,
249+
&DataType::Float64,
250+
)
251+
.get_input_types()
252+
.unwrap_err()
253+
.to_string();
254+
assert_contains!(
255+
&err,
256+
"Cannot infer common type for bitwise operation Float32 & Float64"
257+
);
258+
259+
let err = BinaryTypeCoercer::new(
260+
&DataType::Decimal128(10, 2),
261+
&Operator::BitwiseAnd,
262+
&DataType::Decimal128(10, 2),
263+
)
264+
.get_input_types()
265+
.unwrap_err()
266+
.to_string();
267+
assert_contains!(
268+
&err,
269+
"Cannot infer common type for bitwise operation Decimal128(10, 2) & Decimal128(10, 2)"
270+
);
271+
272+
let dict_int8 = DataType::Dictionary(DataType::Int8.into(), DataType::Int8.into());
273+
test_coercion_binary_rule!(dict_int8, dict_int8, Operator::BitwiseAnd, dict_int8);
274+
275+
Ok(())
276+
}
277+
231278
fn test_math_decimal_coercion_rule(
232279
lhs_type: DataType,
233280
rhs_type: DataType,

datafusion/sqllogictest/test_files/scalar.slt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,6 +1311,14 @@ select a << b, c << d, e << f from signed_integers;
13111311
33554432 123 10485760
13121312
NULL NULL NULL
13131313

1314+
## bitwise operations should reject non-integer types
1315+
1316+
query error DataFusion error: Error during planning: Cannot infer common type for bitwise operation Float32 & Float32
1317+
select arrow_cast(1, 'Float32') & arrow_cast(2, 'Float32');
1318+
1319+
query error DataFusion error: Error during planning: Cannot infer common type for bitwise operation Date32 & Date32
1320+
select arrow_cast(1, 'Date32') & arrow_cast(2, 'Date32');
1321+
13141322
statement ok
13151323
drop table unsigned_integers;
13161324

0 commit comments

Comments
 (0)