Re: [PR] [AURON #1902] Fix ext func caching [auron]

["Copilot (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@auron.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org