This is an automated email from the ASF dual-hosted git repository.
Jackie-Jiang 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 3782011c240 Improve MIN/MAX error message for non-numeric columns
(#19653)
3782011c240 is described below
commit 3782011c240fa91b3df64151cc4186dfaa377fe0
Author: akashchamp <[email protected]>
AuthorDate: Tue Oct 13 02:13:07 2026 +0530
Improve MIN/MAX error message for non-numeric columns (#19653)
---
.../function/AggregationFunctionUtils.java | 16 +++-
.../function/MaxAggregationFunction.java | 53 +++++++----
.../function/MinAggregationFunction.java | 53 +++++++----
.../function/AggregationFunctionUtilsTest.java | 33 +++++++
.../function/MaxAggregationFunctionTest.java | 105 +++++++++++++++++++++
.../function/MinAggregationFunctionTest.java | 105 +++++++++++++++++++++
6 files changed, 332 insertions(+), 33 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java
b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java
index fb59e4dd00e..07c433112a6 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java
@@ -74,6 +74,7 @@ import
org.apache.pinot.segment.spi.datasource.DataSourceMetadata;
import org.apache.pinot.segment.spi.index.reader.Dictionary;
import
org.apache.pinot.segment.spi.index.startree.AggregationFunctionColumnPair;
import org.apache.pinot.spi.data.FieldSpec.DataType;
+import org.apache.pinot.spi.exception.BadQueryRequestException;
import org.apache.pinot.spi.query.QueryThreadContext;
import org.apache.pinot.spi.utils.ByteArray;
@@ -738,13 +739,26 @@ public class AggregationFunctionUtils {
return ((Number)
dataSource.getDataSourceMetadata().getMaxValue()).longValue();
}
+ /// Converts the given dictionary/metadata min/max value to a `double`, for
the numeric MIN/MAX/MINMAXRANGE
+ /// dictionary/metadata based resolution (see [#getMinValueNumeric] and
[#getMaxValueNumeric]).
+ ///
+ /// @throws BadQueryRequestException if `value` is not a [Number] and cannot
be parsed as one, which happens when
+ /// MIN/MAX is applied to a non-numeric (e.g. STRING) column; the message
points the caller to MINSTRING/MAXSTRING
+ /// and the `autoRewriteAggregationType` query option instead of surfacing a
raw [NumberFormatException].
private static Double toDouble(Comparable<?> value) {
if (value instanceof Double) {
return (Double) value;
} else if (value instanceof Number) {
return ((Number) value).doubleValue();
- } else {
+ }
+ try {
return Double.parseDouble(value.toString());
+ } catch (NumberFormatException e) {
+ throw new BadQueryRequestException(
+ "Cannot compute MIN/MAX on non-numeric value: \"" + value + "\".
MIN/MAX only support numeric columns; "
+ + "use MINSTRING/MAXSTRING for string columns, or set the query
option "
+ + "'autoRewriteAggregationType=true' to automatically rewrite
MIN/MAX to MINSTRING/MAXSTRING on "
+ + "string columns.", e);
}
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MaxAggregationFunction.java
b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MaxAggregationFunction.java
index a559c301d8b..e50341a96fb 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MaxAggregationFunction.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MaxAggregationFunction.java
@@ -37,6 +37,9 @@ import
org.apache.pinot.spi.exception.BadQueryRequestException;
public class MaxAggregationFunction extends
BaseSingleInputAggregationFunction<Double, Double> {
protected static final double DEFAULT_INITIAL_VALUE =
Double.NEGATIVE_INFINITY;
+ private static final String NON_NUMERIC_TYPE_MESSAGE_SUFFIX = ". MAX only
supports numeric columns; use MAXSTRING "
+ + "for string columns, or set the query option
'autoRewriteAggregationType=true' to automatically rewrite "
+ + "MAX to MAXSTRING on string columns.";
public MaxAggregationFunction(List<ExpressionContext> arguments, boolean
nullHandlingEnabled) {
this(verifySingleArgument(arguments, "MAX"), nullHandlingEnabled);
@@ -71,14 +74,25 @@ public class MaxAggregationFunction extends
BaseSingleInputAggregationFunction<D
public void aggregate(int length, AggregationResultHolder
aggregationResultHolder,
Map<ExpressionContext, BlockValSet> blockValSetMap) {
BlockValSet blockValSet = blockValSetMap.get(_expression);
-
- if (blockValSet.isSingleValue()) {
- aggregateSV(blockValSet, length, aggregationResultHolder);
- } else {
- aggregateMV(blockValSet, length, aggregationResultHolder);
+ try {
+ if (blockValSet.isSingleValue()) {
+ aggregateSV(blockValSet, length, aggregationResultHolder);
+ } else {
+ aggregateMV(blockValSet, length, aggregationResultHolder);
+ }
+ } catch (NumberFormatException e) {
+ throw nonNumericValueException(blockValSet, e);
}
}
+ /// Wraps a failure to convert a non-numeric (e.g. STRING) value to a
`double` into a descriptive
+ /// [BadQueryRequestException]. Numeric strings still convert successfully
and are not affected.
+ private static BadQueryRequestException nonNumericValueException(BlockValSet
blockValSet, NumberFormatException e) {
+ return new BadQueryRequestException(
+ "Cannot compute max for non-numeric value in column of type: " +
blockValSet.getValueType()
+ + NON_NUMERIC_TYPE_MESSAGE_SUFFIX + " Cause: " + e.getMessage(),
e);
+ }
+
protected void aggregateSV(BlockValSet blockValSet, int length,
AggregationResultHolder aggregationResultHolder) {
switch (blockValSet.getValueType().getStoredType()) {
case INT: {
@@ -153,7 +167,8 @@ public class MaxAggregationFunction extends
BaseSingleInputAggregationFunction<D
break;
}
default:
- throw new BadQueryRequestException("Cannot compute max for non-numeric
type: " + blockValSet.getValueType());
+ throw new BadQueryRequestException(
+ "Cannot compute max for non-numeric type: " +
blockValSet.getValueType() + NON_NUMERIC_TYPE_MESSAGE_SUFFIX);
}
}
@@ -191,11 +206,14 @@ public class MaxAggregationFunction extends
BaseSingleInputAggregationFunction<D
public void aggregateGroupBySV(int length, int[] groupKeyArray,
GroupByResultHolder groupByResultHolder,
Map<ExpressionContext, BlockValSet> blockValSetMap) {
BlockValSet blockValSet = blockValSetMap.get(_expression);
-
- if (blockValSet.isSingleValue()) {
- aggregateSVGroupBySV(blockValSet, length, groupKeyArray,
groupByResultHolder);
- } else {
- aggregateMVGroupBySV(blockValSet, length, groupKeyArray,
groupByResultHolder);
+ try {
+ if (blockValSet.isSingleValue()) {
+ aggregateSVGroupBySV(blockValSet, length, groupKeyArray,
groupByResultHolder);
+ } else {
+ aggregateMVGroupBySV(blockValSet, length, groupKeyArray,
groupByResultHolder);
+ }
+ } catch (NumberFormatException e) {
+ throw nonNumericValueException(blockValSet, e);
}
}
@@ -260,11 +278,14 @@ public class MaxAggregationFunction extends
BaseSingleInputAggregationFunction<D
public void aggregateGroupByMV(int length, int[][] groupKeysArray,
GroupByResultHolder groupByResultHolder,
Map<ExpressionContext, BlockValSet> blockValSetMap) {
BlockValSet blockValSet = blockValSetMap.get(_expression);
-
- if (blockValSet.isSingleValue()) {
- aggregateSVGroupByMV(blockValSet, length, groupKeysArray,
groupByResultHolder);
- } else {
- aggregateMVGroupByMV(blockValSet, length, groupKeysArray,
groupByResultHolder);
+ try {
+ if (blockValSet.isSingleValue()) {
+ aggregateSVGroupByMV(blockValSet, length, groupKeysArray,
groupByResultHolder);
+ } else {
+ aggregateMVGroupByMV(blockValSet, length, groupKeysArray,
groupByResultHolder);
+ }
+ } catch (NumberFormatException e) {
+ throw nonNumericValueException(blockValSet, e);
}
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MinAggregationFunction.java
b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MinAggregationFunction.java
index c002bf0d290..80544112d0d 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MinAggregationFunction.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MinAggregationFunction.java
@@ -37,6 +37,9 @@ import
org.apache.pinot.spi.exception.BadQueryRequestException;
public class MinAggregationFunction extends
BaseSingleInputAggregationFunction<Double, Double> {
protected static final double DEFAULT_VALUE = Double.POSITIVE_INFINITY;
+ private static final String NON_NUMERIC_TYPE_MESSAGE_SUFFIX = ". MIN only
supports numeric columns; use MINSTRING "
+ + "for string columns, or set the query option
'autoRewriteAggregationType=true' to automatically rewrite "
+ + "MIN to MINSTRING on string columns.";
public MinAggregationFunction(List<ExpressionContext> arguments, boolean
nullHandlingEnabled) {
this(verifySingleArgument(arguments, "MIN"), nullHandlingEnabled);
@@ -71,14 +74,25 @@ public class MinAggregationFunction extends
BaseSingleInputAggregationFunction<D
public void aggregate(int length, AggregationResultHolder
aggregationResultHolder,
Map<ExpressionContext, BlockValSet> blockValSetMap) {
BlockValSet blockValSet = blockValSetMap.get(_expression);
-
- if (blockValSet.isSingleValue()) {
- aggregateSV(blockValSet, length, aggregationResultHolder);
- } else {
- aggregateMV(blockValSet, length, aggregationResultHolder);
+ try {
+ if (blockValSet.isSingleValue()) {
+ aggregateSV(blockValSet, length, aggregationResultHolder);
+ } else {
+ aggregateMV(blockValSet, length, aggregationResultHolder);
+ }
+ } catch (NumberFormatException e) {
+ throw nonNumericValueException(blockValSet, e);
}
}
+ /// Wraps a failure to convert a non-numeric (e.g. STRING) value to a
`double` into a descriptive
+ /// [BadQueryRequestException]. Numeric strings still convert successfully
and are not affected.
+ private static BadQueryRequestException nonNumericValueException(BlockValSet
blockValSet, NumberFormatException e) {
+ return new BadQueryRequestException(
+ "Cannot compute min for non-numeric value in column of type: " +
blockValSet.getValueType()
+ + NON_NUMERIC_TYPE_MESSAGE_SUFFIX + " Cause: " + e.getMessage(),
e);
+ }
+
protected void aggregateSV(BlockValSet blockValSet, int length,
AggregationResultHolder aggregationResultHolder) {
switch (blockValSet.getValueType().getStoredType()) {
case INT: {
@@ -153,7 +167,8 @@ public class MinAggregationFunction extends
BaseSingleInputAggregationFunction<D
break;
}
default:
- throw new BadQueryRequestException("Cannot compute min for non-numeric
type: " + blockValSet.getValueType());
+ throw new BadQueryRequestException(
+ "Cannot compute min for non-numeric type: " +
blockValSet.getValueType() + NON_NUMERIC_TYPE_MESSAGE_SUFFIX);
}
}
@@ -191,11 +206,14 @@ public class MinAggregationFunction extends
BaseSingleInputAggregationFunction<D
public void aggregateGroupBySV(int length, int[] groupKeyArray,
GroupByResultHolder groupByResultHolder,
Map<ExpressionContext, BlockValSet> blockValSetMap) {
BlockValSet blockValSet = blockValSetMap.get(_expression);
-
- if (blockValSet.isSingleValue()) {
- aggregateSVGroupBySV(blockValSet, length, groupKeyArray,
groupByResultHolder);
- } else {
- aggregateMVGroupBySV(blockValSet, length, groupKeyArray,
groupByResultHolder);
+ try {
+ if (blockValSet.isSingleValue()) {
+ aggregateSVGroupBySV(blockValSet, length, groupKeyArray,
groupByResultHolder);
+ } else {
+ aggregateMVGroupBySV(blockValSet, length, groupKeyArray,
groupByResultHolder);
+ }
+ } catch (NumberFormatException e) {
+ throw nonNumericValueException(blockValSet, e);
}
}
@@ -260,11 +278,14 @@ public class MinAggregationFunction extends
BaseSingleInputAggregationFunction<D
public void aggregateGroupByMV(int length, int[][] groupKeysArray,
GroupByResultHolder groupByResultHolder,
Map<ExpressionContext, BlockValSet> blockValSetMap) {
BlockValSet blockValSet = blockValSetMap.get(_expression);
-
- if (blockValSet.isSingleValue()) {
- aggregateSVGroupByMV(blockValSet, length, groupKeysArray,
groupByResultHolder);
- } else {
- aggregateMVGroupByMV(blockValSet, length, groupKeysArray,
groupByResultHolder);
+ try {
+ if (blockValSet.isSingleValue()) {
+ aggregateSVGroupByMV(blockValSet, length, groupKeysArray,
groupByResultHolder);
+ } else {
+ aggregateMVGroupByMV(blockValSet, length, groupKeysArray,
groupByResultHolder);
+ }
+ } catch (NumberFormatException e) {
+ throw nonNumericValueException(blockValSet, e);
}
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtilsTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtilsTest.java
index 29041933af2..53cd082fc0c 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtilsTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtilsTest.java
@@ -21,12 +21,15 @@ package org.apache.pinot.core.query.aggregation.function;
import org.apache.pinot.segment.spi.AggregationFunctionType;
import org.apache.pinot.segment.spi.datasource.DataSource;
import org.apache.pinot.segment.spi.index.reader.Dictionary;
+import org.apache.pinot.spi.exception.BadQueryRequestException;
import org.testng.annotations.Test;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertThrows;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
/// Unit test for {@link AggregationFunctionUtils#getAggregationResult}, the
metadata/dictionary based aggregation
@@ -65,6 +68,36 @@ public class AggregationFunctionUtilsTest {
assertEquals(maxResult, 10.0);
}
+ @Test
+ public void testMinAndMaxOnNonNumericDictionaryThrows() {
+ // Reproduces https://github.com/apache/pinot/issues/19603: MIN/MAX
resolved from a dictionary-encoded
+ // non-numeric (e.g. STRING) column must fail with a clear, actionable
message instead of letting an opaque
+ // NumberFormatException escape from Double.parseDouble.
+ Dictionary dictionary = mock(Dictionary.class);
+ when(dictionary.getMinVal()).thenReturn("2013-07-01");
+ when(dictionary.getMaxVal()).thenReturn("2013-07-01");
+ DataSource dataSource = mock(DataSource.class);
+ when(dataSource.getDictionary()).thenReturn(dictionary);
+
+ try {
+
AggregationFunctionUtils.getAggregationResult(mockFunction(AggregationFunctionType.MIN),
dataSource, 100,
+ "TEST");
+ fail("Should throw BadQueryRequestException");
+ } catch (BadQueryRequestException e) {
+ assertTrue(e.getMessage().contains("MINSTRING"));
+ assertTrue(e.getMessage().contains("autoRewriteAggregationType"));
+ }
+
+ try {
+
AggregationFunctionUtils.getAggregationResult(mockFunction(AggregationFunctionType.MAX),
dataSource, 100,
+ "TEST");
+ fail("Should throw BadQueryRequestException");
+ } catch (BadQueryRequestException e) {
+ assertTrue(e.getMessage().contains("MAXSTRING"));
+ assertTrue(e.getMessage().contains("autoRewriteAggregationType"));
+ }
+ }
+
@Test
public void testUnsupportedFunctionThrows() {
// MODE cannot be resolved from dictionary/metadata; the resolver must
reject it rather than return a wrong result.
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/MaxAggregationFunctionTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/MaxAggregationFunctionTest.java
index d74ffd9a70b..480c379709f 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/MaxAggregationFunctionTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/MaxAggregationFunctionTest.java
@@ -18,17 +18,122 @@
*/
package org.apache.pinot.core.query.aggregation.function;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.request.context.RequestContextUtils;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
import org.apache.pinot.queries.FluentQueryTest;
import org.apache.pinot.spi.data.FieldSpec;
import org.apache.pinot.spi.data.FieldSpec.DataType;
import org.apache.pinot.spi.data.FieldSpec.FieldType;
import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.exception.BadQueryRequestException;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
+
public class MaxAggregationFunctionTest extends
AbstractAggregationFunctionTest {
+ /// MAX only supports numeric columns (see
https://github.com/apache/pinot/issues/19603); applying it to a
+ /// non-numeric (e.g. STRING) column must fail with a clear, actionable
message instead of an opaque
+ /// `NumberFormatException`.
+ @Test
+ public void testNonNumericColumnExceptionInAggregateMethod() {
+ ExpressionContext expression = RequestContextUtils.getExpression("column");
+ MaxAggregationFunction function = new
MaxAggregationFunction(List.of(expression), false);
+
+ AggregationResultHolder resultHolder =
function.createAggregationResultHolder();
+ Map<ExpressionContext, BlockValSet> blockValSetMap = new HashMap<>();
+ BlockValSet mockBlockValSet = mock(BlockValSet.class);
+ when(mockBlockValSet.getValueType()).thenReturn(DataType.STRING);
+ when(mockBlockValSet.isSingleValue()).thenReturn(true);
+ blockValSetMap.put(expression, mockBlockValSet);
+
+ try {
+ function.aggregate(10, resultHolder, blockValSetMap);
+ fail("Should throw BadQueryRequestException");
+ } catch (BadQueryRequestException e) {
+ assertTrue(e.getMessage().contains("Cannot compute max for non-numeric
type"));
+ assertTrue(e.getMessage().contains("MAXSTRING"));
+ assertTrue(e.getMessage().contains("autoRewriteAggregationType"));
+ }
+ }
+
+ @Test
+ public void testNonNumericColumnExceptionInAggregateGroupBySVMethod() {
+ ExpressionContext expression = RequestContextUtils.getExpression("column");
+ MaxAggregationFunction function = new
MaxAggregationFunction(List.of(expression), false);
+
+ GroupByResultHolder groupByResultHolder =
function.createGroupByResultHolder(10, 20);
+ Map<ExpressionContext, BlockValSet> blockValSetMap = new HashMap<>();
+ BlockValSet mockBlockValSet = mock(BlockValSet.class);
+ when(mockBlockValSet.getValueType()).thenReturn(DataType.STRING);
+ when(mockBlockValSet.isSingleValue()).thenReturn(true);
+ when(mockBlockValSet.getDoubleValuesSV()).thenThrow(new
NumberFormatException("For input string: \"abc\""));
+ blockValSetMap.put(expression, mockBlockValSet);
+
+ try {
+ function.aggregateGroupBySV(10, new int[10], groupByResultHolder,
blockValSetMap);
+ fail("Should throw BadQueryRequestException");
+ } catch (BadQueryRequestException e) {
+ assertTrue(e.getMessage().contains("Cannot compute max for non-numeric
value"));
+ assertTrue(e.getMessage().contains("MAXSTRING"));
+ assertTrue(e.getMessage().contains("autoRewriteAggregationType"));
+ }
+ }
+
+ @Test
+ public void testNonNumericColumnExceptionInAggregateGroupByMVMethod() {
+ ExpressionContext expression = RequestContextUtils.getExpression("column");
+ MaxAggregationFunction function = new
MaxAggregationFunction(List.of(expression), false);
+
+ GroupByResultHolder groupByResultHolder =
function.createGroupByResultHolder(10, 20);
+ Map<ExpressionContext, BlockValSet> blockValSetMap = new HashMap<>();
+ BlockValSet mockBlockValSet = mock(BlockValSet.class);
+ when(mockBlockValSet.getValueType()).thenReturn(DataType.STRING);
+ when(mockBlockValSet.isSingleValue()).thenReturn(true);
+ when(mockBlockValSet.getDoubleValuesSV()).thenThrow(new
NumberFormatException("For input string: \"abc\""));
+ blockValSetMap.put(expression, mockBlockValSet);
+
+ try {
+ function.aggregateGroupByMV(10, new int[10][], groupByResultHolder,
blockValSetMap);
+ fail("Should throw BadQueryRequestException");
+ } catch (BadQueryRequestException e) {
+ assertTrue(e.getMessage().contains("Cannot compute max for non-numeric
value"));
+ assertTrue(e.getMessage().contains("MAXSTRING"));
+ assertTrue(e.getMessage().contains("autoRewriteAggregationType"));
+ }
+ }
+
+ /// A STRING column holding numeric values keeps working on the group-by
path, since `getDoubleValuesSV()` parses
+ /// them; only values that cannot be parsed are reported with the new
message.
+ @Test
+ public void testNumericStringColumnStillAggregatesInGroupBy() {
+ ExpressionContext expression = RequestContextUtils.getExpression("column");
+ MaxAggregationFunction function = new
MaxAggregationFunction(List.of(expression), false);
+
+ GroupByResultHolder groupByResultHolder =
function.createGroupByResultHolder(10, 20);
+ Map<ExpressionContext, BlockValSet> blockValSetMap = new HashMap<>();
+ BlockValSet mockBlockValSet = mock(BlockValSet.class);
+ when(mockBlockValSet.getValueType()).thenReturn(DataType.STRING);
+ when(mockBlockValSet.isSingleValue()).thenReturn(true);
+ when(mockBlockValSet.getDoubleValuesSV()).thenReturn(new double[]{2, 10});
+ blockValSetMap.put(expression, mockBlockValSet);
+
+ function.aggregateGroupBySV(2, new int[]{0, 0}, groupByResultHolder,
blockValSetMap);
+ assertEquals(groupByResultHolder.getDoubleResult(0), 10.0);
+ }
+
@DataProvider(name = "scenarios")
Object[] scenarios() {
return new Object[] {
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/MinAggregationFunctionTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/MinAggregationFunctionTest.java
index 3ad5beec039..b20df3da57e 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/MinAggregationFunctionTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/MinAggregationFunctionTest.java
@@ -18,17 +18,122 @@
*/
package org.apache.pinot.core.query.aggregation.function;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.common.request.context.RequestContextUtils;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
+import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
import org.apache.pinot.queries.FluentQueryTest;
import org.apache.pinot.spi.data.FieldSpec;
import org.apache.pinot.spi.data.FieldSpec.DataType;
import org.apache.pinot.spi.data.FieldSpec.FieldType;
import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.exception.BadQueryRequestException;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
+
public class MinAggregationFunctionTest extends
AbstractAggregationFunctionTest {
+ /// MIN only supports numeric columns (see
https://github.com/apache/pinot/issues/19603); applying it to a
+ /// non-numeric (e.g. STRING) column must fail with a clear, actionable
message instead of an opaque
+ /// `NumberFormatException`.
+ @Test
+ public void testNonNumericColumnExceptionInAggregateMethod() {
+ ExpressionContext expression = RequestContextUtils.getExpression("column");
+ MinAggregationFunction function = new
MinAggregationFunction(List.of(expression), false);
+
+ AggregationResultHolder resultHolder =
function.createAggregationResultHolder();
+ Map<ExpressionContext, BlockValSet> blockValSetMap = new HashMap<>();
+ BlockValSet mockBlockValSet = mock(BlockValSet.class);
+ when(mockBlockValSet.getValueType()).thenReturn(DataType.STRING);
+ when(mockBlockValSet.isSingleValue()).thenReturn(true);
+ blockValSetMap.put(expression, mockBlockValSet);
+
+ try {
+ function.aggregate(10, resultHolder, blockValSetMap);
+ fail("Should throw BadQueryRequestException");
+ } catch (BadQueryRequestException e) {
+ assertTrue(e.getMessage().contains("Cannot compute min for non-numeric
type"));
+ assertTrue(e.getMessage().contains("MINSTRING"));
+ assertTrue(e.getMessage().contains("autoRewriteAggregationType"));
+ }
+ }
+
+ @Test
+ public void testNonNumericColumnExceptionInAggregateGroupBySVMethod() {
+ ExpressionContext expression = RequestContextUtils.getExpression("column");
+ MinAggregationFunction function = new
MinAggregationFunction(List.of(expression), false);
+
+ GroupByResultHolder groupByResultHolder =
function.createGroupByResultHolder(10, 20);
+ Map<ExpressionContext, BlockValSet> blockValSetMap = new HashMap<>();
+ BlockValSet mockBlockValSet = mock(BlockValSet.class);
+ when(mockBlockValSet.getValueType()).thenReturn(DataType.STRING);
+ when(mockBlockValSet.isSingleValue()).thenReturn(true);
+ when(mockBlockValSet.getDoubleValuesSV()).thenThrow(new
NumberFormatException("For input string: \"abc\""));
+ blockValSetMap.put(expression, mockBlockValSet);
+
+ try {
+ function.aggregateGroupBySV(10, new int[10], groupByResultHolder,
blockValSetMap);
+ fail("Should throw BadQueryRequestException");
+ } catch (BadQueryRequestException e) {
+ assertTrue(e.getMessage().contains("Cannot compute min for non-numeric
value"));
+ assertTrue(e.getMessage().contains("MINSTRING"));
+ assertTrue(e.getMessage().contains("autoRewriteAggregationType"));
+ }
+ }
+
+ @Test
+ public void testNonNumericColumnExceptionInAggregateGroupByMVMethod() {
+ ExpressionContext expression = RequestContextUtils.getExpression("column");
+ MinAggregationFunction function = new
MinAggregationFunction(List.of(expression), false);
+
+ GroupByResultHolder groupByResultHolder =
function.createGroupByResultHolder(10, 20);
+ Map<ExpressionContext, BlockValSet> blockValSetMap = new HashMap<>();
+ BlockValSet mockBlockValSet = mock(BlockValSet.class);
+ when(mockBlockValSet.getValueType()).thenReturn(DataType.STRING);
+ when(mockBlockValSet.isSingleValue()).thenReturn(true);
+ when(mockBlockValSet.getDoubleValuesSV()).thenThrow(new
NumberFormatException("For input string: \"abc\""));
+ blockValSetMap.put(expression, mockBlockValSet);
+
+ try {
+ function.aggregateGroupByMV(10, new int[10][], groupByResultHolder,
blockValSetMap);
+ fail("Should throw BadQueryRequestException");
+ } catch (BadQueryRequestException e) {
+ assertTrue(e.getMessage().contains("Cannot compute min for non-numeric
value"));
+ assertTrue(e.getMessage().contains("MINSTRING"));
+ assertTrue(e.getMessage().contains("autoRewriteAggregationType"));
+ }
+ }
+
+ /// A STRING column holding numeric values keeps working on the group-by
path, since `getDoubleValuesSV()` parses
+ /// them; only values that cannot be parsed are reported with the new
message.
+ @Test
+ public void testNumericStringColumnStillAggregatesInGroupBy() {
+ ExpressionContext expression = RequestContextUtils.getExpression("column");
+ MinAggregationFunction function = new
MinAggregationFunction(List.of(expression), false);
+
+ GroupByResultHolder groupByResultHolder =
function.createGroupByResultHolder(10, 20);
+ Map<ExpressionContext, BlockValSet> blockValSetMap = new HashMap<>();
+ BlockValSet mockBlockValSet = mock(BlockValSet.class);
+ when(mockBlockValSet.getValueType()).thenReturn(DataType.STRING);
+ when(mockBlockValSet.isSingleValue()).thenReturn(true);
+ when(mockBlockValSet.getDoubleValuesSV()).thenReturn(new double[]{2, 10});
+ blockValSetMap.put(expression, mockBlockValSet);
+
+ function.aggregateGroupBySV(2, new int[]{0, 0}, groupByResultHolder,
blockValSetMap);
+ assertEquals(groupByResultHolder.getDoubleResult(0), 2.0);
+ }
+
@DataProvider(name = "scenarios")
Object[] scenarios() {
return new Object[] {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
