Skip to content

Commit a16f336

Browse files
committed
feat: propagate result nullability through JvmScalarUdf proto
Add return_nullable to JvmScalarUdf proto and plumb it through to JvmScalarUdfExpr::nullable. Use the source RLike's own nullable on the convert path so downstream operators can apply null-aware optimizations instead of assuming every JVM-UDF call may yield null. Also clean up Hash/PartialEq to delegate to the children's PhysicalExpr impls (matching SubstringExpr) rather than stringifying via Display, and print children using Display in the EXPLAIN output.
1 parent 85029c5 commit a16f336

4 files changed

Lines changed: 18 additions & 8 deletions

File tree

native/core/src/execution/planner.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,7 @@ impl PhysicalPlanner {
715715
udf.class_name.clone(),
716716
args,
717717
return_type,
718+
udf.return_nullable,
718719
)))
719720
}
720721
expr => Err(GeneralError(format!("Not implemented: {expr:?}"))),

native/proto/src/proto/expr.proto

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,4 +516,6 @@ message JvmScalarUdf {
516516
repeated Expr args = 2;
517517
// Expected return type. Used to import the result FFI_ArrowArray.
518518
DataType return_type = 3;
519+
// Whether the result column may contain nulls.
520+
bool return_nullable = 4;
519521
}

native/spark-expr/src/jvm_udf/mod.rs

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,48 +40,53 @@ pub struct JvmScalarUdfExpr {
4040
class_name: String,
4141
args: Vec<Arc<dyn PhysicalExpr>>,
4242
return_type: DataType,
43+
return_nullable: bool,
4344
}
4445

4546
impl JvmScalarUdfExpr {
4647
pub fn new(
4748
class_name: String,
4849
args: Vec<Arc<dyn PhysicalExpr>>,
4950
return_type: DataType,
51+
return_nullable: bool,
5052
) -> Self {
5153
Self {
5254
class_name,
5355
args,
5456
return_type,
57+
return_nullable,
5558
}
5659
}
5760
}
5861

5962
impl Display for JvmScalarUdfExpr {
6063
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
61-
write!(f, "JvmScalarUdf({}, args={:?})", self.class_name, self.args)
64+
write!(f, "JvmScalarUdf({}", self.class_name)?;
65+
for a in &self.args {
66+
write!(f, ", {a}")?;
67+
}
68+
write!(f, ")")
6269
}
6370
}
6471

6572
impl Hash for JvmScalarUdfExpr {
6673
fn hash<H: Hasher>(&self, state: &mut H) {
6774
self.class_name.hash(state);
6875
for a in &self.args {
69-
format!("{}", a).hash(state);
76+
a.hash(state);
7077
}
7178
self.return_type.hash(state);
79+
self.return_nullable.hash(state);
7280
}
7381
}
7482

7583
impl PartialEq for JvmScalarUdfExpr {
7684
fn eq(&self, other: &Self) -> bool {
7785
self.class_name == other.class_name
7886
&& self.return_type == other.return_type
87+
&& self.return_nullable == other.return_nullable
7988
&& self.args.len() == other.args.len()
80-
&& self
81-
.args
82-
.iter()
83-
.zip(other.args.iter())
84-
.all(|(a, b)| format!("{}", a) == format!("{}", b))
89+
&& self.args.iter().zip(&other.args).all(|(a, b)| a.eq(b))
8590
}
8691
}
8792

@@ -101,7 +106,7 @@ impl PhysicalExpr for JvmScalarUdfExpr {
101106
}
102107

103108
fn nullable(&self, _input_schema: &Schema) -> DFResult<bool> {
104-
Ok(true)
109+
Ok(self.return_nullable)
105110
}
106111

107112
fn evaluate(&self, batch: &RecordBatch) -> DFResult<ColumnarValue> {
@@ -229,6 +234,7 @@ impl PhysicalExpr for JvmScalarUdfExpr {
229234
self.class_name.clone(),
230235
children,
231236
self.return_type.clone(),
237+
self.return_nullable,
232238
)))
233239
}
234240
}

spark/src/main/scala/org/apache/comet/serde/strings.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,7 @@ object CometRLike extends CometExpressionSerde[RLike] {
344344
.addArgs(subjectProto.get)
345345
.addArgs(patternProto.get)
346346
.setReturnType(returnType)
347+
.setReturnNullable(expr.nullable)
347348
Some(
348349
ExprOuterClass.Expr
349350
.newBuilder()

0 commit comments

Comments
 (0)