You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/08/13 01:21:22 UTC

[GitHub] [incubator-pinot] Jackie-Jiang opened a new pull request #5856: [Post-Aggregation] Support post-aggregation in ORDER-BY

Jackie-Jiang opened a new pull request #5856:
URL: https://github.com/apache/incubator-pinot/pull/5856


   ## Description
   Enhance `TableResizer` to support post-aggregation in ORDER-BY clause


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

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



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


[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #5856: [Post-Aggregation] Support post-aggregation in ORDER-BY

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #5856:
URL: https://github.com/apache/incubator-pinot/pull/5856


   


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

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



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


[GitHub] [incubator-pinot] npawar commented on a change in pull request #5856: [Post-Aggregation] Support post-aggregation in ORDER-BY

Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #5856:
URL: https://github.com/apache/incubator-pinot/pull/5856#discussion_r471190549



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java
##########
@@ -98,14 +93,37 @@
     };
   }
 
+  /**
+   * Helper method to construct a OrderByValueExtractor based on the given expression.
+   */
+  private OrderByValueExtractor getOrderByValueExtractor(ExpressionContext expression) {
+    if (expression.getType() == ExpressionContext.Type.LITERAL) {

Review comment:
       what does it mean to have ORDER BY a literal? can you give an example?

##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java
##########
@@ -28,66 +28,61 @@
 import java.util.List;
 import java.util.Map;
 import java.util.PriorityQueue;
-import java.util.function.Function;
 import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
 import org.apache.pinot.core.query.aggregation.function.AggregationFunction;
+import org.apache.pinot.core.query.postaggregation.PostAggregationFunction;
+import org.apache.pinot.core.query.request.context.ExpressionContext;
+import org.apache.pinot.core.query.request.context.FunctionContext;
 import org.apache.pinot.core.query.request.context.OrderByExpressionContext;
+import org.apache.pinot.core.query.request.context.QueryContext;
+import org.apache.pinot.spi.utils.ByteArray;
 
 
 /**
  * Helper class for trimming and sorting records in the IndexedTable, based on the order by information
  */
 @SuppressWarnings({"rawtypes", "unchecked"})
 public class TableResizer {
+  private final DataSchema _dataSchema;
+  private final int _numGroupByExpressions;
+  private final Map<ExpressionContext, Integer> _groupByExpressionIndexMap;
+  private final AggregationFunction[] _aggregationFunctions;
+  private final Map<FunctionContext, Integer> _aggregationFunctionIndexMap;
+  private final int _numOrderByExpressions;
   private final OrderByValueExtractor[] _orderByValueExtractors;
   private final Comparator<IntermediateRecord> _intermediateRecordComparator;
-  private final int _numOrderByExpressions;
 
-  TableResizer(DataSchema dataSchema, AggregationFunction[] aggregationFunctions,
-      List<OrderByExpressionContext> orderByExpressions) {
+  public TableResizer(DataSchema dataSchema, QueryContext queryContext) {
+    _dataSchema = dataSchema;
 
-    // NOTE: the assumption here is that the key columns will appear before the aggregation columns in the data schema
-    // This is handled in the only in the AggregationGroupByOrderByOperator for now
+    // NOTE: The data schema will always have group-by expressions in the front, followed by aggregation functions of
+    //       the same order as in the query context. This is handled in AggregationGroupByOrderByOperator.
 
-    int numColumns = dataSchema.size();
-    int numAggregations = aggregationFunctions.length;
-    int numKeyColumns = numColumns - numAggregations;
-
-    Map<String, Integer> columnIndexMap = new HashMap<>();
-    Map<String, AggregationFunction> aggregationColumnToFunction = new HashMap<>();
-    for (int i = 0; i < numColumns; i++) {
-      String columnName = dataSchema.getColumnName(i);
-      columnIndexMap.put(columnName, i);
-      if (i >= numKeyColumns) {
-        aggregationColumnToFunction.put(columnName, aggregationFunctions[i - numKeyColumns]);
-      }
+    List<ExpressionContext> groupByExpressions = queryContext.getGroupByExpressions();
+    assert groupByExpressions != null;

Review comment:
       why using assert instead of Preconditions.checkState?




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

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



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


[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #5856: [Post-Aggregation] Support post-aggregation in ORDER-BY

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #5856:
URL: https://github.com/apache/incubator-pinot/pull/5856#discussion_r471651295



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/table/TableResizer.java
##########
@@ -98,14 +93,37 @@
     };
   }
 
+  /**
+   * Helper method to construct a OrderByValueExtractor based on the given expression.
+   */
+  private OrderByValueExtractor getOrderByValueExtractor(ExpressionContext expression) {
+    if (expression.getType() == ExpressionContext.Type.LITERAL) {

Review comment:
       Order-by with literal itself does not make a lot of sense, but post-aggregation might contain literal (e.g. `ORDER BY SUM(col1) * 2 - SUM(col2)`)




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

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



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