[
https://issues.apache.org/jira/browse/GROOVY-11915?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18072463#comment-18072463
]
ASF GitHub Bot commented on GROOVY-11915:
-----------------------------------------
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.
> GINQ: Add groupby...into with first-class GroupResult type
> ----------------------------------------------------------
>
> Key: GROOVY-11915
> URL: https://issues.apache.org/jira/browse/GROOVY-11915
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> For GINQ, introduce an optional "into <alias>" clause for groupby that binds
> a GroupResult<K, T> variable (extending Queryable<T> with a key property).
> When used, this would replace the current magic aggregate function rewriting
> with real method calls on a first-class type, enabling composable group
> queries (e.g. {{g.where(...).toList()}}), simplifying the correctVars logic
> in GinqAstWalker, and making the backend Queryable API directly usable for
> grouped queries.
> The existing groupby syntax (without "into") would continue to work unchanged.
> If we ever provided a GINQ SQL provider, GroupResult maps cleanly to SQL
> GROUP BY + aggregate functions.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
