Jackie-Jiang commented on code in PR #16312:
URL: https://github.com/apache/pinot/pull/16312#discussion_r2196051540
##########
pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java:
##########
@@ -175,36 +175,10 @@ private boolean isSameOrderAndGroupByColumns(QueryContext
context) {
return false;
}
- BitSet orderByKeysMatched = new BitSet(orderByKeys.size());
-
- OUTER_GROUP:
- for (ExpressionContext groupExp : groupByKeys) {
- for (int i = 0; i < orderByKeys.size(); i++) {
- OrderByExpressionContext orderExp = orderByKeys.get(i);
- if (groupExp.equals(orderExp.getExpression())) {
- orderByKeysMatched.set(i);
- continue OUTER_GROUP;
- }
- }
-
- return false;
- }
-
- OUTER_ORDER:
- for (int i = 0, n = orderByKeys.size(); i < n; i++) {
- if (orderByKeysMatched.get(i)) {
- continue;
- }
-
- for (ExpressionContext groupExp : groupByKeys) {
- if (groupExp.equals(orderByKeys.get(i).getExpression())) {
- continue OUTER_ORDER;
- }
- }
- return false;
- }
-
- return true;
+ Set<ExpressionContext> groupByKeyIdentSet = new HashSet<>(groupByKeys);
Review Comment:
@bziobrowski Is there a specific reason why you are doing 2 loops instead of
comparing the set?
##########
pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java:
##########
@@ -564,7 +538,11 @@ public boolean isIndexUseAllowed(DataSource dataSource,
FieldConfig.IndexType in
}
public boolean isUnsafeTrim() {
- return !isSameOrderAndGroupByColumns(this) || getHavingFilter() != null;
+ if (_isUnsafeTrim != null) {
Review Comment:
We usually pre-compute this during the `build()` call. We can do the same.
Currently this can still be computed multiple times
--
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]
