andygrove commented on code in PR #4459:
URL: https://github.com/apache/datafusion-comet/pull/4459#discussion_r3736249043
##########
spark/src/test/scala/org/apache/comet/CometRustUdfSuite.scala:
##########
Review Comment:
All three added, and the first one found a bug — details in the thread on
`CometScalaUDF.scala`. Summary:
- **Name collision.** Test written, `ignore`d, because it fails: the Rust
UDF answers the Scala UDF's call (`ArraySeq(0, 1, 2) did not equal List(0, 10,
20)`). The fix isn't available through Spark's public `udf` API, so it's a
follow-up.
- **Concurrent registration.** Added, racing three registrations of
different names and then asserting each name still resolves, plans natively and
returns its own library's answer. I left out a same-name race deliberately:
registration is a driver-side setup call, and racing one name against itself
has no defined winner, so a test on it would only encode whichever ordering
happened to win.
- **`classifyNativeError`.** You were right that a wording change could
silently misclassify, and it was worse than that: matching a bare `"ABI"` meant
*any* library under a directory named `ABI` had its "failed to open" reported
as an ABI mismatch, since every loader message interpolates the path. Now it
matches the message wording (`missing required symbol`, `reports ABI v`, `does
not export`, `' not found in `), with a test per failure mode:
- missing file → `CometRustUdfLoadException`
- a path containing `ABI` → still `CometRustUdfLoadException` (regression
guard for the above)
- `libcomet` itself, which loads but exports no UDF symbols →
`CometRustUdfAbiException`
- a name the library doesn't export → `NoSuchElementException`
The function's scaladoc now names the two native files whose wording it
depends on, so the coupling is at least visible. A proper fix is structured
error codes across the boundary instead of substring matching, which I'd rather
do separately.
Suite is 50 passing, 1 ignored.
--
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]
