gianm commented on code in PR #18521:
URL: https://github.com/apache/druid/pull/18521#discussion_r2361347781
##########
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:
Well, who can say. I'm not aware of plans to make it not true. Right now
there's two implementations of vector cursor factories— regular segments and
columnar frames— and both have `ColumnSelector`.
There's only one implementation of a specialized vectorized virtual column
prior to this patch, `NestedFieldVirtualColumn`. That one doesn't have an
implementation for the `VectorColumnSelectorFactory` based vector selector
methods-- if they ever got called (i.e. if the `ColumnSelector` based one
returned null) an exception would be thrown. So, the one example we have isn't
really compatible with a world where the `ColumnSelector` is optional for
vector cursors.
For these reasons I'd like to keep it required for now.
--
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]
