Copilot commented on code in PR #2453:
URL: https://github.com/apache/groovy/pull/2453#discussion_r3061428291
##########
subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/runtime/Queryable.java:
##########
@@ -245,6 +245,30 @@ default Queryable<Tuple2<?, Queryable<T>>>
groupBy(Function<? super T, ?> classi
return groupBy(classifier, null);
}
+ /**
+ * Group by {@link Queryable} instance, returning {@link GroupResult}
instances
+ * for use with the {@code groupby...into} syntax.
+ *
+ * @param classifier the classifier for group by
+ * @param having the filter condition (may be null)
+ * @param <K> the type of the group key
+ * @return the result of group by as GroupResult instances
+ * @since 5.0.0
+ */
+ <K> Queryable<GroupResult<K, T>> groupByInto(Function<? super T, ? extends
K> classifier, Predicate<? super GroupResult<K, T>> having);
+
+ /**
+ * Group by {@link Queryable} instance without {@code having} clause,
returning {@link GroupResult} instances.
+ *
+ * @param classifier the classifier for group by
+ * @param <K> the type of the group key
+ * @return the result of group by as GroupResult instances
+ * @since 5.0.0
+ */
+ default <K> Queryable<GroupResult<K, T>> groupByInto(Function<? super T, ?
extends K> classifier) {
+ return groupByInto(classifier, null);
+ }
Review Comment:
The new `groupByInto` API introduces `GroupResult` in the public surface
area, but the `GroupResult` type is not present anywhere in the codebase. This
will break compilation for the module and for any consumers; ensure the new
`GroupResult`/implementation classes are included in this PR and exported from
the same package (or add the required imports).
##########
subprojects/groovy-ginq/src/main/groovy/org/apache/groovy/ginq/provider/collection/runtime/QueryableCollection.java:
##########
@@ -262,6 +262,24 @@ public Queryable<Tuple2<?, Queryable<T>>>
groupBy(Function<? super T, ?> classif
return Group.of(stream);
}
+ @Override
+ public <K> Queryable<GroupResult<K, T>> groupByInto(Function<? super T, ?
extends K> classifier, Predicate<? super GroupResult<K, T>> having) {
+ Collector<T, ?, ? extends Map<K, List<T>>> groupingBy =
+ isParallel() ? Collectors.groupingByConcurrent(classifier,
Collectors.toList())
+ : Collectors.groupingBy(classifier,
Collectors.toList());
+
+ // Materialize group elements as lists so they can be iterated
multiple times
+ // (e.g., having g.count() > 1 followed by select g.count())
+ Stream<GroupResult<K, T>> stream =
+ this.stream()
+ .collect(groupingBy)
+ .entrySet().stream()
+ .map(m -> GroupResult.<K, T>of(m.getKey(),
from(m.getValue())))
+ .filter(gr -> null == having || having.test(gr));
+
+ return from(stream);
Review Comment:
`GroupResult` is referenced as a return type and via `GroupResult.of(...)`,
but there is no `GroupResult` type (or `of` factory) present in the repository.
As-is, this will not compile; please add the new
`GroupResult`/`GroupResultImpl` sources to the module (in the same package or
with the needed imports), or adjust the API to use an existing type.
--
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]
