adjust

apache · Jun 12, 2024 · 3858f36 · 3858f36
1 parent 5f60f65
commit 3858f36
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 35 deletions.
diff --git a/processing/src/main/java/org/apache/druid/segment/CursorMaker.java b/processing/src/main/java/org/apache/druid/segment/CursorMaker.java
@@ -28,19 +28,38 @@
 
 public interface CursorMaker
 {
-  default boolean canVectorize()
-  {
-    return false;
-  }
-
+  /**
+   * Create a {@link Sequence} of {@link Cursor} for use with non-vectorized query engines. Each {@link Cursor} of the
+   * sequence corresponds to a {@link org.apache.druid.java.util.common.granularity.Granularity} bucket determined by
+   * {@link CursorBuildSpec#getGranularity()}.
+   * <p>
+   * Consuming this {@link Sequence} will automatically close all resources associated with this {@link CursorMaker}
+   * so calling {@link #cleanup()} is not needed.
+   */
   Sequence<Cursor> makeCursors();
 
+  /**
+   * Create a {@link VectorCursor} for use with vectorized query engines.
+   * <p>
+   * Advancing this {@link VectorCursor} to the end or explicitly calling {@link VectorCursor#close()} will
+   * automatically close all resources associated with this {@link CursorMaker} so calling {@link #cleanup()} is not
+   * needed.
+   */
   @Nullable
   default VectorCursor makeVectorCursor()
   {
     throw new UOE("Cannot vectorize. Check 'canVectorize' before calling 'makeVectorCursor' on %s.", this.getClass().getName());
   }
 
+  /**
+   * Returns true if this {@link CursorMaker} supports creating vectorized selectors. This operation may acquire
+   * underlying resources, so calling {@link #cleanup()} is necessary if no cursors are created and consumed.
+   */
+  default boolean canVectorize()
+  {
+    return false;
+  }
+
   /**
    * Release any resources acquired if cursors are not consumed. Typically consuming a cursor or vector cursor releases
    * the resources upon completion, but if for some reason this will not happen, this method must be called.

diff --git a/processing/src/main/java/org/apache/druid/segment/QueryableIndexStorageAdapter.java b/processing/src/main/java/org/apache/druid/segment/QueryableIndexStorageAdapter.java
@@ -196,9 +196,9 @@ public boolean canVectorize(
       final boolean descending
   )
   {
-    // todo (clint):
-    // this uses the old-school canVectorize implementation instead of delegating to the CursorMaker, because
-    // the cursor maker expects to make cursors one way or another, and so opens stuff, so need to fix some tests
+    // For safety, this uses the old-school canVectorize implementation instead of delegating to the CursorMaker,
+    // because QueryableIndexCursorMaker expects to make cursors one way or another and so opens stuff that must be
+    // cleaned if for some reason a cursor or vector cursor is not constructed
     if (filter != null) {
 
       final boolean filterCanVectorize = filter.canVectorizeMatcher(this);

diff --git a/processing/src/main/java/org/apache/druid/segment/UnnestStorageAdapter.java b/processing/src/main/java/org/apache/druid/segment/UnnestStorageAdapter.java
@@ -160,15 +160,7 @@ public Sequence<Cursor> makeCursors(
       @Nullable QueryMetrics<?> queryMetrics
   )
   {
-    final CursorBuildSpec buildSpec = CursorBuildSpec.builder()
-                                                     .setFilter(filter)
-                                                     .setInterval(interval)
-                                                     .setGranularity(gran)
-                                                     .setVirtualColumns(virtualColumns)
-                                                     .isDescending(descending)
-                                                     .setQueryMetrics(queryMetrics)
-                                                     .build();
-    return asCursorMaker(buildSpec).makeCursors();
+    return delegateMakeCursorToMaker(filter, interval, virtualColumns, gran, descending, queryMetrics);
   }
 
   @Override

diff --git a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java
@@ -338,14 +338,7 @@ public VectorCursor makeVectorCursor(
       @Nullable QueryMetrics<?> queryMetrics
   )
   {
-    final CursorBuildSpec buildSpec = CursorBuildSpec.builder()
-                                                     .setFilter(filter)
-                                                     .setInterval(interval)
-                                                     .setVirtualColumns(virtualColumns)
-                                                     .isDescending(descending)
-                                                     .setQueryMetrics(queryMetrics)
-                                                     .build();
-    return asCursorMaker(buildSpec).makeVectorCursor();
+    return delegateMakeVectorCursorToMaker(filter, interval, virtualColumns, descending, vectorSize, queryMetrics);
   }
 
   @Override
@@ -359,15 +352,7 @@ public Sequence<Cursor> makeCursors(
   )
   {
 
-    final CursorBuildSpec buildSpec = CursorBuildSpec.builder()
-                                                     .setFilter(filter)
-                                                     .setInterval(interval)
-                                                     .setGranularity(gran)
-                                                     .setVirtualColumns(virtualColumns)
-                                                     .isDescending(descending)
-                                                     .setQueryMetrics(queryMetrics)
-                                                     .build();
-    return asCursorMaker(buildSpec).makeCursors();
+    return delegateMakeCursorToMaker(filter, interval, virtualColumns, gran, descending, queryMetrics);
   }
 
   /**

diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java
@@ -211,7 +211,7 @@ public static ColumnValueSelector<ExprEval> makeExprEvalSelector(
             !ColumnHolder.TIME_COLUMN_NAME.equals(column), // __time doesn't need an LRU cache since it is sorted.
             rowIdSupplier
         );
-      } else if (inputType.is(ValueType.STRING) && plan.is(ExpressionPlan.Trait.SINGLE_INPUT_MAPPABLE)) {
+      } else if (inputType.is(ValueType.STRING)) {
         return new SingleStringInputCachingExpressionColumnValueSelector(
             columnSelectorFactory.makeDimensionSelector(new DefaultDimensionSpec(column, column, ColumnType.STRING)),
             plan.getExpression(),