gianm commented on code in PR #18521:
URL: https://github.com/apache/druid/pull/18521#discussion_r2361332424
##########
processing/src/main/java/org/apache/druid/segment/VirtualColumn.java:
##########
@@ -129,11 +192,40 @@ default boolean canVectorize(ColumnInspector inspector)
}
/**
- * Build a {@link SingleValueDimensionVectorSelector} corresponding to this
virtual column. Also provides the name
- * that the virtual column was referenced with (through {@link
DimensionSpec#getDimension()}, which is useful if this
- * column uses dot notation. The virtual column is expected to apply any
necessary decoration from the
- * {@link DimensionSpec}.
+ * Build a selector corresponding to this virtual column.
+ *
+ * The virtual column is expected to apply any necessary {@link
DimensionSpec#decorate(DimensionSelector)} or
+ * {@link DimensionSpec#getExtractionFn()} from the dimensionSpec.
+ *
+ * @param dimensionSpec spec the column was referenced with. Also provides
the name that the
+ * virtual column was referenced with, which is useful
if this column uses dot notation.
+ * @param factory object for fetching underlying selectors.
+ * @param columnSelector object for fetching underlying columns, if
available. Generally only available for
+ * regular segments.
+ * @param offset offset to use with underlying columns. Available
only if columnSelector is available.
*/
+ default SingleValueDimensionVectorSelector
makeSingleValueVectorDimensionSelector(
+ DimensionSpec dimensionSpec,
+ VectorColumnSelectorFactory factory,
+ ColumnSelector columnSelector,
+ ReadableVectorOffset offset
Review Comment:
These were supposed to not be nullable, because the idea is for vectorized
methods, the `ColumnSelector` is always available. I updated the javadoc to
match.
##########
processing/src/main/java/org/apache/druid/segment/VirtualColumns.java:
##########
@@ -297,137 +287,98 @@ public boolean canVectorize(ColumnInspector
columnInspector)
/**
* Create a single value dimension vector (string) selector.
*
- * @throws IllegalArgumentException if the virtual column does not exist
(see {@link #exists(String)}
- */
- public SingleValueDimensionVectorSelector
makeSingleValueDimensionVectorSelector(
- DimensionSpec dimensionSpec,
- VectorColumnSelectorFactory factory
- )
- {
- final VirtualColumn virtualColumn =
getVirtualColumnForSelector(dimensionSpec.getDimension());
- final SingleValueDimensionVectorSelector selector =
virtualColumn.makeSingleValueVectorDimensionSelector(
- dimensionSpec,
- factory
- );
- Preconditions.checkNotNull(selector, "selector");
- return selector;
- }
-
- /**
- * Try to create an optimized single value dimension (string) vector
selector, directly from a
- * {@link ColumnSelector}. If this method returns null, callers should try
to fallback to
- * {@link #makeSingleValueDimensionVectorSelector(DimensionSpec,
VectorColumnSelectorFactory)} instead.
+ * @param dimensionSpec spec the column was referenced with
+ * @param factory object for fetching underlying selectors
+ * @param columnSelector object for fetching underlying columns
+ * @param offset offset to use with underlying columns
*
* @throws IllegalArgumentException if the virtual column does not exist
(see {@link #exists(String)}
*/
- @Nullable
public SingleValueDimensionVectorSelector
makeSingleValueDimensionVectorSelector(
DimensionSpec dimensionSpec,
+ VectorColumnSelectorFactory factory,
ColumnSelector columnSelector,
ReadableVectorOffset offset
Review Comment:
These were supposed to not be nullable, because the idea is for vectorized
methods, the `ColumnSelector` is always available. I updated the javadoc to
match.
--
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]
