[GitHub] [druid] gianm commented on a change in pull request #11930: Scan: Add "orderBy" parameter.

[GitBox <gi...@apache.org>]

gianm commented on a change in pull request #11930:
URL: https://github.com/apache/druid/pull/11930#discussion_r753276862



##########
File path: processing/src/main/java/org/apache/druid/query/scan/ScanQuery.java
##########
@@ -417,10 +542,92 @@ public String toString()
            ", limit=" + scanRowsLimit +
            ", dimFilter=" + dimFilter +
            ", columns=" + columns +
-           ", legacy=" + legacy +
+           (orderBys.isEmpty() ? "" : ", orderBy=" + orderBys) +
+           (legacy == null ? "" : ", legacy=" + legacy) +
+           ", context=" + getContext() +
            '}';
   }
 
+  /**
+   * Verify and reconcile the two ways of specifying ordering: "orderBy", which can refer to any column, and
+   * "order", which refers to the __time column.
+   *
+   * If only "order" is provided, it is returned as-is, along with an equivalent "orderBy".
+   *
+   * If only "orderBy" is provided, it is returned as-is. If it can be converted into an equivalent "order", then that
+   * equivalent "order" is also returned. Otherwise, "orderBy" is returned as-is and "order" is returned as NONE.
+   *
+   * If both "orderBy" and "order" are provided, this returns them as-is if they are compatible, or throws an
+   * exception if they are incompatible.
+   *
+   * @param orderByFromUser "orderBy" specified by the user (can refer to any column)
+   * @param orderFromUser   "order" specified by the user (refers to time order)
+   */
+  private static Pair<List<OrderBy>, Order> verifyAndReconcileOrdering(
+      @Nullable final List<OrderBy> orderByFromUser,
+      @Nullable final Order orderFromUser
+  )
+  {
+    final List<OrderBy> orderByRetVal;
+    final Order orderRetVal;
+
+    // Compute the returned orderBy.
+    if (orderByFromUser != null) {
+      orderByRetVal = orderByFromUser;
+    } else if (orderFromUser == null || orderFromUser == Order.NONE) {
+      orderByRetVal = Collections.emptyList();
+    } else {
+      orderByRetVal = Collections.singletonList(new OrderBy(ColumnHolder.TIME_COLUMN_NAME, orderFromUser));
+    }
+
+    // Compute the returned order.
+    orderRetVal = computeTimeOrderFromOrderBys(orderByRetVal);

Review comment:
       > I guess you didn't want to change the scan engine / query runner factory to work on OrderBy instead of Order and just do the checking there?
   
   Yeah, I wanted to leave that logic as-is for now, but then go in and change it in a later patch.




-- 
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-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org