stariy95 commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1485816260
##########
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:
This again breaks stage logic separation, but I don't see a good option here
as with the `hasToManyJoins()` method
##########
cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingStage.java:
##########
@@ -71,4 +85,26 @@ private void processOrdering(QualifierTranslator
qualifierTranslator, Translator
context.getSelectBuilder().orderBy(orderingNodeBuilder);
}
+ private Optional<Node> getResultColumn(TranslatorContext context,
OrderingNodeBuilder orderBuilder) {
+ String orderStr = getSqlString(orderBuilder);
+
+ return context.getResultNodeList().stream()
+ .map(result -> result.getNode())
+ // the column may have an alias so just check that the
orderStr part matches
+ .filter(resultNode ->
getSqlString(node(resultNode)).startsWith(orderStr))
+ .findFirst();
Review Comment:
This seems a bit heavy, but we could worry about performance later.
##########
cayenne/src/main/java/org/apache/cayenne/access/translator/select/DistinctStage.java:
##########
@@ -70,7 +70,7 @@ public void perform(TranslatorContext context) {
context.getSelectBuilder().distinct();
}
- private boolean hasToManyJoin(TranslatorContext context) {
+ static boolean hasToManyJoin(TranslatorContext context) {
Review Comment:
We may move this logic to the `TableTree` class to keep stages as separeted
as possible.
##########
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:
This change is not safe, logic in the `DistinctStage` relies on joins
defined, while ordering expression translation could add mode joins. Probably
we should try to split `OrderingStage` into two parts.
--
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]
