MaxGekk commented on PR #56869: URL: https://github.com/apache/spark/pull/56869#issuecomment-4840417797
Thanks for the careful review! Addressed in `9cb1be0`: 1. **Arg-error invariant** -- `SECTION 17e` asserts the built-in's argument error wins over a compatible-arity same-named session function: with `system.builtin` first, `abs(1, 2)` hits the built-in `abs` and throws, while `session.abs(1, 2)` stays reachable -- locking the "fast path and slow loop fail on the same candidate" invariant. 2. **Fresh-context recompute** -- `SECTION 17f` covers the recompute trigger `17a` didn't: a SQL-function body whose pinned path differs from the caller's (built-in vs. a catalog-first shadowing function), so one statement yields both resolutions. 3. **Cross-reference comment** -- added a bidirectional note between the fast-path and `resolveFunctionCandidate`'s `system.builtin.<name>` branch that they must stay equivalent. (Per @cloud-fan's suggestion the per-pass memo also moved onto `AnalysisContext`, which removed the `ThreadLocal`/`WeakReference` machinery.) -- 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]
