Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]

2024-02-13 Thread via GitHub


Jugen commented on PR #605:
URL: https://github.com/apache/cayenne/pull/605#issuecomment-1943106918

   @stariy95 have added OrderingGroupByStage as suggested.


-- 
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



Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]

2024-02-13 Thread via GitHub


Jugen commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1487416426


##
cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingStage.java:
##
@@ -54,13 +60,15 @@ private void processOrdering(QualifierTranslator 
qualifierTranslator, Translator
 nodeBuilder = function("UPPER", nodeBuilder);
 }
 
-// If query is DISTINCT then we need to add all ORDER BY clauses as 
result columns
-if(!context.isDistinctSuppression()) {
-// TODO: need to check duplicates?
-// need UPPER() function here too, 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);
+// If query is DISTINCT or GROUPING then we need to add all missing 
ORDER BY clauses as result columns

Review Comment:
   It unfortunately can't go into OrderingDistinctStage because this needs to 
happen **before** the GroupByStage.
   I can move it into a new OrderingGroupByStage though .



-- 
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