Copilot commented on code in PR #1903:
URL: https://github.com/apache/auron/pull/1903#discussion_r2882406305
##########
native-engine/datafusion-ext-functions/src/lib.rs:
##########
@@ -86,5 +93,55 @@ pub fn create_auron_ext_function(
}
"Spark_IsNaN" => Arc::new(spark_isnan::spark_isnan),
_ => df_unimplemented_err!("spark ext function not implemented:
{name}")?,
+ };
Review Comment:
This still constructs a fresh `Arc` for each `ScalarFunctionImplementation`
via `Arc::new(...)` on every call. If DataFusion equality/dedup relies on
`Arc::ptr_eq` for the function implementation (as described in #1902),
logically identical ext functions will still compare unequal and won’t be
deduplicated. Consider caching/reusing a single `Arc` per function (e.g.,
`static OnceCell<ScalarFunctionImplementation>` per match arm or a global map
keyed by `name`) and cloning that `Arc` here instead of allocating a new one
each time.
##########
native-engine/auron-planner/src/planner.rs:
##########
@@ -906,17 +906,12 @@ impl PhysicalPlanner {
let fun_name = &e.name;
let fun =
datafusion_ext_functions::create_auron_ext_function(
fun_name,
- self.partition_id,
- )?;
- Arc::new(create_udf(
- &format!("spark_ext_function_{fun_name}"),
args.iter()
.map(|e| e.data_type(input_schema))
.collect::<Result<Vec<_>, _>>()?,
convert_required!(e.return_type)?,
- Volatility::Volatile,
- fun,
- ))
+ )?;
+ Arc::new(ScalarUDF::from(fun))
} else {
Review Comment:
`create_auron_ext_function` returns a new `AuronExtFunction` instance per
invocation, and the planner wraps it in a new `ScalarUDF` each time. If
expression deduplication depends on pointer equality of the UDF/impl (directly
or indirectly), this will still prevent caching of repeated ext function
expressions. Consider reusing a shared `Arc<ScalarUDF>` (or shared impl) per
function signature/name rather than constructing a new one for every parsed
expression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
