szehon-ho commented on code in PR #55885:
URL: https://github.com/apache/spark/pull/55885#discussion_r3262523236
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/functions/ReducibleFunction.java:
##########
@@ -60,6 +60,53 @@
@Evolving
public interface ReducibleFunction<I, O> {
+ /**
+ * Generic reducer for parameterized functions (bucket, truncate, etc.).
+ *
+ * If this function is 'reducible' on another function, return the {@link
Reducer}.
+ * <p>
+ * This method supports functions with any number of parameters of any type.
+ * <p>
+ * Examples:
+ * <ul>
+ * <li>bucket(4, x) and bucket(2, x):
+ * <br>thisParams = [4], otherParams = [2]
+ * <br>Extract with: thisParams.getInt(0), otherParams.getInt(0)
+ * </li>
+ * <li>truncate(x, 3) and truncate(x, 5):
+ * <br>thisParams = [3], otherParams = [5]
+ * <br>Extract with: thisParams.getInt(0), otherParams.getInt(0)
+ * </li>
+ * <li>hypothetical range_bucket(x, 0L, 100L, 4):
+ * <br>thisParams = [0L, 100L, 4]
+ * <br>Extract with: thisParams.getLong(0), thisParams.getLong(1),
thisParams.getInt(2)
+ * </li>
+ * </ul>
+ *
+ * @param thisParams parameters for this function
+ * @param otherFunction the other parameterized function
+ * @param otherParams parameters for the other function
+ * @return a reduction function if reducible, null otherwise
+ * @since 4.0.0
+ */
+ default Reducer<I, O> reducer(
+ ReducibleParameters thisParams,
+ ReducibleFunction<?, ?> otherFunction,
+ ReducibleParameters otherParams) {
+ // Default: try old Int-based API for backward compatibility
+ if (thisParams.count() == 1 && otherParams.count() == 1) {
Review Comment:
i think it makes more sense to have it the other way. the old one should
call the new (more generic one). we can have the temporary code on spark side:
```
val thisParams = extractParameters(thisExpr)
val otherParams = extractParameters(otherExpr)
if (singleIntParam(thisParams) && singleIntParam(otherParams)) {
Option(thisFunction.reducer(thisParams.getInt(0), otherFunction,
otherParams.getInt(0)))
.orElse(Option(thisFunction.reducer(thisParams, otherFunction,
otherParams)))
} else if (!thisParams.isEmpty && !otherParams.isEmpty) {
Option(thisFunction.reducer(thisParams, otherFunction, otherParams))
} else {
Option(thisFunction.reducer(otherFunction))
}
```
imo, its not much uglier than this code :)
In practice, Iceberg is the only implementation of this so we can hopefully
remove this ugly wrapper from spark side.
--
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]
