gemini-code-assist[bot] commented on code in PR #38832:
URL: https://github.com/apache/beam/pull/38832#discussion_r3365257557
##########
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/UdfImpl.java:
##########
@@ -70,17 +70,30 @@ public static Function create(Method method) {
}
/*
- * Finds a method in a given class by name.
+ * Finds a method in a given class by name. In case of overloaded methods
with the same name,
+ * this prioritizes the overload with the maximum number of parameters. This
ensures Calcite
+ * can resolve optional/default trailing parameters correctly when binding
UDF overloads.
+ *
* @param clazz class to search method in
* @param name name of the method to find
- * @return the first method with matching name or null when no method found
+ * @return the matching method with the highest parameter count or null when
no method found
*/
static @Nullable Method findMethod(Class<?> clazz, String name) {
+ Method bestMethod = null;
for (Method method : clazz.getMethods()) {
if (method.getName().equals(name) && !method.isBridge()) {
- return method;
+ if (bestMethod == null) {
+ bestMethod = method;
+ } else {
+ int cmp =
+ Integer.compare(
+ method.getParameterTypes().length,
bestMethod.getParameterTypes().length);
Review Comment:
Using `method.getParameterTypes().length` allocates a new array on every
call because `getParameterTypes()` returns a clone of the underlying parameter
types array. Since Java 8, you can use `getParameterCount()` which is much more
efficient as it avoids any array allocation.
```suggestion
int cmp =
Integer.compare(
method.getParameterCount(),
bestMethod.getParameterCount());
```
##########
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamSortRel.java:
##########
@@ -40,6 +40,7 @@
import org.apache.beam.sdk.state.ValueState;
import org.apache.beam.sdk.transforms.DoFn;
import org.apache.beam.sdk.transforms.Flatten;
+import org.apache.beam.sdk.transforms.GroupByKey;
Review Comment:
The class `WithKeys` is used in the `expand` method (line 219) but is not
imported in this file. This will cause a compilation error. Please add the
import for `org.apache.beam.sdk.transforms.WithKeys`.
```suggestion
import org.apache.beam.sdk.transforms.GroupByKey;
import org.apache.beam.sdk.transforms.WithKeys;
```
--
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]
