Jackie-Jiang commented on code in PR #9957:
URL: https://github.com/apache/pinot/pull/9957#discussion_r1072950324
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java:
##########
@@ -166,4 +166,28 @@ public static boolean
isServerReturnFinalResult(Map<String, String> queryOptions
public static String getOrderByAlgorithm(Map<String, String> queryOptions) {
return queryOptions.get(QueryOptionKey.ORDER_BY_ALGORITHM);
}
+
+ @Nullable
+ public static Integer getMultiStageLeafLimit(Map<String, String>
queryOptions) {
+ String maxLeafLimitStr =
queryOptions.get(QueryOptionKey.MULTI_STAGE_LEAF_LIMIT);
+ return maxLeafLimitStr != null ? Integer.parseInt(maxLeafLimitStr) : null;
+ }
+
+ @Nullable
+ public static Integer getNumGroupLimit(Map<String, String> queryOptions) {
+ String maxNumGroupLimit =
queryOptions.get(QueryOptionKey.NUM_GROUPS_LIMIT);
+ return maxNumGroupLimit != null ? Integer.parseInt(maxNumGroupLimit) :
null;
+ }
+
+ @Nullable
+ public static Integer getMaxInitResultCap(Map<String, String> queryOptions) {
Review Comment:
```suggestion
public static Integer getMaxInitialResultHolderCapacity(Map<String,
String> queryOptions) {
```
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java:
##########
@@ -166,4 +166,28 @@ public static boolean
isServerReturnFinalResult(Map<String, String> queryOptions
public static String getOrderByAlgorithm(Map<String, String> queryOptions) {
return queryOptions.get(QueryOptionKey.ORDER_BY_ALGORITHM);
}
+
+ @Nullable
+ public static Integer getMultiStageLeafLimit(Map<String, String>
queryOptions) {
+ String maxLeafLimitStr =
queryOptions.get(QueryOptionKey.MULTI_STAGE_LEAF_LIMIT);
+ return maxLeafLimitStr != null ? Integer.parseInt(maxLeafLimitStr) : null;
+ }
+
+ @Nullable
+ public static Integer getNumGroupLimit(Map<String, String> queryOptions) {
+ String maxNumGroupLimit =
queryOptions.get(QueryOptionKey.NUM_GROUPS_LIMIT);
+ return maxNumGroupLimit != null ? Integer.parseInt(maxNumGroupLimit) :
null;
+ }
+
+ @Nullable
+ public static Integer getMaxInitResultCap(Map<String, String> queryOptions) {
+ String maxInitResultCap =
queryOptions.get(QueryOptionKey.MAX_INITIAL_RESULT_HOLDER_CAPACITY);
+ return maxInitResultCap != null ? Integer.parseInt(maxInitResultCap) :
null;
+ }
+
+ @Nullable
+ public static Integer getGroupByTrimThreshold(Map<String, String>
queryOptions) {
Review Comment:
```suggestion
public static Integer getGroupTrimThreshold(Map<String, String>
queryOptions) {
```
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java:
##########
@@ -99,6 +99,14 @@ public void testGeneratedQueries()
super.testGeneratedQueries(false, true);
}
+ @Test
+ public void testQueryOptions()
+ throws Exception {
+ String pinotQuery = "SET multistageLeafLimit = 1; SELECT * FROM mytable;";
+ String h2Query = "SELECT * FROM mytable";
Review Comment:
Hmm, this test cannot prove the option is taking effect. Can you also verify
that the pinot query only returns one record?
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java:
##########
@@ -166,4 +166,28 @@ public static boolean
isServerReturnFinalResult(Map<String, String> queryOptions
public static String getOrderByAlgorithm(Map<String, String> queryOptions) {
return queryOptions.get(QueryOptionKey.ORDER_BY_ALGORITHM);
}
+
+ @Nullable
+ public static Integer getMultiStageLeafLimit(Map<String, String>
queryOptions) {
+ String maxLeafLimitStr =
queryOptions.get(QueryOptionKey.MULTI_STAGE_LEAF_LIMIT);
+ return maxLeafLimitStr != null ? Integer.parseInt(maxLeafLimitStr) : null;
+ }
+
+ @Nullable
+ public static Integer getNumGroupLimit(Map<String, String> queryOptions) {
Review Comment:
Suggest keeping the name consistent for readability, same for other getters
```suggestion
public static Integer getNumGroupsLimit(Map<String, String> queryOptions) {
```
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/maker/InstancePlanMakerImplV2.java:
##########
@@ -209,27 +209,36 @@ private void applyQueryOptions(QueryContext queryContext)
{
// Set group-by query options
if (QueryContextUtils.isAggregationQuery(queryContext) &&
queryContext.getGroupByExpressions() != null) {
-
// Set maxInitialResultHolderCapacity
-
queryContext.setMaxInitialResultHolderCapacity(_maxInitialResultHolderCapacity);
-
+ Integer initResultCap =
QueryOptionsUtils.getMaxInitResultCap(queryOptions);
Review Comment:
Let's use the same way to set the arguments for readability, e.g. always do
if check
--
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]
