You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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.
Follow-up from review of #4459.
CometRustUDF.registeraccepts adeterministicparameter and carries it in theRustUdfCallproto, but the native side ignores it:ImportedCScalarUdf::try_newhardcodesVolatility::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 = falseat registration, so the flag cannot silently lie. This issue is about actually supporting it.The obstacle is lifetime mismatch. The
Signatureis built once per library load and cached process-wide per(library_path), while determinism is declared perCometRustUDF.registercall. Two registrations of the same kernel with different determinism would need different volatility from one cachedScalarUDFImpl.Options worth weighing:
(library_path, name, volatility)rather than on the library alone.ScalarUDFImplper call site in the planner from the cached kernel, taking volatility fromRustUdfCall.deterministic, and keep only the loaded library in the cache.get_propertyidea in Add a forward-flexible property slot to the Comet UDF C ABI #5254.Once this lands, drop the guard in
CometRustUDF.register, the note on thedeterministicfield inexpr.proto, the "Immutable functions only" limitation indocs/source/user-guide/latest/rust_udfs.md, and the corresponding note on theCometCScalarUdftrait docs.