This is an automated email from the ASF dual-hosted git repository.
yashmayya 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 77d6de9d42c Fix string parsing for value aggregator (#16519)
77d6de9d42c is described below
commit 77d6de9d42ce7455996296f61b4258a4d02415c0
Author: Xiaotian (Jackie) Jiang <[email protected]>
AuthorDate: Wed Aug 6 01:46:03 2025 -0600
Fix string parsing for value aggregator (#16519)
---
.../pinot/core/data/manager/TableIndexingTest.java | 3 +-
.../pinot/core/startree/v2/MaxStarTreeV2Test.java | 6 ++--
.../pinot/core/startree/v2/MinStarTreeV2Test.java | 6 ++--
.../startree/v2/SumPrecisionStarTreeV2Test.java | 2 +-
.../pinot/core/startree/v2/SumStarTreeV2Test.java | 6 ++--
.../src/test/resources/TableIndexingTest.csv | 4 +--
.../local/aggregator/AvgValueAggregator.java | 4 +--
.../local/aggregator/MaxValueAggregator.java | 10 +++---
.../aggregator/MinMaxRangeValueAggregator.java | 5 ++-
.../local/aggregator/MinValueAggregator.java | 10 +++---
.../aggregator/PercentileEstValueAggregator.java | 16 +++++++--
.../PercentileTDigestValueAggregator.java | 4 +--
.../local/aggregator/SumValueAggregator.java | 10 +++---
.../local/aggregator/ValueAggregatorUtils.java | 40 +++++++---------------
14 files changed, 62 insertions(+), 64 deletions(-)
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/data/manager/TableIndexingTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/data/manager/TableIndexingTest.java
index def39a8bb65..9ca4a67544a 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/data/manager/TableIndexingTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/data/manager/TableIndexingTest.java
@@ -523,7 +523,8 @@ public class TableIndexingTest {
Assert.fail("No expected status found for test case: " + testCase);
} else if (testCase._expectedSuccess && testCase._error != null) {
Assert.fail("Expected success for test case: " + testCase + " but got
error: " + testCase._error);
- } else if (!testCase._expectedSuccess &&
!testCase.getErrorMessage().equals(testCase._expectedMessage)) {
+ } else if (!testCase._expectedSuccess &&
!testCase.getErrorMessage().equals(testCase._expectedMessage)
+ && !testCase.getErrorMessage().matches(testCase._expectedMessage)) {
Assert.fail("Expected error: \"" + testCase._expectedMessage + "\" for
test case: " + testCase + " but got: \""
+ testCase.getErrorMessage() + "\"");
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/MaxStarTreeV2Test.java
b/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/MaxStarTreeV2Test.java
index 1dfcc2cedbf..fa910a2c3ea 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/MaxStarTreeV2Test.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/MaxStarTreeV2Test.java
@@ -26,10 +26,10 @@ import org.apache.pinot.spi.data.FieldSpec.DataType;
import static org.testng.Assert.assertEquals;
-public class MaxStarTreeV2Test extends BaseStarTreeV2Test<Number, Double> {
+public class MaxStarTreeV2Test extends BaseStarTreeV2Test<Object, Double> {
@Override
- ValueAggregator<Number, Double> getValueAggregator() {
+ ValueAggregator<Object, Double> getValueAggregator() {
return new MaxValueAggregator();
}
@@ -39,7 +39,7 @@ public class MaxStarTreeV2Test extends
BaseStarTreeV2Test<Number, Double> {
}
@Override
- Number getRandomRawValue(Random random) {
+ Object getRandomRawValue(Random random) {
return random.nextDouble();
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/MinStarTreeV2Test.java
b/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/MinStarTreeV2Test.java
index b261c76fa25..8d9bc9c9477 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/MinStarTreeV2Test.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/MinStarTreeV2Test.java
@@ -26,10 +26,10 @@ import org.apache.pinot.spi.data.FieldSpec.DataType;
import static org.testng.Assert.assertEquals;
-public class MinStarTreeV2Test extends BaseStarTreeV2Test<Number, Double> {
+public class MinStarTreeV2Test extends BaseStarTreeV2Test<Object, Double> {
@Override
- ValueAggregator<Number, Double> getValueAggregator() {
+ ValueAggregator<Object, Double> getValueAggregator() {
return new MinValueAggregator();
}
@@ -39,7 +39,7 @@ public class MinStarTreeV2Test extends
BaseStarTreeV2Test<Number, Double> {
}
@Override
- Number getRandomRawValue(Random random) {
+ Object getRandomRawValue(Random random) {
return random.nextFloat();
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/SumPrecisionStarTreeV2Test.java
b/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/SumPrecisionStarTreeV2Test.java
index 87c74aa5915..df622a5e03b 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/SumPrecisionStarTreeV2Test.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/SumPrecisionStarTreeV2Test.java
@@ -41,7 +41,7 @@ public class SumPrecisionStarTreeV2Test extends
BaseStarTreeV2Test<Object, BigDe
}
@Override
- Double getRandomRawValue(Random random) {
+ Object getRandomRawValue(Random random) {
return random.nextDouble();
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/SumStarTreeV2Test.java
b/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/SumStarTreeV2Test.java
index dc198c89ed6..e18449a2036 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/SumStarTreeV2Test.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/SumStarTreeV2Test.java
@@ -26,10 +26,10 @@ import org.apache.pinot.spi.data.FieldSpec.DataType;
import static org.testng.Assert.assertEquals;
-public class SumStarTreeV2Test extends BaseStarTreeV2Test<Number, Double> {
+public class SumStarTreeV2Test extends BaseStarTreeV2Test<Object, Double> {
@Override
- ValueAggregator<Number, Double> getValueAggregator() {
+ ValueAggregator<Object, Double> getValueAggregator() {
return new SumValueAggregator();
}
@@ -39,7 +39,7 @@ public class SumStarTreeV2Test extends
BaseStarTreeV2Test<Number, Double> {
}
@Override
- Number getRandomRawValue(Random random) {
+ Object getRandomRawValue(Random random) {
return random.nextInt();
}
diff --git a/pinot-core/src/test/resources/TableIndexingTest.csv
b/pinot-core/src/test/resources/TableIndexingTest.csv
index 8c39fe3bfaf..420f2c72fe7 100644
--- a/pinot-core/src/test/resources/TableIndexingTest.csv
+++ b/pinot-core/src/test/resources/TableIndexingTest.csv
@@ -344,7 +344,7 @@ STRING;sv;dict;json_index;false;Column: col Unrecognized
token 'str': was expect
STRING;sv;dict;native_text_index;true;
STRING;sv;dict;text_index;true;
STRING;sv;dict;range_index;true;
-STRING;sv;dict;startree_index;false;class java.lang.String cannot be cast to
class java.lang.Number (java.lang.String and java.lang.Number are in module
java.base of loader 'bootstrap')
+STRING;sv;dict;startree_index;false;For input string: "str-.*"
STRING;sv;dict;vector_index;false;Cannot create vector index on single-value
column: col
STRING;sv;dict;multi_col_text_index;true;
STRING;mv;dict;timestamp_index;false;Cannot create TIMESTAMP index on column:
col of stored type other than LONG
@@ -380,7 +380,7 @@ JSON;sv;dict;json_index;true;
JSON;sv;dict;native_text_index;false;expected [1] but found [0]
JSON;sv;dict;text_index;false;expected [1] but found [0]
JSON;sv;dict;range_index;true;
-JSON;sv;dict;startree_index;false;class java.lang.String cannot be cast to
class java.lang.Number (java.lang.String and java.lang.Number are in module
java.base of loader 'bootstrap')
+JSON;sv;dict;startree_index;false;For input string: "\{"field":".*"\}"
JSON;sv;dict;vector_index;false;Cannot create vector index on single-value
column: col
JSON;sv;dict;multi_col_text_index;false;Cannot create TEXT index on column:
col of stored type other than STRING
BYTES;sv;raw;timestamp_index;false;Cannot create TIMESTAMP index on column:
col of stored type other than LONG
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/AvgValueAggregator.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/AvgValueAggregator.java
index 6a6124108c2..7ba199642e4 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/AvgValueAggregator.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/AvgValueAggregator.java
@@ -42,7 +42,7 @@ public class AvgValueAggregator implements
ValueAggregator<Object, AvgPair> {
if (rawValue instanceof byte[]) {
return deserializeAggregatedValue((byte[]) rawValue);
} else {
- return new AvgPair(((Number) rawValue).doubleValue(), 1L);
+ return new AvgPair(ValueAggregatorUtils.toDouble(rawValue), 1L);
}
}
@@ -51,7 +51,7 @@ public class AvgValueAggregator implements
ValueAggregator<Object, AvgPair> {
if (rawValue instanceof byte[]) {
value.apply(deserializeAggregatedValue((byte[]) rawValue));
} else {
- value.apply(((Number) rawValue).doubleValue(), 1L);
+ value.apply(ValueAggregatorUtils.toDouble(rawValue), 1L);
}
return value;
}
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/MaxValueAggregator.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/MaxValueAggregator.java
index 13669188731..f5b51833fae 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/MaxValueAggregator.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/MaxValueAggregator.java
@@ -22,7 +22,7 @@ import org.apache.pinot.segment.spi.AggregationFunctionType;
import org.apache.pinot.spi.data.FieldSpec.DataType;
-public class MaxValueAggregator implements ValueAggregator<Number, Double> {
+public class MaxValueAggregator implements ValueAggregator<Object, Double> {
public static final DataType AGGREGATED_VALUE_TYPE = DataType.DOUBLE;
@Override
@@ -36,13 +36,13 @@ public class MaxValueAggregator implements
ValueAggregator<Number, Double> {
}
@Override
- public Double getInitialAggregatedValue(Number rawValue) {
- return rawValue.doubleValue();
+ public Double getInitialAggregatedValue(Object rawValue) {
+ return ValueAggregatorUtils.toDouble(rawValue);
}
@Override
- public Double applyRawValue(Double value, Number rawValue) {
- return Math.max(value, rawValue.doubleValue());
+ public Double applyRawValue(Double value, Object rawValue) {
+ return Math.max(value, ValueAggregatorUtils.toDouble(rawValue));
}
@Override
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/MinMaxRangeValueAggregator.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/MinMaxRangeValueAggregator.java
index 484e22b4f93..d6aeb1195ea 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/MinMaxRangeValueAggregator.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/MinMaxRangeValueAggregator.java
@@ -42,7 +42,7 @@ public class MinMaxRangeValueAggregator implements
ValueAggregator<Object, MinMa
if (rawValue instanceof byte[]) {
return deserializeAggregatedValue((byte[]) rawValue);
} else {
- double doubleValue = ((Number) rawValue).doubleValue();
+ double doubleValue = ValueAggregatorUtils.toDouble(rawValue);
return new MinMaxRangePair(doubleValue, doubleValue);
}
}
@@ -52,8 +52,7 @@ public class MinMaxRangeValueAggregator implements
ValueAggregator<Object, MinMa
if (rawValue instanceof byte[]) {
value.apply(deserializeAggregatedValue((byte[]) rawValue));
} else {
- double doubleValue = ((Number) rawValue).doubleValue();
- value.apply(doubleValue);
+ value.apply(ValueAggregatorUtils.toDouble(rawValue));
}
return value;
}
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/MinValueAggregator.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/MinValueAggregator.java
index c7abc28979e..2ebc95c123c 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/MinValueAggregator.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/MinValueAggregator.java
@@ -22,7 +22,7 @@ import org.apache.pinot.segment.spi.AggregationFunctionType;
import org.apache.pinot.spi.data.FieldSpec.DataType;
-public class MinValueAggregator implements ValueAggregator<Number, Double> {
+public class MinValueAggregator implements ValueAggregator<Object, Double> {
public static final DataType AGGREGATED_VALUE_TYPE = DataType.DOUBLE;
@Override
@@ -36,13 +36,13 @@ public class MinValueAggregator implements
ValueAggregator<Number, Double> {
}
@Override
- public Double getInitialAggregatedValue(Number rawValue) {
- return rawValue.doubleValue();
+ public Double getInitialAggregatedValue(Object rawValue) {
+ return ValueAggregatorUtils.toDouble(rawValue);
}
@Override
- public Double applyRawValue(Double value, Number rawValue) {
- return Math.min(value, rawValue.doubleValue());
+ public Double applyRawValue(Double value, Object rawValue) {
+ return Math.min(value, ValueAggregatorUtils.toDouble(rawValue));
}
@Override
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/PercentileEstValueAggregator.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/PercentileEstValueAggregator.java
index 7ff8a241581..4915461b87e 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/PercentileEstValueAggregator.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/PercentileEstValueAggregator.java
@@ -51,7 +51,7 @@ public class PercentileEstValueAggregator implements
ValueAggregator<Object, Qua
_maxByteSize = Math.max(_maxByteSize, bytes.length);
} else {
initialValue = new QuantileDigest(DEFAULT_MAX_ERROR);
- initialValue.add(((Number) rawValue).longValue());
+ initialValue.add(toLong(rawValue));
_maxByteSize = Math.max(_maxByteSize, initialValue.getByteSize());
}
return initialValue;
@@ -62,12 +62,24 @@ public class PercentileEstValueAggregator implements
ValueAggregator<Object, Qua
if (rawValue instanceof byte[]) {
value.merge(deserializeAggregatedValue((byte[]) rawValue));
} else {
- value.add(((Number) rawValue).longValue());
+ value.add(toLong(rawValue));
}
_maxByteSize = Math.max(_maxByteSize, value.getByteSize());
return value;
}
+ private static long toLong(Object rawValue) {
+ if (rawValue instanceof Number) {
+ return ((Number) rawValue).longValue();
+ }
+ String stringValue = rawValue.toString();
+ try {
+ return Long.parseLong(stringValue);
+ } catch (NumberFormatException e) {
+ return (long) Double.parseDouble(stringValue);
+ }
+ }
+
@Override
public QuantileDigest applyAggregatedValue(QuantileDigest value,
QuantileDigest aggregatedValue) {
value.merge(aggregatedValue);
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/PercentileTDigestValueAggregator.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/PercentileTDigestValueAggregator.java
index da21438a4ed..054b52f9bd7 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/PercentileTDigestValueAggregator.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/PercentileTDigestValueAggregator.java
@@ -61,7 +61,7 @@ public class PercentileTDigestValueAggregator implements
ValueAggregator<Object,
_maxByteSize = Math.max(_maxByteSize, bytes.length);
} else {
initialValue = TDigest.createMergingDigest(_compressionFactor);
- initialValue.add(((Number) rawValue).doubleValue());
+ initialValue.add(ValueAggregatorUtils.toDouble(rawValue));
_maxByteSize = Math.max(_maxByteSize, initialValue.byteSize());
}
return initialValue;
@@ -72,7 +72,7 @@ public class PercentileTDigestValueAggregator implements
ValueAggregator<Object,
if (rawValue instanceof byte[]) {
value.add(deserializeAggregatedValue((byte[]) rawValue));
} else {
- value.add(((Number) rawValue).doubleValue());
+ value.add(ValueAggregatorUtils.toDouble(rawValue));
}
_maxByteSize = Math.max(_maxByteSize, value.byteSize());
return value;
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/SumValueAggregator.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/SumValueAggregator.java
index 429b464c548..1f79b6cc6c8 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/SumValueAggregator.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/SumValueAggregator.java
@@ -22,7 +22,7 @@ import org.apache.pinot.segment.spi.AggregationFunctionType;
import org.apache.pinot.spi.data.FieldSpec.DataType;
-public class SumValueAggregator implements ValueAggregator<Number, Double> {
+public class SumValueAggregator implements ValueAggregator<Object, Double> {
public static final DataType AGGREGATED_VALUE_TYPE = DataType.DOUBLE;
@Override
@@ -36,13 +36,13 @@ public class SumValueAggregator implements
ValueAggregator<Number, Double> {
}
@Override
- public Double getInitialAggregatedValue(Number rawValue) {
- return rawValue.doubleValue();
+ public Double getInitialAggregatedValue(Object rawValue) {
+ return ValueAggregatorUtils.toDouble(rawValue);
}
@Override
- public Double applyRawValue(Double value, Number rawValue) {
- return value + rawValue.doubleValue();
+ public Double applyRawValue(Double value, Object rawValue) {
+ return value + ValueAggregatorUtils.toDouble(rawValue);
}
@Override
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/SumStarTreeV2Test.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/ValueAggregatorUtils.java
similarity index 50%
copy from
pinot-core/src/test/java/org/apache/pinot/core/startree/v2/SumStarTreeV2Test.java
copy to
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/ValueAggregatorUtils.java
index dc198c89ed6..3728e29baf6 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/SumStarTreeV2Test.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/ValueAggregatorUtils.java
@@ -16,35 +16,21 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.core.startree.v2;
+package org.apache.pinot.segment.local.aggregator;
-import java.util.Random;
-import org.apache.pinot.segment.local.aggregator.SumValueAggregator;
-import org.apache.pinot.segment.local.aggregator.ValueAggregator;
-import org.apache.pinot.spi.data.FieldSpec.DataType;
-
-import static org.testng.Assert.assertEquals;
-
-
-public class SumStarTreeV2Test extends BaseStarTreeV2Test<Number, Double> {
-
- @Override
- ValueAggregator<Number, Double> getValueAggregator() {
- return new SumValueAggregator();
- }
-
- @Override
- DataType getRawValueType() {
- return DataType.INT;
- }
-
- @Override
- Number getRandomRawValue(Random random) {
- return random.nextInt();
+public class ValueAggregatorUtils {
+ private ValueAggregatorUtils() {
}
- @Override
- protected void assertAggregatedValue(Double starTreeResult, Double
nonStarTreeResult) {
- assertEquals(starTreeResult, nonStarTreeResult, 1e-5);
+ /// Tries to convert the given value to a double.
+ /// We need this for [ValueAggregator] because the raw value might not be
converted to the desired data type yet if it
+ /// is not specified in the schema.
+ /// TODO: Provide a way to specify the desired data type for the raw value.
+ public static double toDouble(Object value) {
+ if (value instanceof Number) {
+ return ((Number) value).doubleValue();
+ } else {
+ return Double.parseDouble(value.toString());
+ }
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
