Skip to content

Commit db2d21e

Browse files
KontinuationJefffreyalamb
authored
fix: preserve async UDF return field metadata (#22663)
## Which issue does this PR close? Closes #22662. ## Rationale for this change Async scalar UDFs can compute output field metadata in `return_field_from_args(...)`, but `AsyncFuncExpr` rebuilt the output field from only name, data type, and nullability. This dropped metadata from async UDF result fields. ## What changes are included in this PR? This PR updates `AsyncFuncExpr::field(...)` to preserve the planned `return_field` metadata and only rename the field for the async expression output. It also adds a regression test that verifies an async UDF result batch preserves metadata attached by `return_field_from_args(...)`. ## Are these changes tested? Yes. Added a regression test in: - `datafusion/core/tests/user_defined/user_defined_async_scalar_functions.rs` The test fails without the fix and passes with the fix. ## Are there any user-facing changes? Yes. Async scalar UDF result fields now preserve metadata attached by `return_field_from_args(...)`. --------- Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
1 parent dae03ee commit db2d21e

3 files changed

Lines changed: 120 additions & 12 deletions

File tree

datafusion/core/tests/user_defined/user_defined_async_scalar_functions.rs

Lines changed: 107 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,15 @@
1818
use std::sync::Arc;
1919

2020
use arrow::array::{Int32Array, RecordBatch, StringArray};
21-
use arrow::datatypes::{DataType, Field, Schema};
21+
use arrow::datatypes::{DataType, Field, FieldRef, Schema};
2222
use async_trait::async_trait;
2323
use datafusion::prelude::*;
2424
use datafusion_common::test_util::format_batches;
2525
use datafusion_common::{Result, assert_batches_eq};
2626
use datafusion_expr::async_udf::{AsyncScalarUDF, AsyncScalarUDFImpl};
2727
use datafusion_expr::{
28-
ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature, Volatility,
28+
ColumnarValue, ReturnFieldArgs, ScalarFunctionArgs, ScalarUDFImpl, Signature,
29+
Volatility,
2930
};
3031

3132
fn register_table_and_udf() -> Result<SessionContext> {
@@ -113,6 +114,110 @@ async fn test_async_udf_metrics() -> Result<()> {
113114
Ok(())
114115
}
115116

117+
#[tokio::test]
118+
async fn test_async_udf_preserves_result_field_metadata() -> Result<()> {
119+
#[derive(Debug, PartialEq, Eq, Hash, Clone)]
120+
struct AsyncExtensionUDF {
121+
signature: Signature,
122+
}
123+
124+
impl Default for AsyncExtensionUDF {
125+
fn default() -> Self {
126+
Self {
127+
signature: Signature::exact(vec![DataType::Utf8], Volatility::Volatile),
128+
}
129+
}
130+
}
131+
132+
impl ScalarUDFImpl for AsyncExtensionUDF {
133+
fn name(&self) -> &str {
134+
"async_extension"
135+
}
136+
137+
fn signature(&self) -> &Signature {
138+
&self.signature
139+
}
140+
141+
fn return_type(&self, _arg_types: &[DataType]) -> Result<DataType> {
142+
Ok(DataType::Utf8)
143+
}
144+
145+
fn return_field_from_args(&self, args: ReturnFieldArgs) -> Result<FieldRef> {
146+
Ok(args.arg_fields[0]
147+
.as_ref()
148+
.clone()
149+
.with_name(self.name())
150+
.with_metadata(std::collections::HashMap::from([(
151+
"ARROW:extension:name".to_string(),
152+
"test.async.extension".to_string(),
153+
)]))
154+
.into())
155+
}
156+
157+
fn invoke_with_args(&self, _args: ScalarFunctionArgs) -> Result<ColumnarValue> {
158+
panic!("Call invoke_async_with_args instead")
159+
}
160+
}
161+
162+
#[async_trait]
163+
impl AsyncScalarUDFImpl for AsyncExtensionUDF {
164+
async fn invoke_async_with_args(
165+
&self,
166+
args: ScalarFunctionArgs,
167+
) -> Result<ColumnarValue> {
168+
Ok(args.args[0].clone())
169+
}
170+
}
171+
172+
let batch = RecordBatch::try_new(
173+
Arc::new(Schema::new(vec![
174+
Field::new("id", DataType::Int32, false),
175+
Field::new("value", DataType::Utf8, false),
176+
])),
177+
vec![
178+
Arc::new(Int32Array::from(vec![1, 2, 3])),
179+
Arc::new(StringArray::from(vec!["one", "two", "three"])),
180+
],
181+
)?;
182+
183+
let ctx = SessionContext::new();
184+
ctx.register_batch("test_table", batch)?;
185+
ctx.register_udf(
186+
AsyncScalarUDF::new(Arc::new(AsyncExtensionUDF::default())).into_scalar_udf(),
187+
);
188+
189+
let result = ctx
190+
.sql("SELECT async_extension(value) AS result FROM test_table")
191+
.await?
192+
.collect()
193+
.await?;
194+
195+
assert_eq!(result[0].schema().field(0).name(), "result");
196+
assert_eq!(
197+
result[0]
198+
.schema()
199+
.field(0)
200+
.metadata()
201+
.get("ARROW:extension:name"),
202+
Some(&"test.async.extension".to_string())
203+
);
204+
205+
assert_batches_eq!(
206+
&[
207+
"+--------+",
208+
"| result |",
209+
"+--------+",
210+
"| one |",
211+
"| two |",
212+
"| three |",
213+
"+--------+",
214+
],
215+
&result
216+
);
217+
218+
Ok(())
219+
}
220+
116221
#[derive(Debug, PartialEq, Eq, Hash, Clone)]
117222
struct TestAsyncUDFImpl {
118223
batch_size: usize,

datafusion/physical-expr/src/async_scalar_function.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,9 @@ impl AsyncFuncExpr {
8888
}
8989

9090
/// Return the output field generated by evaluating this function
91-
pub fn field(&self, input_schema: &Schema) -> Result<Field> {
92-
Ok(Field::new(
93-
&self.name,
94-
self.func.data_type(input_schema)?,
95-
self.func.nullable(input_schema)?,
96-
))
91+
#[deprecated(since = "55.0.0", note = "Use return_field instead")]
92+
pub fn field(&self, _input_schema: &Schema) -> Result<Field> {
93+
Ok(self.return_field.as_ref().clone().with_name(&self.name))
9794
}
9895

9996
/// Return the ideal batch size for this function
@@ -211,6 +208,12 @@ impl PhysicalExpr for AsyncFuncExpr {
211208
self.func.data_type(input_schema)
212209
}
213210

211+
fn return_field(&self, _input_schema: &Schema) -> Result<FieldRef> {
212+
Ok(Arc::new(
213+
self.return_field.as_ref().clone().with_name(&self.name),
214+
))
215+
}
216+
214217
fn nullable(&self, input_schema: &Schema) -> Result<bool> {
215218
self.func.nullable(input_schema)
216219
}

datafusion/physical-plan/src/async_func.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::{
2323
check_if_same_properties,
2424
};
2525
use arrow::array::RecordBatch;
26-
use arrow_schema::{Fields, Schema, SchemaRef};
26+
use arrow_schema::{FieldRef, Fields, Schema, SchemaRef};
2727
use datafusion_common::tree_node::{Transformed, TreeNode, TreeNodeRecursion};
2828
use datafusion_common::{Result, assert_eq_or_internal_err};
2929
use datafusion_execution::{RecordBatchStream, SendableRecordBatchStream, TaskContext};
@@ -61,16 +61,16 @@ impl AsyncFuncExec {
6161
) -> Result<Self> {
6262
let async_fields = async_exprs
6363
.iter()
64-
.map(|async_expr| async_expr.field(input.schema().as_ref()))
65-
.collect::<Result<Vec<_>>>()?;
64+
.map(|async_expr| async_expr.return_field(input.schema().as_ref()))
65+
.collect::<Result<Vec<FieldRef>>>()?;
6666

6767
// compute the output schema: input schema then async expressions
6868
let fields: Fields = input
6969
.schema()
7070
.fields()
7171
.iter()
7272
.cloned()
73-
.chain(async_fields.into_iter().map(Arc::new))
73+
.chain(async_fields)
7474
.collect();
7575

7676
let schema = Arc::new(Schema::new(fields));

0 commit comments

Comments
 (0)