You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ro...@apache.org on 2022/11/16 21:36:30 UTC
[pinot] branch master updated: [multistage] add big_decimal support and numeric type tests (#9806)
This is an automated email from the ASF dual-hosted git repository.
rongr 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 9aa49778f2 [multistage] add big_decimal support and numeric type tests (#9806)
9aa49778f2 is described below
commit 9aa49778f22540ab9f7e41ba623d948a7398fce2
Author: Almog Gavra <al...@gmail.com>
AuthorDate: Wed Nov 16 13:36:23 2022 -0800
[multistage] add big_decimal support and numeric type tests (#9806)
* [multistage] add big_decimal support and numeric type tests
Co-authored-by: Rong Rong <ro...@apache.org>
---
.../org/apache/pinot/query/type/TypeFactory.java | 2 +
.../pinot/query/runtime/QueryRunnerTestBase.java | 24 ++++-
.../runtime/queries/ResourceBasedQueriesTest.java | 53 ++++++++--
.../src/test/resources/queries/NumericTypes.json | 117 +++++++++++++++++++++
.../java/org/apache/pinot/spi/data/FieldSpec.java | 3 +
5 files changed, 187 insertions(+), 12 deletions(-)
diff --git a/pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeFactory.java b/pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeFactory.java
index 55797b0e52..9354ccfb95 100644
--- a/pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeFactory.java
+++ b/pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeFactory.java
@@ -70,6 +70,8 @@ public class TypeFactory extends JavaTypeFactoryImpl {
return createSqlType(SqlTypeName.VARCHAR);
case BYTES:
return createSqlType(SqlTypeName.VARBINARY);
+ case BIG_DECIMAL:
+ return createSqlType(SqlTypeName.DECIMAL);
case JSON:
// TODO: support JSON, JSON should be supported using a special RelDataType as it is not a simple String,
// nor can it be easily parsed as a STRUCT.
diff --git a/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTestBase.java b/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTestBase.java
index 3bffc35439..80fd4a98cb 100644
--- a/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTestBase.java
+++ b/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/QueryRunnerTestBase.java
@@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
+import java.math.BigDecimal;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
@@ -36,6 +37,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Random;
+import java.util.stream.Collectors;
import org.apache.pinot.core.transport.ServerInstance;
import org.apache.pinot.query.QueryEnvironment;
import org.apache.pinot.query.QueryServerEnclosure;
@@ -109,7 +111,10 @@ public abstract class QueryRunnerTestBase extends QueryTestSet {
}
protected void compareRowEquals(List<Object[]> resultRows, List<Object[]> expectedRows) {
- Assert.assertEquals(resultRows.size(), expectedRows.size());
+ Assert.assertEquals(resultRows.size(), expectedRows.size(),
+ String.format("Mismatched number of results. expected: %s, actual: %s",
+ expectedRows.stream().map(Arrays::toString).collect(Collectors.joining(",\n")),
+ resultRows.stream().map(Arrays::toString).collect(Collectors.joining(",\n"))));
Comparator<Object> valueComp = (l, r) -> {
if (l == null && r == null) {
@@ -131,6 +136,12 @@ public abstract class QueryRunnerTestBase extends QueryTestSet {
return ((String) l).compareTo((String) r);
} else if (l instanceof Boolean) {
return ((Boolean) l).compareTo((Boolean) r);
+ } else if (l instanceof BigDecimal) {
+ if (r instanceof BigDecimal) {
+ return ((BigDecimal) l).compareTo((BigDecimal) r);
+ } else {
+ return ((BigDecimal) l).compareTo(new BigDecimal((String) r));
+ }
} else {
throw new RuntimeException("non supported type " + l.getClass());
}
@@ -150,6 +161,9 @@ public abstract class QueryRunnerTestBase extends QueryTestSet {
for (int i = 0; i < resultRows.size(); i++) {
Object[] resultRow = resultRows.get(i);
Object[] expectedRow = expectedRows.get(i);
+ Assert.assertEquals(expectedRow.length, resultRow.length,
+ String.format("Unexpected row size mismatch. Expected: %s, Actual: %s", Arrays.toString(expectedRow),
+ Arrays.toString(resultRow)));
for (int j = 0; j < resultRow.length; j++) {
Assert.assertEquals(valueComp.compare(resultRow[j], expectedRow[j]), 0,
"Not match at (" + i + "," + j + ")! Expected: " + Arrays.toString(expectedRow)
@@ -248,9 +262,15 @@ public abstract class QueryRunnerTestBase extends QueryTestSet {
case STRING:
fieldType = "varchar(128)";
break;
+ case FLOAT:
+ fieldType = "real";
+ break;
case DOUBLE:
fieldType = "double";
break;
+ case BIG_DECIMAL:
+ fieldType = "NUMERIC";
+ break;
default:
throw new UnsupportedOperationException("Unsupported type conversion to h2 type: " + dataType);
}
@@ -292,6 +312,8 @@ public abstract class QueryRunnerTestBase extends QueryTestSet {
public String _description;
@JsonProperty("outputs")
public List<List<Object>> _outputs = Collections.emptyList();
+ @JsonProperty("expectedException")
+ public String _expectedException;
}
public static class ColumnAndType {
diff --git a/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/ResourceBasedQueriesTest.java b/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/ResourceBasedQueriesTest.java
index 36e7d688da..f9de64c8bc 100644
--- a/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/ResourceBasedQueriesTest.java
+++ b/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/ResourceBasedQueriesTest.java
@@ -24,11 +24,14 @@ import com.google.common.collect.ImmutableList;
import java.io.File;
import java.net.URL;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import java.util.stream.Collectors;
import org.apache.pinot.common.datatable.DataTableFactory;
import org.apache.pinot.core.common.datatable.DataTableBuilderFactory;
import org.apache.pinot.query.QueryEnvironmentTestBase;
@@ -43,6 +46,7 @@ import org.apache.pinot.spi.config.table.TableType;
import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
+import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.DataProvider;
@@ -58,7 +62,8 @@ public class ResourceBasedQueriesTest extends QueryRunnerTestBase {
"BasicQuery.json",
"SpecialSyntax.json",
"LexicalStructure.json",
- "ValueExpressions.json"
+ "ValueExpressions.json",
+ "NumericTypes.json"
);
@BeforeClass
@@ -153,20 +158,46 @@ public class ResourceBasedQueriesTest extends QueryRunnerTestBase {
// TODO: name the test using testCaseName for testng reports
@Test(dataProvider = "testResourceQueryTestCaseProviderInputOnly")
- public void testQueryTestCasesWithH2(String testCaseName, String sql)
+ public void testQueryTestCasesWithH2(String testCaseName, String sql, String expect)
throws Exception {
// query pinot
- List<Object[]> resultRows = queryRunner(sql);
- // query H2 for data
- List<Object[]> expectedRows = queryH2(sql);
- compareRowEquals(resultRows, expectedRows);
+ runQuery(sql, expect).ifPresent(rows -> {
+ try {
+ compareRowEquals(rows, queryH2(sql));
+ } catch (Exception e) {
+ Assert.fail(e.getMessage());
+ }
+ });
}
@Test(dataProvider = "testResourceQueryTestCaseProviderBoth")
- public void testQueryTestCasesWithOutput(String testCaseName, String sql, List<Object[]> expectedRows)
+ public void testQueryTestCasesWithOutput(String testCaseName, String sql, List<Object[]> expectedRows, String expect)
throws Exception {
- List<Object[]> resultRows = queryRunner(sql);
- compareRowEquals(resultRows, expectedRows);
+ runQuery(sql, expect).ifPresent(rows -> compareRowEquals(rows, expectedRows));
+ }
+
+ private Optional<List<Object[]>> runQuery(String sql, final String except) {
+ try {
+ // query pinot
+ List<Object[]> resultRows = queryRunner(sql);
+
+ Assert.assertNull(except,
+ "Expected error with message '" + except + "'. But instead rows were returned: "
+ + resultRows.stream().map(Arrays::toString).collect(Collectors.joining(",\n")));
+
+ return Optional.of(resultRows);
+ } catch (Exception e) {
+ if (except == null) {
+ throw e;
+ } else {
+ Pattern pattern = Pattern.compile(except);
+ Assert.assertTrue(pattern.matcher(e.getMessage()).matches(),
+ String.format("Caught exception %s, but it did not match the expected pattern %s.",
+ e.getMessage(), except));
+ }
+ }
+
+ return Optional.empty();
}
@DataProvider
@@ -193,7 +224,7 @@ public class ResourceBasedQueriesTest extends QueryRunnerTestBase {
for (List<Object> objs : orgRows) {
expectedRows.add(objs.toArray());
}
- Object[] testEntry = new Object[]{testCaseName, sql, expectedRows};
+ Object[] testEntry = new Object[]{testCaseName, sql, expectedRows, queryCase._expectedException};
providerContent.add(testEntry);
}
}
@@ -219,7 +250,7 @@ public class ResourceBasedQueriesTest extends QueryRunnerTestBase {
}
if (queryCase._outputs == null || queryCase._outputs.isEmpty()) {
String sql = replaceTableName(testCaseName, queryCase._sql);
- Object[] testEntry = new Object[]{testCaseName, sql};
+ Object[] testEntry = new Object[]{testCaseName, sql, queryCase._expectedException};
providerContent.add(testEntry);
}
}
diff --git a/pinot-query-runtime/src/test/resources/queries/NumericTypes.json b/pinot-query-runtime/src/test/resources/queries/NumericTypes.json
new file mode 100644
index 0000000000..a5cc4e4e78
--- /dev/null
+++ b/pinot-query-runtime/src/test/resources/queries/NumericTypes.json
@@ -0,0 +1,117 @@
+{
+ "smallint": {
+ "psql": "8.1.1",
+ "ignored": true,
+ "comment": "not supported"
+ },
+ "integers": {
+ "comment": "we don't support BIGINT notation",
+ "tables": {
+ "ints": {
+ "schema": [
+ {"name": "int32", "type": "INT"},
+ {"name": "int64", "type": "LONG"}
+ ],
+ "inputs": [
+ [0, 0],
+ [123, 321],
+ [-2147483648, -9223372036854775808],
+ [2147483647, 9223372036854775807]
+ ]
+ }
+ },
+ "queries": [
+ {
+ "psql": "8.1.1",
+ "description": "test selecting integer values",
+ "sql": "SELECT * FROM {ints}"
+ },
+ {
+ "psql": "8.1.1",
+ "ignored": true,
+ "comment": "we don't properly support overflow behavior because we return doubles",
+ "description": "overflow behavior",
+ "sql": "SELECT int32 + 1, int32 - 1, int64 + 1, int64 -1 FROM {ints}",
+ "expectedException": "*out of range*"
+ }
+ ]
+ },
+ "numeric": {
+ "comment": "ANSI SQL calls this type NUMERIC(precision, scale) but we call it BIG_DECIMAL",
+ "tables": {
+ "numeric": {
+ "schema": [
+ {"name": "big", "type": "BIG_DECIMAL"}
+ ],
+ "inputs": [
+ ["92233720368547758071"],
+ ["92233720368547758071.0000000001"]
+ ]
+ }
+ },
+ "queries": [
+ {
+ "psql": "8.1.2",
+ "TODO": "for some reason, when we select * with this test the timestamp column gets returned as well...",
+ "description": "test support for numeric types of arbitrary precision and scale with values larger than long can support",
+ "sql": "SELECT big FROM {numeric}"
+ }
+ ]
+ },
+ "nan_infinity": {
+ "ignored": true,
+ "comment": "we don't support NaN or Infinity",
+ "tables": {
+ "numeric": {
+ "schema": [
+ {"name": "big", "type": "BIG_DECIMAL"},
+ {"name": "float", "type": "FLOAT"},
+ {"name": "double", "type": "DOUBLE"}
+ ],
+ "inputs": [
+ ["NaN", "NaN", "NaN"],
+ ["Infinity", "Infinity", "Infinity"],
+ ["-Infinity", "-Infinity", "-Infinity"]
+ ]
+ }
+ },
+ "queries": [
+ {
+ "psql": "8.1.2",
+ "TODO": "for some reason, when we select * with this test the timestamp column gets returned as well...",
+ "sql": "SELECT big, floatv, doublev FROM {numeric}",
+ "outputs": [
+ ["NaN"],
+ ["Infinity"],
+ ["-Infinity"]
+ ]
+ }
+ ]
+ },
+ "floating_point": {
+ "tables": {
+ "floats": {
+ "schema": [
+ {"name": "floatv", "type": "FLOAT"},
+ {"name": "doublev", "type": "DOUBLE"}
+ ],
+ "inputs": [
+ [0, 0],
+ [123.456, 123.456],
+ [1E-37, 1E-307],
+ [1E+37, 1E+307]
+ ]
+ }
+ },
+ "queries": [
+ {
+ "description": "test floating point precision",
+ "sql": "SELECT * FROM {floats}"
+ },
+ {
+ "description": "test floating point overflow",
+ "sql": "SELECT floatv + 1, doublev + 1 FROM {floats}"
+ }
+ ]
+ }
+}
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/data/FieldSpec.java b/pinot-spi/src/main/java/org/apache/pinot/spi/data/FieldSpec.java
index 02c825470f..9b3ea0862b 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/data/FieldSpec.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/data/FieldSpec.java
@@ -55,6 +55,7 @@ public abstract class FieldSpec implements Comparable<FieldSpec>, Serializable {
public static final String DEFAULT_DIMENSION_NULL_VALUE_OF_STRING = "null";
public static final String DEFAULT_DIMENSION_NULL_VALUE_OF_JSON = "null";
public static final byte[] DEFAULT_DIMENSION_NULL_VALUE_OF_BYTES = new byte[0];
+ public static final BigDecimal DEFAULT_DIMENSION_NULL_VALUE_OF_BIG_DECIMAL = BigDecimal.ZERO;
public static final Integer DEFAULT_METRIC_NULL_VALUE_OF_INT = 0;
public static final Long DEFAULT_METRIC_NULL_VALUE_OF_LONG = 0L;
@@ -238,6 +239,8 @@ public abstract class FieldSpec implements Comparable<FieldSpec>, Serializable {
return DEFAULT_DIMENSION_NULL_VALUE_OF_JSON;
case BYTES:
return DEFAULT_DIMENSION_NULL_VALUE_OF_BYTES;
+ case BIG_DECIMAL:
+ return DEFAULT_DIMENSION_NULL_VALUE_OF_BIG_DECIMAL;
default:
throw new IllegalStateException("Unsupported dimension/time data type: " + dataType);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org