You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2021/11/02 00:55:01 UTC

[pinot] branch master updated: Add getIndexSegment() to the Operator interface (#7674)

This is an automated email from the ASF dual-hosted git repository.

jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 990ae47  Add getIndexSegment() to the Operator interface (#7674)
990ae47 is described below

commit 990ae472df55220d34e69836ab21a37f57a38b29
Author: Xiaotian (Jackie) Jiang <17...@users.noreply.github.com>
AuthorDate: Mon Nov 1 17:54:46 2021 -0700

    Add getIndexSegment() to the Operator interface (#7674)
    
    Add getIndexSegment() to the Operator interface, which can be implemented by the single-segment operators.
    Make MinMaxValueBasedSelectionOrderByCombineOperator to use the interface api instead of casting the operator class so that the logic can be applied to different operators.
---
 .../org/apache/pinot/core/common/Operator.java     | 25 +++++++++++++++++++++-
 .../AcquireReleaseColumnsSegmentOperator.java      | 17 ++++++++++-----
 .../apache/pinot/core/operator/BaseOperator.java   |  9 --------
 ...xValueBasedSelectionOrderByCombineOperator.java |  9 ++++----
 .../core/operator/query/DistinctOperator.java      |  5 +++++
 .../core/operator/query/SelectionOnlyOperator.java |  5 +++++
 .../operator/query/SelectionOrderByOperator.java   |  9 ++++----
 7 files changed, 55 insertions(+), 24 deletions(-)

diff --git a/pinot-core/src/main/java/org/apache/pinot/core/common/Operator.java b/pinot-core/src/main/java/org/apache/pinot/core/common/Operator.java
index e5c36de..6e30b83 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/common/Operator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/common/Operator.java
@@ -19,9 +19,12 @@
 package org.apache.pinot.core.common;
 
 import org.apache.pinot.core.operator.ExecutionStatistics;
+import org.apache.pinot.segment.spi.IndexSegment;
+import org.apache.pinot.spi.annotations.InterfaceAudience;
 import org.apache.pinot.spi.exception.EarlyTerminationException;
 
 
+@InterfaceAudience.Private
 public interface Operator<T extends Block> {
 
   /**
@@ -36,5 +39,25 @@ public interface Operator<T extends Block> {
    */
   T nextBlock();
 
-  ExecutionStatistics getExecutionStatistics();
+  /**
+   * Returns the name of the operator.
+   * NOTE: This method is called for tracing purpose. The sub-class should try to return a constant to avoid the
+   * unnecessary overhead.
+   */
+  String getOperatorName();
+
+  /**
+   * Returns the index segment associated with the operator.
+   */
+  default IndexSegment getIndexSegment() {
+    throw new UnsupportedOperationException();
+  }
+
+  /**
+   * Returns the execution statistics associated with the operator. This method should be called after the operator has
+   * finished execution.
+   */
+  default ExecutionStatistics getExecutionStatistics() {
+    throw new UnsupportedOperationException();
+  }
 }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/AcquireReleaseColumnsSegmentOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/AcquireReleaseColumnsSegmentOperator.java
index 422374f..160c3ca 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/AcquireReleaseColumnsSegmentOperator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/AcquireReleaseColumnsSegmentOperator.java
@@ -18,8 +18,8 @@
  */
 package org.apache.pinot.core.operator;
 
-import org.apache.pinot.core.common.Block;
 import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.operator.blocks.IntermediateResultsBlock;
 import org.apache.pinot.core.plan.PlanNode;
 import org.apache.pinot.segment.spi.FetchContext;
 import org.apache.pinot.segment.spi.IndexSegment;
@@ -35,13 +35,15 @@ import org.apache.pinot.segment.spi.IndexSegment;
  * The reason this is done is the planners access segment buffers,
  * and we need to acquire the segment before any access is made to the buffers.
  */
-public class AcquireReleaseColumnsSegmentOperator extends BaseOperator {
+@SuppressWarnings("unchecked")
+public class AcquireReleaseColumnsSegmentOperator extends BaseOperator<IntermediateResultsBlock> {
   private static final String OPERATOR_NAME = "AcquireReleaseColumnsSegmentOperator";
 
   private final PlanNode _planNode;
   private final IndexSegment _indexSegment;
   private final FetchContext _fetchContext;
-  private Operator _childOperator;
+
+  private Operator<IntermediateResultsBlock> _childOperator;
 
   public AcquireReleaseColumnsSegmentOperator(PlanNode planNode, IndexSegment indexSegment, FetchContext fetchContext) {
     _planNode = planNode;
@@ -53,8 +55,8 @@ public class AcquireReleaseColumnsSegmentOperator extends BaseOperator {
    * Runs the planNode to get the childOperator, and then proceeds with execution.
    */
   @Override
-  protected Block getNextBlock() {
-    _childOperator = _planNode.run();
+  protected IntermediateResultsBlock getNextBlock() {
+    _childOperator = (Operator<IntermediateResultsBlock>) _planNode.run();
     return _childOperator.nextBlock();
   }
 
@@ -78,6 +80,11 @@ public class AcquireReleaseColumnsSegmentOperator extends BaseOperator {
   }
 
   @Override
+  public IndexSegment getIndexSegment() {
+    return _indexSegment;
+  }
+
+  @Override
   public ExecutionStatistics getExecutionStatistics() {
     return _childOperator == null ? new ExecutionStatistics(0, 0, 0, 0) : _childOperator.getExecutionStatistics();
   }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/BaseOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/BaseOperator.java
index 58d7f6c..e99d5c5 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/BaseOperator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/BaseOperator.java
@@ -52,13 +52,4 @@ public abstract class BaseOperator<T extends Block> implements Operator<T> {
 
   // Make it protected because we should always call nextBlock()
   protected abstract T getNextBlock();
-
-  // Enforcing sub-class to implement the getOperatorName(), as they can just return a static final,
-  // as opposed to this super class calling getClass().getSimpleName().
-  public abstract String getOperatorName();
-
-  @Override
-  public ExecutionStatistics getExecutionStatistics() {
-    throw new UnsupportedOperationException();
-  }
 }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/combine/MinMaxValueBasedSelectionOrderByCombineOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/combine/MinMaxValueBasedSelectionOrderByCombineOperator.java
index 515d4b6..a22cbd1 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/combine/MinMaxValueBasedSelectionOrderByCombineOperator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/combine/MinMaxValueBasedSelectionOrderByCombineOperator.java
@@ -34,7 +34,6 @@ import org.apache.pinot.common.request.context.OrderByExpressionContext;
 import org.apache.pinot.common.utils.DataSchema;
 import org.apache.pinot.core.common.Operator;
 import org.apache.pinot.core.operator.blocks.IntermediateResultsBlock;
-import org.apache.pinot.core.operator.query.SelectionOrderByOperator;
 import org.apache.pinot.core.query.request.context.QueryContext;
 import org.apache.pinot.core.query.selection.SelectionOperatorUtils;
 import org.apache.pinot.segment.spi.datasource.DataSourceMetadata;
@@ -82,8 +81,8 @@ public class MinMaxValueBasedSelectionOrderByCombineOperator extends BaseCombine
     String firstOrderByColumn = firstOrderByExpression.getExpression().getIdentifier();
 
     _minMaxValueContexts = new ArrayList<>(_numOperators);
-    for (Operator operator : _operators) {
-      _minMaxValueContexts.add(new MinMaxValueContext((SelectionOrderByOperator) operator, firstOrderByColumn));
+    for (Operator<IntermediateResultsBlock> operator : _operators) {
+      _minMaxValueContexts.add(new MinMaxValueContext(operator, firstOrderByColumn));
     }
     if (firstOrderByExpression.isAsc()) {
       // For ascending order, sort on column min value in ascending order
@@ -289,11 +288,11 @@ public class MinMaxValueBasedSelectionOrderByCombineOperator extends BaseCombine
   }
 
   private static class MinMaxValueContext {
-    final SelectionOrderByOperator _operator;
+    final Operator<IntermediateResultsBlock> _operator;
     final Comparable _minValue;
     final Comparable _maxValue;
 
-    MinMaxValueContext(SelectionOrderByOperator operator, String column) {
+    MinMaxValueContext(Operator<IntermediateResultsBlock> operator, String column) {
       _operator = operator;
       DataSourceMetadata dataSourceMetadata = operator.getIndexSegment().getDataSource(column).getDataSourceMetadata();
       _minValue = dataSourceMetadata.getMinValue();
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/query/DistinctOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/query/DistinctOperator.java
index 00dbe8f..6a4ab0b 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/query/DistinctOperator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/query/DistinctOperator.java
@@ -74,6 +74,11 @@ public class DistinctOperator extends BaseOperator<IntermediateResultsBlock> {
   }
 
   @Override
+  public IndexSegment getIndexSegment() {
+    return _indexSegment;
+  }
+
+  @Override
   public ExecutionStatistics getExecutionStatistics() {
     long numEntriesScannedInFilter = _transformOperator.getExecutionStatistics().getNumEntriesScannedInFilter();
     long numEntriesScannedPostFilter = (long) _numDocsScanned * _transformOperator.getNumColumnsProjected();
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOnlyOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOnlyOperator.java
index 3a76520..b9f9840 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOnlyOperator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOnlyOperator.java
@@ -100,6 +100,11 @@ public class SelectionOnlyOperator extends BaseOperator<IntermediateResultsBlock
   }
 
   @Override
+  public IndexSegment getIndexSegment() {
+    return _indexSegment;
+  }
+
+  @Override
   public ExecutionStatistics getExecutionStatistics() {
     long numEntriesScannedInFilter = _transformOperator.getExecutionStatistics().getNumEntriesScannedInFilter();
     long numEntriesScannedPostFilter = (long) _numDocsScanned * _transformOperator.getNumColumnsProjected();
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOrderByOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOrderByOperator.java
index 9d7109f..bcca4d8 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOrderByOperator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOrderByOperator.java
@@ -163,10 +163,6 @@ public class SelectionOrderByOperator extends BaseOperator<IntermediateResultsBl
     };
   }
 
-  public IndexSegment getIndexSegment() {
-    return _indexSegment;
-  }
-
   @Override
   protected IntermediateResultsBlock getNextBlock() {
     if (_expressions.size() == _orderByExpressions.size()) {
@@ -319,6 +315,11 @@ public class SelectionOrderByOperator extends BaseOperator<IntermediateResultsBl
   }
 
   @Override
+  public IndexSegment getIndexSegment() {
+    return _indexSegment;
+  }
+
+  @Override
   public ExecutionStatistics getExecutionStatistics() {
     long numEntriesScannedInFilter = _transformOperator.getExecutionStatistics().getNumEntriesScannedInFilter();
     int numTotalDocs = _indexSegment.getSegmentMetadata().getTotalDocs();

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