Jackie-Jiang commented on code in PR #11307:
URL: https://github.com/apache/pinot/pull/11307#discussion_r1334896231
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -940,6 +954,23 @@ private static Set<String>
getSegmentPartitionedColumns(@Nullable TableConfig ta
return segmentPartitionedColumns;
}
+ /**
+ * Retrieve multivalued columns for a table.
+ * From the table Schema , we get the multi valued columns of dimension
fields.
+ *
+ * @param tableSchema
+ * @param columnName
+ * @return multivalued columns of the table .
+ */
+ private static boolean checkIfColumnIsMultiValued(@Nonnull Schema
tableSchema, @Nonnull String columnName) {
Review Comment:
(minor, convention) We don't usually annotate `Nonnull` and assume non-null
unless annotated with `Nullable`. We don't annotate both of them because it is
easy to mix them
```suggestion
private static boolean isMultiValueColumn(Schema schema, String
columnName) {
```
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -1073,6 +1104,50 @@ private static void
handleDistinctCountBitmapOverride(PinotQuery pinotQuery) {
}
}
+ /**
+ * Rewrites selected 'Distinct' prefixed function to 'Distinct----MV'
function for the field of multivalued type.
+ */
+ @VisibleForTesting
+ static void handleDistinctMultiValuedOverride(PinotQuery pinotQuery, Schema
tableSchema) {
+ for (Expression expression : pinotQuery.getSelectList()) {
+ handleDistinctMultiValuedOverride(expression, tableSchema);
+ }
+ List<Expression> orderByExpressions = pinotQuery.getOrderByList();
+ if (orderByExpressions != null) {
+ for (Expression expression : orderByExpressions) {
+ // NOTE: Order-by is always a Function with the ordering of the
Expression
+
handleDistinctMultiValuedOverride(expression.getFunctionCall().getOperands().get(0),
tableSchema);
+ }
+ }
+ Expression havingExpression = pinotQuery.getHavingExpression();
+ if (havingExpression != null) {
+ handleDistinctMultiValuedOverride(havingExpression, tableSchema);
+ }
+ }
+
+ /**
+ * Rewrites selected 'Distinct' prefixed function to 'Distinct----MV'
function for the field of multivalued type.
+ */
+ private static void handleDistinctMultiValuedOverride(Expression expression,
Schema tableSchema) {
+ Function function = expression.getFunctionCall();
+ if (function == null) {
+ return;
+ }
+ if
(DISTINCT_MV_COL_FUNCTION_OVERRIDE_MAP.containsKey(function.getOperator())) {
Review Comment:
To reduce a map lookup, we can use `get()` then check if the return value is
`null`
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -380,6 +389,11 @@ protected BrokerResponse handleRequest(long requestId,
String query, @Nullable S
handleDistinctCountBitmapOverride(serverPinotQuery);
}
+ final Schema tableSchema = _tableCache.getSchema(tableName);
Review Comment:
Move `Schema schema = _tableCache.getSchema(rawTableName);` from line 506
here
--
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]
