metanil commented on PR #55885:
URL: https://github.com/apache/spark/pull/55885#issuecomment-4920122511
**Reducer dispatch redesign: `Outcome` ADT + generalized-first**
Buidling on @szehon-ho's three-state suggestion, I made the whole dispatch a
small state machine — the previous flag-based version had become whack-a-mole.
`TransformExpression.reducer` now:
1. probes the generalized `reducer(Literal[], ...)` overload **first**;
2. falls back to the deprecated `reducer(int, ..., int)` overload **only**
when the generalized one is unimplemented (throws
`UnsupportedOperationException`) **and** each side has a single non-null
integer parameter;
3. treats every other generalized outcome as authoritative.
So,
- the deprecated overload is **no longer tried first** — the generalized
one is;
- a `UnsupportedOperationException` from the generalized overload **is**
now exactly what triggers the deprecated fallback (single-int only), rather
than never;
- "no deprecated probe for other shapes" and "all-UOE → `implements no
reducer` warning" still hold.
@sunchao The Javadoc now documents that order and the terminal behavior:
> Returning `null` means "not reducible for these parameters" and is
authoritative: Spark consults no other overload. Dispatch order: Spark tries
this generalized overload first; only if it is not implemented (throws
UnsupportedOperationException`) and each side has a single non-null integer
parameter does Spark fall back to the deprecated `reducer(int,
ReducibleFunction, int)` overload. If every eligible overload throws
`UnsupportedOperationException`, Spark ogs an "implements no reducer" warning;
any other exception is logged and the pair is treated as not reducible (the
join falls back to a shuffle).
**One deliberate behavior change to call out:** a `null` from the
generalized overload is now **authoritative** — Spark does not fall back to the
deprecated overload on `null`, only on `UnsupportedOperationException`. So a
dual-API connector hose generalized overload returns `null` for a single-int
pair its deprecated overload would reduce now shuffles instead. It also
tightens up the null-handling from our earlier `Try.orElse` note — null vs UOE
vs other-exception are now distinct, explicit outcomes rather than collapsing
through `Try`.
--
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]
