kevinwilfong opened a new pull request, #13105:
URL: https://github.com/apache/gluten/pull/13105
## What changes are proposed in this pull request?
A UdfEntry restates a signature that the registered Velox function has
already declared. The two are written separately and can disagree, and a
function can only be called with the type combinations the library thought to
list. For a UDAF the intermediate type is restated too, and a wrong one stays
invisible until a partial aggregation exchanges a state the other side cannot
read.
To improve on this, this change adds a second kind of entry that carries
only a name. RegistryUdfEntry and RegistryUdafEntry are declared through their
own symbol pairs (getNumRegistryUdf / getRegistryUdfEntries and the UDAF
equivalents), so existing libraries keep their layout and need no rebuild. The
library still registers the function through registerUdf().
When a call to such a function is planned, UDFResolver asks Velox to bind
the actual argument types: resolveFunction for a scalar, and a SignatureBinder
pass over the registered aggregate signatures for a UDAF, which yields the
return and intermediate types from the same bound signature. Results are
memoized per (name, argument types), since this runs once per call site during
analysis on the driver, where the libraries are already loaded.
This is worth doing for any function, and it is the only practical option
for one whose signature carries type variables -- array(T) -> T, or (K, V) ->
map(K,V) -- where a UdfEntry means one entry per type combination.
Names declared this way join UDFNames / UDAFNames, so the existing offload
gates -- VeloxHiveUDFTransformer, getFunctionDescriptions,
HashAggregateExecTransformer and the AggregateRel validator -- pick them up
unchanged. A UdfEntry still wins over a by-name declaration of the same name,
so a library can pin one call shape by hand and leave the rest to Velox.
Binding is exact: Velox does not support coercion for signatures carrying type
variables, so udfAllowTypeConversion does not extend to them, and a call that
binds to nothing falls back to the JVM as before.
## How was this patch tested?
**`UDFResolverSuite`** — the resolver in isolation, no native library loaded
(4 new):
- a UDF and a UDAF declared by name alone land in `UDFNames` / `UDAFNames`,
so they reach
the existing offload gates
- an unregistered name is still reported as unsupported. `getUdfExpression`
used to fail
inside `UDFMap.getOrElse`; a by-name declaration has no `UDFMap` entry, so
the miss is now
caught after binding and both paths are pinned
**`VeloxUdfSuiteLocal`** — against `libmyudf` / `libmyudaf` loaded through
`spark.gluten.sql.columnar.backend.velox.udfLibraryPaths` (6 new). This
exercises the whole
path: declaration read from the `.so`, registration over JNI, binding
against the Velox
registry, and the resulting plan.
- `myudf_map_cardinality` on `map<string,double>`: offloads to
`ProjectExecTransformer` and
resolves a `bigint` return type, with no signature ever stated to Gluten
- the same on `map<string,array<bigint>>` — a nested value type, the shape
an enumerated
list of signatures would have to spell out
- a call on an `array` argument binds to nothing and is rejected; the test
walks the cause
chain and requires the resolver's own `GlutenNotSupportException`, so it
cannot pass on an
unrelated analysis failure
- `myudaf_arbitrary` resolves both its return type and its aggregation
buffer from the bound
signature, over `bigint`, `string` and a nested map, and reports a
wrong-arity call
- a grouped aggregation over `myudaf_arbitrary` splits into partial and
final stages around a
shuffle, asserting two `HashAggregateExecTransformer` nodes with a
`ShuffleExchangeLike`
between them and the correct answers
## Was this patch authored or co-authored using generative AI tooling?
co-authored with Claude Opus 5
--
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]
