This is an automated email from the ASF dual-hosted git repository.
gortiz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new e1328a3bf5 Improve query options validation and error handling.
(#14158)
e1328a3bf5 is described below
commit e1328a3bf51e63e8f96f00908fdde617a5837f43
Author: Bolek Ziobrowski <[email protected]>
AuthorDate: Mon Oct 7 15:20:53 2024 +0200
Improve query options validation and error handling. (#14158)
This PR adds the missing validation and improves error messages, e.g.
`numGroupsLimit must be a number between 0 and 2^31-1, got: 10000000000`
for the following options:
numGroupsLimit
maxInitialResultHolderCapacity
multiStageLeafLimit
groupTrimThreshold
maxStreamingPendingBlocks
maxRowsInJoin
maxRowsInWindow
numReplicaGroupsToQuery
maxExecutionThreads > 0
minSegmentGroupTrimSize > -1
minServerGroupTrimSize
minBrokerGroupTrimSize > 0, v1 only
timeoutMs
maxServerResponseSizeBytes
maxQueryResponseSizeBytes
---
.../broker/requesthandler/QueryValidationTest.java | 2 +-
.../common/utils/config/QueryOptionsUtils.java | 93 ++++++----
.../common/utils/config/QueryOptionsUtilsTest.java | 95 +++++++++++
.../pinot/queries/WithOptionQueriesTest.java | 190 +++++++++++++++++++++
.../query/runtime/queries/QueryRunnerTest.java | 49 +++++-
5 files changed, 384 insertions(+), 45 deletions(-)
diff --git
a/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/QueryValidationTest.java
b/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/QueryValidationTest.java
index f46833b078..a5a4c469a4 100644
---
a/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/QueryValidationTest.java
+++
b/pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/QueryValidationTest.java
@@ -117,7 +117,7 @@ public class QueryValidationTest {
public void testReplicaGroupToQueryInvalidQuery() {
PinotQuery pinotQuery =
CalciteSqlParser.compileToPinotQuery("SET
numReplicaGroupsToQuery='illegal'; SELECT COUNT(*) FROM MY_TABLE");
- Assert.assertThrows(IllegalStateException.class,
+ Assert.assertThrows(IllegalArgumentException.class,
() -> BaseSingleStageBrokerRequestHandler.validateRequest(pinotQuery,
10));
}
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java
index d4f8e7edbb..12ad735865 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java
@@ -18,7 +18,6 @@
*/
package org.apache.pinot.common.utils.config;
-import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
@@ -99,37 +98,19 @@ public class QueryOptionsUtils {
@Nullable
public static Long getTimeoutMs(Map<String, String> queryOptions) {
String timeoutMsString = queryOptions.get(QueryOptionKey.TIMEOUT_MS);
- if (timeoutMsString != null) {
- long timeoutMs = Long.parseLong(timeoutMsString);
- Preconditions.checkState(timeoutMs > 0, "Query timeout must be positive,
got: %s", timeoutMs);
- return timeoutMs;
- } else {
- return null;
- }
+ return checkedParseLong(QueryOptionKey.TIMEOUT_MS, timeoutMsString, 1);
}
@Nullable
public static Long getMaxServerResponseSizeBytes(Map<String, String>
queryOptions) {
String responseSize =
queryOptions.get(QueryOptionKey.MAX_SERVER_RESPONSE_SIZE_BYTES);
- if (responseSize != null) {
- long maxSize = Long.parseLong(responseSize);
- Preconditions.checkState(maxSize > 0, "maxServerResponseSize must be
positive. got %s", maxSize);
- return maxSize;
- }
-
- return null;
+ return checkedParseLong(QueryOptionKey.MAX_SERVER_RESPONSE_SIZE_BYTES,
responseSize, 1);
}
@Nullable
public static Long getMaxQueryResponseSizeBytes(Map<String, String>
queryOptions) {
String responseSize =
queryOptions.get(QueryOptionKey.MAX_QUERY_RESPONSE_SIZE_BYTES);
- if (responseSize != null) {
- long maxSize = Long.parseLong(responseSize);
- Preconditions.checkState(maxSize > 0, "maxQueryResponseSize must be
positive. got %s", maxSize);
- return maxSize;
- }
-
- return null;
+ return checkedParseLong(QueryOptionKey.MAX_QUERY_RESPONSE_SIZE_BYTES,
responseSize, 1);
}
public static boolean isAndScanReorderingEnabled(Map<String, String>
queryOptions) {
@@ -146,7 +127,7 @@ public class QueryOptionsUtils {
public static long getUpsertViewFreshnessMs(Map<String, String>
queryOptions) {
String freshnessMsString =
queryOptions.get(QueryOptionKey.UPSERT_VIEW_FRESHNESS_MS);
- return freshnessMsString != null ? Long.parseLong(freshnessMsString) : -1;
+ return freshnessMsString != null ? Long.parseLong(freshnessMsString) : -1;
//can blow up with NFE
}
public static boolean isScanStarTreeNodes(Map<String, String> queryOptions) {
@@ -198,7 +179,7 @@ public class QueryOptionsUtils {
@Nullable
public static Integer getNumReplicaGroupsToQuery(Map<String, String>
queryOptions) {
String numReplicaGroupsToQuery =
queryOptions.get(QueryOptionKey.NUM_REPLICA_GROUPS_TO_QUERY);
- return numReplicaGroupsToQuery != null ?
Integer.parseInt(numReplicaGroupsToQuery) : null;
+ return checkedParseInt(QueryOptionKey.NUM_REPLICA_GROUPS_TO_QUERY,
numReplicaGroupsToQuery);
}
public static boolean isExplainPlanVerbose(Map<String, String> queryOptions)
{
@@ -220,25 +201,25 @@ public class QueryOptionsUtils {
@Nullable
public static Integer getMaxExecutionThreads(Map<String, String>
queryOptions) {
String maxExecutionThreadsString =
queryOptions.get(QueryOptionKey.MAX_EXECUTION_THREADS);
- return maxExecutionThreadsString != null ?
Integer.parseInt(maxExecutionThreadsString) : null;
+ return checkedParseInt(QueryOptionKey.MAX_EXECUTION_THREADS,
maxExecutionThreadsString);
}
@Nullable
public static Integer getMinSegmentGroupTrimSize(Map<String, String>
queryOptions) {
String minSegmentGroupTrimSizeString =
queryOptions.get(QueryOptionKey.MIN_SEGMENT_GROUP_TRIM_SIZE);
- return minSegmentGroupTrimSizeString != null ?
Integer.parseInt(minSegmentGroupTrimSizeString) : null;
+ return checkedParseInt(QueryOptionKey.MIN_SEGMENT_GROUP_TRIM_SIZE,
minSegmentGroupTrimSizeString);
}
@Nullable
public static Integer getMinServerGroupTrimSize(Map<String, String>
queryOptions) {
String minServerGroupTrimSizeString =
queryOptions.get(QueryOptionKey.MIN_SERVER_GROUP_TRIM_SIZE);
- return minServerGroupTrimSizeString != null ?
Integer.parseInt(minServerGroupTrimSizeString) : null;
+ return checkedParseInt(QueryOptionKey.MIN_SERVER_GROUP_TRIM_SIZE,
minServerGroupTrimSizeString);
}
@Nullable
public static Integer getMinBrokerGroupTrimSize(Map<String, String>
queryOptions) {
String minBrokerGroupTrimSizeString =
queryOptions.get(QueryOptionKey.MIN_BROKER_GROUP_TRIM_SIZE);
- return minBrokerGroupTrimSizeString != null ?
Integer.parseInt(minBrokerGroupTrimSizeString) : null;
+ return checkedParseInt(QueryOptionKey.MIN_BROKER_GROUP_TRIM_SIZE,
minBrokerGroupTrimSizeString);
}
public static boolean isNullHandlingEnabled(Map<String, String>
queryOptions) {
@@ -261,25 +242,67 @@ public class QueryOptionsUtils {
@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;
+ return checkedParseInt(QueryOptionKey.MULTI_STAGE_LEAF_LIMIT,
maxLeafLimitStr);
}
@Nullable
public static Integer getNumGroupsLimit(Map<String, String> queryOptions) {
String maxNumGroupLimit =
queryOptions.get(QueryOptionKey.NUM_GROUPS_LIMIT);
- return maxNumGroupLimit != null ? Integer.parseInt(maxNumGroupLimit) :
null;
+ return checkedParseInt(QueryOptionKey.NUM_GROUPS_LIMIT, maxNumGroupLimit);
}
@Nullable
public static Integer getMaxInitialResultHolderCapacity(Map<String, String>
queryOptions) {
String maxInitResultCap =
queryOptions.get(QueryOptionKey.MAX_INITIAL_RESULT_HOLDER_CAPACITY);
- return maxInitResultCap != null ? Integer.parseInt(maxInitResultCap) :
null;
+ return checkedParseInt(QueryOptionKey.MAX_INITIAL_RESULT_HOLDER_CAPACITY,
maxInitResultCap);
}
@Nullable
public static Integer getGroupTrimThreshold(Map<String, String>
queryOptions) {
String groupByTrimThreshold =
queryOptions.get(QueryOptionKey.GROUP_TRIM_THRESHOLD);
- return groupByTrimThreshold != null ?
Integer.parseInt(groupByTrimThreshold) : null;
+ return checkedParseInt(QueryOptionKey.GROUP_TRIM_THRESHOLD,
groupByTrimThreshold);
+ }
+
+ private static Long checkedParseLong(String optionName, String optionValue,
int minValue) {
+ try {
+ if (optionValue != null) {
+ Long value = Long.parseLong(optionValue);
+ if (value < minValue) {
+ throw longParseException(optionName, optionValue, minValue);
+ }
+ return value;
+ } else {
+ return null;
+ }
+ } catch (NumberFormatException nfe) {
+ throw longParseException(optionName, optionValue, minValue);
+ }
+ }
+
+ private static IllegalArgumentException longParseException(String
optionName, String optionValue, int minValue) {
+ return new IllegalArgumentException(
+ String.format("%s must be a number between %d and 2^63-1, got: %s",
optionName, minValue, optionValue));
+ }
+
+ private static Integer checkedParseInt(String optionName, String
optionValue) {
+ try {
+ if (optionValue != null) {
+ int value = Integer.parseInt(optionValue);
+ if (value < 0) {
+ throw intParseException(optionName, optionValue);
+ }
+ return value;
+ } else {
+ return null;
+ }
+ } catch (NumberFormatException nfe) {
+ throw intParseException(optionName, optionValue);
+ }
+ }
+
+ private static IllegalArgumentException intParseException(String optionName,
String optionValue) {
+ return new IllegalArgumentException(
+ String.format("%s must be a number between 0 and 2^31-1, got: %s",
optionName, optionValue));
}
public static boolean shouldDropResults(Map<String, String> queryOptions) {
@@ -289,13 +312,13 @@ public class QueryOptionsUtils {
@Nullable
public static Integer getMaxStreamingPendingBlocks(Map<String, String>
queryOptions) {
String maxStreamingPendingBlocks =
queryOptions.get(QueryOptionKey.MAX_STREAMING_PENDING_BLOCKS);
- return maxStreamingPendingBlocks != null ?
Integer.parseInt(maxStreamingPendingBlocks) : null;
+ return checkedParseInt(QueryOptionKey.MAX_STREAMING_PENDING_BLOCKS,
maxStreamingPendingBlocks);
}
@Nullable
public static Integer getMaxRowsInJoin(Map<String, String> queryOptions) {
String maxRowsInJoin = queryOptions.get(QueryOptionKey.MAX_ROWS_IN_JOIN);
- return maxRowsInJoin != null ? Integer.parseInt(maxRowsInJoin) : null;
+ return checkedParseInt(QueryOptionKey.MAX_ROWS_IN_JOIN, maxRowsInJoin);
}
@Nullable
@@ -307,7 +330,7 @@ public class QueryOptionsUtils {
@Nullable
public static Integer getMaxRowsInWindow(Map<String, String> queryOptions) {
String maxRowsInWindow =
queryOptions.get(QueryOptionKey.MAX_ROWS_IN_WINDOW);
- return maxRowsInWindow != null ? Integer.parseInt(maxRowsInWindow) : null;
+ return checkedParseInt(QueryOptionKey.MAX_ROWS_IN_WINDOW, maxRowsInWindow);
}
@Nullable
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/utils/config/QueryOptionsUtilsTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/utils/config/QueryOptionsUtilsTest.java
index 4f0985569b..6f0f469c5b 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/utils/config/QueryOptionsUtilsTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/utils/config/QueryOptionsUtilsTest.java
@@ -20,6 +20,8 @@
package org.apache.pinot.common.utils.config;
import com.google.common.collect.ImmutableMap;
+import java.util.Arrays;
+import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import org.apache.pinot.spi.config.table.FieldConfig;
@@ -27,6 +29,8 @@ import org.apache.pinot.spi.utils.CommonConstants;
import org.testng.Assert;
import org.testng.annotations.Test;
+import static
org.apache.pinot.spi.utils.CommonConstants.Broker.Request.QueryOptionKey.*;
+
public class QueryOptionsUtilsTest {
@@ -65,4 +69,95 @@ public class QueryOptionsUtilsTest {
Map.of(CommonConstants.Broker.Request.QueryOptionKey.SKIP_INDEXES,
skipIndexesStr);
QueryOptionsUtils.getSkipIndexes(queryOptions);
}
+
+ @Test
+ public void testIntegerSettingParseSuccess() {
+ HashMap<String, String> map = new HashMap<>();
+
+ for (String setting : Arrays.asList(NUM_GROUPS_LIMIT,
MAX_INITIAL_RESULT_HOLDER_CAPACITY, MULTI_STAGE_LEAF_LIMIT,
+ GROUP_TRIM_THRESHOLD, MAX_STREAMING_PENDING_BLOCKS, MAX_ROWS_IN_JOIN,
MAX_STREAMING_PENDING_BLOCKS,
+ MAX_EXECUTION_THREADS, MIN_SEGMENT_GROUP_TRIM_SIZE,
MIN_SERVER_GROUP_TRIM_SIZE)) {
+ map.clear();
+ for (Integer val : new Integer[]{null, 1, 10, Integer.MAX_VALUE}) {
+ map.put(setting, val != null ? String.valueOf(val) : null);
+ Assert.assertEquals(getValue(map, setting), val);
+ }
+ }
+
+ for (String setting : Arrays.asList(TIMEOUT_MS,
MAX_SERVER_RESPONSE_SIZE_BYTES, MAX_QUERY_RESPONSE_SIZE_BYTES)) {
+ map.clear();
+ for (Long val : new Long[]{null, 1L, 10L, Long.MAX_VALUE}) {
+ map.put(setting, val != null ? String.valueOf(val) : null);
+ Assert.assertEquals(getValue(map, setting), val);
+ }
+ }
+ }
+
+ @Test
+ public void testIntegerSettingParseErrors() {
+ HashMap<String, String> map = new HashMap<>();
+
+ for (String setting : Arrays.asList(NUM_GROUPS_LIMIT,
MAX_INITIAL_RESULT_HOLDER_CAPACITY, MULTI_STAGE_LEAF_LIMIT,
+ GROUP_TRIM_THRESHOLD, MAX_STREAMING_PENDING_BLOCKS, MAX_ROWS_IN_JOIN,
MAX_STREAMING_PENDING_BLOCKS,
+ MAX_EXECUTION_THREADS, MIN_SEGMENT_GROUP_TRIM_SIZE,
MIN_SERVER_GROUP_TRIM_SIZE)) {
+ for (String val : new String[]{"-10000000000", "-2147483648", "-1",
"2147483648", "10000000000"}) {
+ map.clear();
+ map.put(setting, val);
+ try {
+ getValue(map, setting);
+ Assert.fail();
+ } catch (IllegalArgumentException ise) {
+ Assert.assertEquals(ise.getMessage(), setting + " must be a number
between 0 and 2^31-1, got: " + val);
+ }
+ }
+ }
+
+ for (String setting : Arrays.asList(TIMEOUT_MS,
MAX_SERVER_RESPONSE_SIZE_BYTES, MAX_QUERY_RESPONSE_SIZE_BYTES)) {
+ for (String val : new String[]{
+ "-100000000000000000000", "-9223372036854775809", "-1", "0",
"9223372036854775808", "100000000000000000000"
+ }) {
+ map.clear();
+ map.put(setting, val);
+ try {
+ getValue(map, setting);
+ Assert.fail();
+ } catch (IllegalArgumentException ise) {
+ Assert.assertEquals(ise.getMessage(), setting + " must be a number
between 1 and 2^63-1, got: " + val);
+ }
+ }
+ }
+ }
+
+ private static Object getValue(Map<String, String> map, String key) {
+ switch (key) {
+ //ints
+ case NUM_GROUPS_LIMIT:
+ return QueryOptionsUtils.getNumGroupsLimit(map);
+ case MAX_INITIAL_RESULT_HOLDER_CAPACITY:
+ return QueryOptionsUtils.getMaxInitialResultHolderCapacity(map);
+ case MULTI_STAGE_LEAF_LIMIT:
+ return QueryOptionsUtils.getMultiStageLeafLimit(map);
+ case GROUP_TRIM_THRESHOLD:
+ return QueryOptionsUtils.getGroupTrimThreshold(map);
+ case MAX_STREAMING_PENDING_BLOCKS:
+ return QueryOptionsUtils.getMaxStreamingPendingBlocks(map);
+ case MAX_ROWS_IN_JOIN:
+ return QueryOptionsUtils.getMaxRowsInJoin(map);
+ case MAX_EXECUTION_THREADS:
+ return QueryOptionsUtils.getMaxExecutionThreads(map);
+ case MIN_SEGMENT_GROUP_TRIM_SIZE:
+ return QueryOptionsUtils.getMinSegmentGroupTrimSize(map);
+ case MIN_SERVER_GROUP_TRIM_SIZE:
+ return QueryOptionsUtils.getMinServerGroupTrimSize(map);
+ //longs
+ case TIMEOUT_MS:
+ return QueryOptionsUtils.getTimeoutMs(map);
+ case MAX_SERVER_RESPONSE_SIZE_BYTES:
+ return QueryOptionsUtils.getMaxServerResponseSizeBytes(map);
+ case MAX_QUERY_RESPONSE_SIZE_BYTES:
+ return QueryOptionsUtils.getMaxQueryResponseSizeBytes(map);
+ default:
+ throw new IllegalArgumentException("Unexpected key!");
+ }
+ }
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/queries/WithOptionQueriesTest.java
b/pinot-core/src/test/java/org/apache/pinot/queries/WithOptionQueriesTest.java
new file mode 100644
index 0000000000..654a0add08
--- /dev/null
+++
b/pinot-core/src/test/java/org/apache/pinot/queries/WithOptionQueriesTest.java
@@ -0,0 +1,190 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pinot.queries;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import org.apache.commons.io.FileUtils;
+import
org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoader;
+import
org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl;
+import org.apache.pinot.segment.local.segment.readers.GenericRowRecordReader;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.segment.spi.IndexSegment;
+import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import
org.apache.pinot.spi.utils.CommonConstants.Broker.Request.QueryOptionKey;
+import org.apache.pinot.spi.utils.ReadMode;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import static
org.apache.pinot.spi.utils.CommonConstants.Broker.Request.QueryOptionKey.MAX_QUERY_RESPONSE_SIZE_BYTES;
+import static
org.apache.pinot.spi.utils.CommonConstants.Broker.Request.QueryOptionKey.MAX_SERVER_RESPONSE_SIZE_BYTES;
+import static
org.apache.pinot.spi.utils.CommonConstants.Broker.Request.QueryOptionKey.TIMEOUT_MS;
+
+
+public class WithOptionQueriesTest extends BaseQueriesTest {
+
+ private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(),
"WithOptionQueriesTest");
+ private static final String RAW_TABLE_NAME = "testTable";
+ private static final String SEGMENT_NAME = "testSegment";
+
+ private static final int NUM_RECORDS = 10;
+ private static final String X_COL = "x";
+ private static final String Y_COL = "y";
+
+ private static final Schema SCHEMA = new
Schema.SchemaBuilder().addSingleValueDimension(X_COL, FieldSpec.DataType.INT)
+ .addSingleValueDimension(Y_COL, FieldSpec.DataType.DOUBLE).build();
+
+ private static final TableConfig TABLE_CONFIG =
+ new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).build();
+
+ private IndexSegment _indexSegment;
+ private List<IndexSegment> _indexSegments;
+
+ @Override
+ protected String getFilter() {
+ return "";
+ }
+
+ @Override
+ protected IndexSegment getIndexSegment() {
+ return _indexSegment;
+ }
+
+ @Override
+ protected List<IndexSegment> getIndexSegments() {
+ return _indexSegments;
+ }
+
+ private final List<Object[]> _allRecords = new ArrayList<>();
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ FileUtils.deleteQuietly(INDEX_DIR);
+
+ List<GenericRow> records = new ArrayList<>(NUM_RECORDS);
+ for (int i = 0; i < NUM_RECORDS; i++) {
+ GenericRow record = new GenericRow();
+ record.putValue(X_COL, i);
+ record.putValue(Y_COL, 0.25);
+ records.add(record);
+ _allRecords.add(new Object[]{i, 0.25});
+ }
+
+ SegmentGeneratorConfig segmentGeneratorConfig = new
SegmentGeneratorConfig(TABLE_CONFIG, SCHEMA);
+ segmentGeneratorConfig.setTableName(RAW_TABLE_NAME);
+ segmentGeneratorConfig.setSegmentName(SEGMENT_NAME);
+ segmentGeneratorConfig.setOutDir(INDEX_DIR.getPath());
+
+ SegmentIndexCreationDriverImpl driver = new
SegmentIndexCreationDriverImpl();
+ driver.init(segmentGeneratorConfig, new GenericRowRecordReader(records));
+ driver.build();
+
+ ImmutableSegment immutableSegment = ImmutableSegmentLoader.load(new
File(INDEX_DIR, SEGMENT_NAME), ReadMode.mmap);
+ _indexSegment = immutableSegment;
+ _indexSegments = Arrays.asList(immutableSegment, immutableSegment);
+ }
+
+ @Test
+ public void testOptionParsingFailure() {
+ HashMap<String, String> options = new HashMap<>();
+
+ // int values
+ for (String setting : Arrays.asList(QueryOptionKey.NUM_GROUPS_LIMIT,
+ QueryOptionKey.MAX_INITIAL_RESULT_HOLDER_CAPACITY,
QueryOptionKey.GROUP_TRIM_THRESHOLD,
+ QueryOptionKey.MAX_EXECUTION_THREADS,
QueryOptionKey.MIN_SEGMENT_GROUP_TRIM_SIZE,
+ QueryOptionKey.MIN_SERVER_GROUP_TRIM_SIZE,
QueryOptionKey.MIN_BROKER_GROUP_TRIM_SIZE)) {
+
+ options.clear();
+ for (String value : new String[]{"-10000000000", "-2147483648", "-1",
"2147483648", "10000000000"}) {
+ options.put(setting, value);
+
+ IllegalArgumentException exception =
Assert.expectThrows(IllegalArgumentException.class, () -> {
+ getBrokerResponse("SELECT x, count(*) FROM " + RAW_TABLE_NAME + "
GROUP BY x", options);
+ });
+ Assert.assertEquals(setting + " must be a number between 0 and 2^31-1,
got: " + value, exception.getMessage());
+ }
+ }
+ }
+
+ @Test
+ public void testOptionParsingSuccess() {
+ HashMap<String, String> options = new HashMap<>();
+ List<Object> groupRows = new ArrayList();
+ groupRows.add(new Object[]{0d, 40L}); //four times 10 records because
segment gets multiplied under the hood
+
+ // int values
+ for (String setting : Arrays.asList(QueryOptionKey.NUM_GROUPS_LIMIT,
+ QueryOptionKey.MAX_INITIAL_RESULT_HOLDER_CAPACITY,
QueryOptionKey.MULTI_STAGE_LEAF_LIMIT,
+ QueryOptionKey.GROUP_TRIM_THRESHOLD,
QueryOptionKey.MAX_STREAMING_PENDING_BLOCKS,
+ QueryOptionKey.MAX_ROWS_IN_JOIN, QueryOptionKey.MAX_EXECUTION_THREADS,
+ QueryOptionKey.MIN_SEGMENT_GROUP_TRIM_SIZE,
QueryOptionKey.MIN_SERVER_GROUP_TRIM_SIZE,
+ QueryOptionKey.MIN_BROKER_GROUP_TRIM_SIZE,
QueryOptionKey.NUM_REPLICA_GROUPS_TO_QUERY)) {
+
+ options.clear();
+ for (String value : new String[]{"0", "1", "10000", "2147483647"}) {
+
+ options.put(setting, value);
+ List<Object[]> rows =
+ getBrokerResponse("SELECT mod(x,1), count(*) FROM " +
RAW_TABLE_NAME + " GROUP BY mod(x,1)",
+ options).getResultTable().getRows();
+ if (QueryOptionKey.NUM_GROUPS_LIMIT == setting && "0".equals(value)) {
+ Assert.assertEquals(0, rows.size());
+ } else {
+ assertEquals(rows, groupRows);
+ }
+ }
+ }
+
+ //long values
+ for (String setting : Arrays.asList(TIMEOUT_MS,
MAX_SERVER_RESPONSE_SIZE_BYTES, MAX_QUERY_RESPONSE_SIZE_BYTES)) {
+ options.clear();
+ for (String value : new String[]{"1", "10000", "9223372036854775807"}) {
+ options.put(setting, value);
+ List<Object[]> rows = getBrokerResponse("SELECT * FROM " +
RAW_TABLE_NAME, options).getResultTable().getRows();
+ assertEquals(rows, _allRecords);
+ }
+ }
+ }
+
+ private void assertEquals(List actual, List expected) {
+ if (actual == expected) {
+ return;
+ }
+
+ if (actual == null || expected == null || actual.size() !=
expected.size()) {
+ Assert.fail("Expected \n" + expected + "\n but got \n" + actual);
+ }
+
+ for (int i = 0; i < actual.size(); i++) {
+ Assert.assertEquals(actual.get(i), expected.get(i));
+ }
+ }
+}
diff --git
a/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTest.java
b/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTest.java
index 76802c7b77..3e946d5eab 100644
---
a/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTest.java
+++
b/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTest.java
@@ -22,6 +22,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
@@ -38,6 +39,7 @@ import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.data.readers.GenericRow;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.utils.CommonConstants;
+import
org.apache.pinot.spi.utils.CommonConstants.Broker.Request.QueryOptionKey;
import org.apache.pinot.spi.utils.JsonUtils;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.testng.Assert;
@@ -186,24 +188,25 @@ public class QueryRunnerTest extends QueryRunnerTestBase {
* Test compares against its desired exceptions.
*/
@Test(dataProvider = "testDataWithSqlExecutionExceptions")
- public void testSqlWithExceptionMsgChecker(String sql, String exceptionMsg) {
+ public void testSqlWithExceptionMsgChecker(String sql, String expectedError)
{
try {
// query pinot
ResultTable resultTable = queryRunner(sql, false).getResultTable();
- Assert.fail("Expected error with message '" + exceptionMsg + "'. But
instead rows were returned: "
+ Assert.fail("Expected error with message '" + expectedError + "'. But
instead rows were returned: "
+ JsonUtils.objectToPrettyString(resultTable));
} catch (Exception e) {
// NOTE: The actual message is (usually) something like:
// Received error query execution result block:
{200=QueryExecutionError:
// Query execution error on: Server_localhost_12345
// java.lang.IllegalArgumentException: Illegal Json Path: $['path']
does not match document
+ // In some cases there is no prefix.
String exceptionMessage = e.getMessage();
Assert.assertTrue(
exceptionMessage.startsWith("Received error query execution result
block: ") || exceptionMessage.startsWith(
- "Error occurred during stage submission"),
+ "Error occurred during stage submission") ||
exceptionMessage.equals(expectedError),
"Exception message didn't start with proper heading: " +
exceptionMessage);
- Assert.assertTrue(exceptionMessage.contains(exceptionMsg),
- "Exception should contain: " + exceptionMsg + ", but found: " +
exceptionMessage);
+ Assert.assertTrue(exceptionMessage.contains(expectedError),
+ "Exception should contain: " + expectedError + ", but found: " +
exceptionMessage);
}
}
@@ -289,9 +292,11 @@ public class QueryRunnerTest extends QueryRunnerTestBase {
}
@DataProvider(name = "testDataWithSqlExecutionExceptions")
- protected Object[][] provideTestSqlWithExecutionException() {
+ protected Iterator<Object[]> provideTestSqlWithExecutionException() {
//@formatter:off
- return new Object[][]{
+ List<Object[]> testCases = new ArrayList();
+ testCases.addAll(
+ Arrays.asList(
// Missing index
new Object[]{"SELECT col1 FROM a WHERE textMatch(col1, 'f') LIMIT 10",
"without text index"},
@@ -327,8 +332,34 @@ public class QueryRunnerTest extends QueryRunnerTestBase {
new Object[]{
"SELECT CAST(json_extract_scalar(a.col1, b.col2, 'INT') AS INT)
FROM a JOIN b ON a.col1 = b.col1",
"Unsupported function: JSONEXTRACTSCALAR"
- }
- };
+ }));
//@formatter:on
+
+ // int values
+ for (String setting : Arrays.asList(QueryOptionKey.NUM_GROUPS_LIMIT,
+ QueryOptionKey.MAX_INITIAL_RESULT_HOLDER_CAPACITY,
QueryOptionKey.MULTI_STAGE_LEAF_LIMIT,
+ QueryOptionKey.GROUP_TRIM_THRESHOLD,
QueryOptionKey.MAX_STREAMING_PENDING_BLOCKS,
+ QueryOptionKey.MAX_ROWS_IN_JOIN, QueryOptionKey.MAX_EXECUTION_THREADS,
+ QueryOptionKey.MIN_SEGMENT_GROUP_TRIM_SIZE,
QueryOptionKey.MIN_SERVER_GROUP_TRIM_SIZE)) {
+
+ for (String val : new String[]{"-10000000000", "-2147483648", "-1",
"2147483648", "10000000000"}) {
+ testCases.add(new Object[]{
+ "set " + setting + " = " + val + "; SELECT col1, count(*) FROM a
GROUP BY col1",
+ setting + " must be a number between 0 and 2^31-1, got: " + val
+ });
+ }
+ }
+
+ // int values; triggered for query with window clause
+ for (String setting : Arrays.asList(QueryOptionKey.MAX_ROWS_IN_WINDOW)) {
+ for (String val : new String[]{"-10000000000", "-2147483648", "-1",
"2147483648", "10000000000"}) {
+ testCases.add(new Object[]{
+ "set " + setting + " = " + val + "; SELECT ROW_NUMBER() over
(PARTITION BY col1) FROM a",
+ setting + " must be a number between 0 and 2^31-1, got: " + val
+ });
+ }
+ }
+
+ return testCases.iterator();
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
