walterddr commented on code in PR #10819:
URL: https://github.com/apache/pinot/pull/10819#discussion_r1212172231
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/plannode/MailboxReceiveNode.java:
##########
@@ -65,15 +69,26 @@ public MailboxReceiveNode(int planFragmentId, DataSchema
dataSchema, int senderS
_exchangeType = exchangeType;
_partitionKeySelector = partitionKeySelector;
if (!CollectionUtils.isEmpty(fieldCollations)) {
- _collationKeys = new ArrayList<>(fieldCollations.size());
- _collationDirections = new ArrayList<>(fieldCollations.size());
+ int numCollations = fieldCollations.size();
+ _collationKeys = new ArrayList<>(numCollations);
+ _collationDirections = new ArrayList<>(numCollations);
+ _collationNullDirections = new ArrayList<>(numCollations);
for (RelFieldCollation fieldCollation : fieldCollations) {
- _collationDirections.add(fieldCollation.getDirection());
_collationKeys.add(new
RexExpression.InputRef(fieldCollation.getFieldIndex()));
+ Direction direction = fieldCollation.getDirection();
+ Preconditions.checkArgument(direction == Direction.ASCENDING ||
direction == Direction.DESCENDING,
+ "Unsupported ORDER-BY direction: %s", direction);
Review Comment:
unnecessary precondition check? or this is a null check?
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/parser/CalciteRexExpressionParser.java:
##########
@@ -99,31 +100,42 @@ private static List<Expression>
convertDistinctSelectList(RexExpression.Function
return selectExpr;
}
- public static List<Expression> convertOrderByList(List<RexExpression>
rexInputRefs,
- List<RelFieldCollation.Direction> directions, PinotQuery pinotQuery) {
- List<Expression> orderByExpr = new ArrayList<>();
-
- for (int i = 0; i < rexInputRefs.size(); i++) {
- orderByExpr.add(convertOrderBy(rexInputRefs.get(i), directions.get(i),
pinotQuery));
+ public static List<Expression> convertOrderByList(SortNode node, PinotQuery
pinotQuery) {
+ List<RexExpression> collationKeys = node.getCollationKeys();
+ List<RelFieldCollation.Direction> collationDirections =
node.getCollationDirections();
+ List<RelFieldCollation.NullDirection> collationNullDirections =
node.getCollationNullDirections();
+ int numKeys = collationKeys.size();
+ List<Expression> orderByExpressions = new ArrayList<>(numKeys);
+ for (int i = 0; i < numKeys; i++) {
+ orderByExpressions.add(
+ convertOrderBy(collationKeys.get(i), collationDirections.get(i),
collationNullDirections.get(i), pinotQuery));
}
- return orderByExpr;
+ return orderByExpressions;
}
private static Expression convertOrderBy(RexExpression rexNode,
RelFieldCollation.Direction direction,
- PinotQuery pinotQuery) {
- Expression expression;
- switch (direction) {
- case DESCENDING:
- expression = getFunctionExpression("DESC");
- expression.getFunctionCall().addToOperands(toExpression(rexNode,
pinotQuery));
- break;
- case ASCENDING:
- default:
- expression = getFunctionExpression("ASC");
- expression.getFunctionCall().addToOperands(toExpression(rexNode,
pinotQuery));
- break;
+ RelFieldCollation.NullDirection nullDirection, PinotQuery pinotQuery) {
+ if (direction == RelFieldCollation.Direction.ASCENDING) {
+ Expression expression = getFunctionExpression("asc");
+ expression.getFunctionCall().addToOperands(toExpression(rexNode,
pinotQuery));
+ if (nullDirection == RelFieldCollation.NullDirection.FIRST) {
+ Expression nullFirstExpression = getFunctionExpression("nullsfirst");
+ nullFirstExpression.getFunctionCall().addToOperands(expression);
+ return nullFirstExpression;
+ } else {
+ return expression;
+ }
+ } else {
+ Expression expression = getFunctionExpression("desc");
+ expression.getFunctionCall().addToOperands(toExpression(rexNode,
pinotQuery));
+ if (nullDirection == RelFieldCollation.NullDirection.LAST) {
+ Expression nullFirstExpression = getFunctionExpression("nullslast");
Review Comment:
`nullLastExpression`?
also it means nullFirst/nullLast is added upon request only if the default
behavior is different from Postgres --> if so we should add comment to clarify
that.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
