LakshSingla commented on code in PR #16714:
URL: https://github.com/apache/druid/pull/16714#discussion_r1671749802
##########
processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/vector/VectorGroupByEngine.java:
##########
@@ -214,35 +214,32 @@ private static boolean canVectorizeDimensions(
final List<DimensionSpec> dimensions
)
{
- return dimensions
- .stream()
- .allMatch(
- dimension -> {
- if (!dimension.canVectorize()) {
- return false;
- }
+ for (DimensionSpec dimension : dimensions) {
+ if (!dimension.canVectorize()) {
+ return false;
+ }
- if (dimension.mustDecorate()) {
- // group by on multi value dimensions are not currently
supported
- // DimensionSpecs that decorate may turn singly-valued columns
into multi-valued selectors.
- // To be safe, we must return false here.
- return false;
- }
+ if (dimension.mustDecorate()) {
+ // group by on multi value dimensions are not currently supported
+ // DimensionSpecs that decorate may turn singly-valued columns into
multi-valued selectors.
+ // To be safe, we must return false here.
+ return false;
+ }
- if (!dimension.getOutputType().isPrimitive()) {
- // group by on arrays and complex types is not currently
supported in the vector processing engine
- return false;
- }
+ if (!dimension.getOutputType().isPrimitive()) {
+ // group by on arrays and complex types is not currently supported in
the vector processing engine
+ return false;
+ }
- // Now check column capabilities.
- final ColumnCapabilities columnCapabilities =
inspector.getColumnCapabilities(dimension.getDimension());
- // null here currently means the column does not exist, nil
columns can be vectorized
- if (columnCapabilities == null) {
- return true;
- }
- // must be single valued
- return columnCapabilities.hasMultipleValues().isFalse();
- });
+ // Now check column capabilities.
+ final ColumnCapabilities columnCapabilities =
inspector.getColumnCapabilities(dimension.getDimension());
+ if (columnCapabilities != null &&
columnCapabilities.hasMultipleValues().isFalse()) {
Review Comment:
Should it have been following? Earlier we used to return true if
`multipleValues = false`. Now, we return false in that case, true otherwise.
```suggestion
if (columnCapabilities != null &&
columnCapabilities.hasMultipleValues().isMaybeTrue()) {
```
--
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]
