andygrove opened a new pull request, #5773: URL: https://github.com/apache/datafusion-comet/pull/5773
## Which issue does this PR close? Closes #. ## Rationale for this change Spark's `V2ExpressionUtils.resolveScalarFunction` has two lowerings for a DataSourceV2 catalog scalar function. When the per-type implementation class exposes a static `invoke` "magic method" with a matching signature, Spark wraps the call as `StaticInvoke`; when it does not, Spark falls back to `ApplyFunctionExpression` and calls `function.produceResult(row)` reflectively at runtime. The magic method is an optional performance opt-in in the DSv2 API rather than a requirement, so any V2 catalog function -- Iceberg or otherwise -- can land on the second path. Comet only recognized `StaticInvoke`, so a function without a matching magic method fell back to Spark even when we had a native handler for the exact class. Concretely: an Iceberg release (or fork) that omits the magic method on one of its per-type implementations, and any third-party V2 catalog whose `ScalarFunction` implementations do not add the magic method (it is an optional opt-in), land here. Iceberg's per-type implementation classes are the identity carried on both lowerings, so a single set of handlers keyed by class name covers them. ## What changes are included in this PR? - `CometIcebergSystemFunctions.handlers` is now keyed by class name only; both the `StaticInvoke` and `ApplyFunctionExpression` entry points consult the same map. - `CometIcebergParameterizedTransform` and `CometIcebergTemporalTransform` extract `(arguments, dataType)` through a single `unwrap` helper so a future third lowering has one seam to extend. - New `CometApplyFunctionExpression` serde is registered under `classOf[ApplyFunctionExpression]` in `exprSerdeMap` and routes the second lowering to the same class-name-keyed handler map. ## How are these changes tested? New unit tests in `CometIcebergSystemFunctionSuite`: - `ApplyFunctionExpression reaches the same handlers as StaticInvoke` -- constructs an `ApplyFunctionExpression` wrapping Iceberg's `BucketFunction$BucketInt` and verifies `getSupportLevel` returns `Compatible` for a valid call and `Unsupported` for a zero-bucket call, matching the guard on the `StaticInvoke` path. - `Iceberg handler map is keyed by Iceberg implementation class names` -- guards against a rename of one of Iceberg's per-type implementation classes silently dropping the native path. The existing `support levels follow Iceberg's bind rules` test continues to exercise the `StaticInvoke` path unchanged; the shared handlers keep both entry points aligned. -- 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]
