andygrove opened a new issue, #5249: URL: https://github.com/apache/datafusion-comet/issues/5249
Follow-up from review of #4459. `CometRustUDF.register` accepts a `deterministic` parameter and carries it in the `RustUdfCall` proto, but the native side ignores it: `ImportedCScalarUdf::try_new` hardcodes `Volatility::Immutable`. A nondeterministic UDF planned as immutable may be constant-folded, evaluated once and reused, or eliminated as a common subexpression. #4459 closes the hole by rejecting `deterministic = false` at registration, so the flag cannot silently lie. This issue is about actually supporting it. The obstacle is lifetime mismatch. The `Signature` is built once per library load and cached process-wide per `(library_path)`, while determinism is declared per `CometRustUDF.register` call. Two registrations of the same kernel with different determinism would need different volatility from one cached `ScalarUDFImpl`. Options worth weighing: - Key the cached adapter on `(library_path, name, volatility)` rather than on the library alone. - Build the `ScalarUDFImpl` per call site in the planner from the cached kernel, taking volatility from `RustUdfCall.deterministic`, and keep only the loaded library in the cache. - Let the kernel declare its own volatility over the ABI and validate the registration against it, which overlaps with the `get_property` idea in #REPLACE_PROPERTY. Once this lands, drop the guard in `CometRustUDF.register`, the note on the `deterministic` field in `expr.proto`, the "Immutable functions only" limitation in `docs/source/user-guide/latest/rust_udfs.md`, and the corresponding note on the `CometCScalarUdf` trait docs. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
