stariy95 commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1486041480
##########
cayenne/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java:
##########
@@ -40,11 +40,11 @@ public class DefaultSelectTranslator implements
SelectTranslator {
private static final TranslationStage[] TRANSLATION_STAGES = {
new ColumnExtractorStage(),
new PrefetchNodeStage(),
- new OrderingStage(),
new QualifierTranslationStage(),
new HavingTranslationStage(),
- new GroupByStage(),
new DistinctStage(),
+ new OrderingStage(), // Relies on DistinctStage
Review Comment:
Translation stages are there just to split all the complext logic to
apprehensible chunks. So it should be totally fine to add more stages. This
logic is separated enough, we'll only need a way to read ordering nodes in a
later stage:
```java
if (isDistinctOrGroupByQuery) {
Optional<Node> column = getResultColumn(context,
order(nodeBuilder));
if (!column.isPresent()) {
// deepCopy as some DB expect exactly the same expression in
select and in ordering
ResultNodeDescriptor descriptor =
context.addResultNode(nodeBuilder.build().deepCopy());
if(exp instanceof ASTAggregateFunctionCall) {
descriptor.setAggregate(true);
}
}
}
```
##########
cayenne/src/main/java/org/apache/cayenne/access/translator/select/GroupByStage.java:
##########
@@ -41,7 +41,7 @@ public void perform(TranslatorContext context) {
}
}
- private boolean hasAggregate(TranslatorContext context) {
+ static boolean hasAggregate(TranslatorContext context) {
Review Comment:
Yeah, we may add a flag to the context and keep the logic in the
`GroupByStage`, then it should be decent enough.
--
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: commits-unsubscr...@cayenne.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
