This is an automated email from the ASF dual-hosted git repository.
jackie 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 38acdc6d29 Use string to represent BigDecimal response (#11716)
38acdc6d29 is described below
commit 38acdc6d29229a74235a86ea6b7dd7603ca35bf9
Author: Xiaotian (Jackie) Jiang <[email protected]>
AuthorDate: Mon Oct 2 13:31:45 2023 -0700
Use string to represent BigDecimal response (#11716)
---
.../org/apache/pinot/common/utils/DataSchema.java | 7 +-
.../apache/pinot/common/utils/DataSchemaTest.java | 10 +
.../pinot/queries/BigDecimalQueriesTest.java | 79 ++---
.../apache/pinot/queries/DistinctQueriesTest.java | 6 +-
.../org/apache/pinot/queries/ExprMinMaxTest.java | 4 +-
.../query/runtime/queries/QueryRunnerTest.java | 18 +-
.../query/runtime/queries/QueryRunnerTestBase.java | 329 ++++++++++++---------
.../runtime/queries/ResourceBasedQueriesTest.java | 29 +-
8 files changed, 256 insertions(+), 226 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
index fabad7a049..e9a7e858fe 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java
@@ -449,10 +449,13 @@ public class DataSchema {
}
/**
- * Formats the value to human-readable format based on the type to be used
in the query response.
+ * Formats the value based on the type to be used in the JSON query
response. For BIG_DECIMAL, even though JSON can
+ * serialize BigDecimal, it is best practice to convert it to String to
avoid precision loss during deserialization.
*/
public Serializable format(Object value) {
switch (this) {
+ case BIG_DECIMAL:
+ return ((BigDecimal) value).toPlainString();
case TIMESTAMP:
assert value instanceof Timestamp;
return value.toString();
@@ -479,7 +482,7 @@ public class DataSchema {
case DOUBLE:
return ((Number) value).doubleValue();
case BIG_DECIMAL:
- return (BigDecimal) value;
+ return ((BigDecimal) value).toPlainString();
case BOOLEAN:
return ((int) value) == 1;
case TIMESTAMP:
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/utils/DataSchemaTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/utils/DataSchemaTest.java
index 65eae2ce97..3a22b1d30c 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/utils/DataSchemaTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/utils/DataSchemaTest.java
@@ -18,8 +18,11 @@
*/
package org.apache.pinot.common.utils;
+import java.math.BigDecimal;
import java.nio.ByteBuffer;
+import java.sql.Timestamp;
import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.utils.BytesUtils;
import org.testng.Assert;
import org.testng.annotations.Test;
@@ -178,5 +181,12 @@ public class DataSchemaTest {
Assert.assertEquals(fromDataType(FieldSpec.DataType.BOOLEAN, false),
BOOLEAN_ARRAY);
Assert.assertEquals(fromDataType(FieldSpec.DataType.TIMESTAMP, false),
TIMESTAMP_ARRAY);
Assert.assertEquals(fromDataType(FieldSpec.DataType.BYTES, false),
BYTES_ARRAY);
+
+ BigDecimal bigDecimalValue = new
BigDecimal("1.2345678901234567890123456789");
+ Assert.assertEquals(BIG_DECIMAL.format(bigDecimalValue),
bigDecimalValue.toPlainString());
+ Timestamp timestampValue = new Timestamp(1234567890123L);
+ Assert.assertEquals(TIMESTAMP.format(timestampValue),
timestampValue.toString());
+ byte[] bytesValue = {12, 34, 56};
+ Assert.assertEquals(BYTES.format(bytesValue),
BytesUtils.toHexString(bytesValue));
}
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/queries/BigDecimalQueriesTest.java
b/pinot-core/src/test/java/org/apache/pinot/queries/BigDecimalQueriesTest.java
index 9e37d669bb..3d50c89484 100644
---
a/pinot-core/src/test/java/org/apache/pinot/queries/BigDecimalQueriesTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/queries/BigDecimalQueriesTest.java
@@ -138,9 +138,7 @@ public class BigDecimalQueriesTest extends BaseQueriesTest {
@Test
public void testQueriesWithDictColumn()
throws Exception {
- TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
- .setTableName(RAW_TABLE_NAME)
- .build();
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).build();
setUp(tableConfig);
testQueries();
}
@@ -150,10 +148,8 @@ public class BigDecimalQueriesTest extends BaseQueriesTest
{
throws Exception {
List<String> noDictionaryColumns = new ArrayList<String>();
noDictionaryColumns.add(BIG_DECIMAL_COLUMN);
- TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
- .setTableName(RAW_TABLE_NAME)
- .setNoDictionaryColumns(noDictionaryColumns)
- .build();
+ TableConfig tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME)
+ .setNoDictionaryColumns(noDictionaryColumns).build();
setUp(tableConfig);
testQueries();
}
@@ -174,7 +170,7 @@ public class BigDecimalQueriesTest extends BaseQueriesTest {
Object[] row = rows.get(i);
assertEquals(row.length, 1);
if (row[0] != null) {
- assertEquals(row[0], BASE_BIG_DECIMAL.add(BigDecimal.valueOf(i)));
+ assertEquals(row[0],
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(i)).toPlainString());
}
}
}
@@ -204,7 +200,7 @@ public class BigDecimalQueriesTest extends BaseQueriesTest {
if (k >= NUM_RECORDS) {
assertNull(values[0]);
} else {
- assertEquals(values[0],
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(NUM_RECORDS - 1 - k)));
+ assertEquals(values[0],
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(NUM_RECORDS - 1 - k)).toPlainString());
}
}
k++;
@@ -228,7 +224,7 @@ public class BigDecimalQueriesTest extends BaseQueriesTest {
if (i % 4 == 3) {
i++;
}
- assertEquals(row[0], BASE_BIG_DECIMAL.add(BigDecimal.valueOf(i)));
+ assertEquals(row[0],
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(i)).toPlainString());
i++;
}
// The default null ordering is 'NULLS LAST'. Therefore, null will
appear as the last record.
@@ -236,8 +232,8 @@ public class BigDecimalQueriesTest extends BaseQueriesTest {
}
{
int limit = 40;
- String query = String.format("SELECT DISTINCT %s FROM testTable ORDER BY
%s LIMIT %d",
- BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN, limit);
+ String query = String.format("SELECT DISTINCT %s FROM testTable ORDER BY
%s LIMIT %d", BIG_DECIMAL_COLUMN,
+ BIG_DECIMAL_COLUMN, limit);
BrokerResponseNative brokerResponse = getBrokerResponse(query,
queryOptions);
ResultTable resultTable = brokerResponse.getResultTable();
DataSchema dataSchema = resultTable.getDataSchema();
@@ -255,7 +251,7 @@ public class BigDecimalQueriesTest extends BaseQueriesTest {
if (i % 4 == 3) {
i++;
}
- assertEquals(row[0], BASE_BIG_DECIMAL.add(BigDecimal.valueOf(i)));
+ assertEquals(row[0],
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(i)).toPlainString());
i++;
index++;
}
@@ -285,13 +281,14 @@ public class BigDecimalQueriesTest extends
BaseQueriesTest {
assertEquals((long) rows.get(0)[0], 3 * NUM_RECORDS);
}
{
- String query = String.format("SELECT %s FROM testTable GROUP BY %s ORDER
BY %s DESC",
- BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN);
+ String query =
+ String.format("SELECT %s FROM testTable GROUP BY %s ORDER BY %s
DESC", BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN,
+ BIG_DECIMAL_COLUMN);
BrokerResponseNative brokerResponse = getBrokerResponse(query,
queryOptions);
ResultTable resultTable = brokerResponse.getResultTable();
DataSchema dataSchema = resultTable.getDataSchema();
- assertEquals(dataSchema, new DataSchema(new String[]{BIG_DECIMAL_COLUMN},
- new ColumnDataType[]{ColumnDataType.BIG_DECIMAL}));
+ assertEquals(dataSchema,
+ new DataSchema(new String[]{BIG_DECIMAL_COLUMN}, new
ColumnDataType[]{ColumnDataType.BIG_DECIMAL}));
List<Object[]> rows = resultTable.getRows();
assertEquals(rows.size(), 10);
// The default null ordering is 'NULLS LAST'. Therefore, null will
appear as the last record.
@@ -304,7 +301,7 @@ public class BigDecimalQueriesTest extends BaseQueriesTest {
}
Object[] row = rows.get(index);
assertEquals(row.length, 1);
- assertEquals(row[0],
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(NUM_RECORDS - i - 1)));
+ assertEquals(row[0],
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(NUM_RECORDS - i - 1)).toPlainString());
index++;
i++;
}
@@ -329,7 +326,7 @@ public class BigDecimalQueriesTest extends BaseQueriesTest {
// Null values are inserted at: index % 4 == 3. All null values are
grouped into a single null.
i++;
}
- assertEquals(row[1],
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(NUM_RECORDS - i - 1)));
+ assertEquals(row[1],
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(NUM_RECORDS - i - 1)).toPlainString());
i++;
}
}
@@ -346,8 +343,9 @@ public class BigDecimalQueriesTest extends BaseQueriesTest {
{
// Note: defining decimal literals within quotes preserves precision.
BigDecimal lowerLimit = BASE_BIG_DECIMAL.add(BigDecimal.valueOf(69));
- String query = String.format("SELECT %s FROM testTable WHERE %s > '%s'
LIMIT 30",
- BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN, lowerLimit);
+ String query =
+ String.format("SELECT %s FROM testTable WHERE %s > '%s' LIMIT 30",
BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN,
+ lowerLimit);
BrokerResponseNative brokerResponse = getBrokerResponse(query,
queryOptions);
ResultTable resultTable = brokerResponse.getResultTable();
DataSchema dataSchema = resultTable.getDataSchema();
@@ -363,14 +361,14 @@ public class BigDecimalQueriesTest extends
BaseQueriesTest {
// Null values are inserted at: index % 4 == 3.
i++;
}
- assertEquals(row[0], BASE_BIG_DECIMAL.add(BigDecimal.valueOf(69 + i +
1)));
+ assertEquals(row[0], BASE_BIG_DECIMAL.add(BigDecimal.valueOf(69 + i +
1)).toPlainString());
i++;
}
}
{
// Note: defining decimal literals within quotes preserves precision.
- String query = String.format("SELECT %s FROM testTable WHERE %s = '%s'",
- BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN,
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(69)));
+ String query = String.format("SELECT %s FROM testTable WHERE %s = '%s'",
BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN,
+ BASE_BIG_DECIMAL.add(BigDecimal.valueOf(69)));
BrokerResponseNative brokerResponse = getBrokerResponse(query,
queryOptions);
ResultTable resultTable = brokerResponse.getResultTable();
DataSchema dataSchema = resultTable.getDataSchema();
@@ -381,31 +379,17 @@ public class BigDecimalQueriesTest extends
BaseQueriesTest {
for (int i = 0; i < 4; i++) {
Object[] row = rows.get(i);
assertEquals(row.length, 1);
- assertEquals(row[0], BASE_BIG_DECIMAL.add(BigDecimal.valueOf(69)));
+ assertEquals(row[0],
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(69)).toPlainString());
}
}
{
- // This returns currently 25 rows instead of a single row!
-// int limit = 25;
-// String query = String.format(
-// "SELECT SUMPRECISION(%s) AS sum FROM (SELECT %s FROM testTable
ORDER BY %s LIMIT %d)",
-// BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN, limit);
-// BrokerResponseNative brokerResponse = getBrokerResponse(query);
-// ResultTable resultTable = brokerResponse.getResultTable();
-// DataSchema dataSchema = resultTable.getDataSchema();
-// assertEquals(dataSchema, new DataSchema(new String[]{"sum"}, new
ColumnDataType[]{ColumnDataType.BIG_DECIMAL}));
-// List<Object[]> rows = resultTable.getRows();
-// assertEquals(rows.size(), 1);
- }
- {
- String query = String.format(
- "SELECT MAX(%s) AS maxValue FROM testTable GROUP BY %s HAVING
maxValue < %s ORDER BY maxValue",
- BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN,
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(5)));
+ String query =
+ String.format("SELECT MAX(%s) AS maxValue FROM testTable GROUP BY %s
HAVING maxValue < %s ORDER BY maxValue",
+ BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN,
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(5)));
BrokerResponseNative brokerResponse = getBrokerResponse(query,
queryOptions);
ResultTable resultTable = brokerResponse.getResultTable();
DataSchema dataSchema = resultTable.getDataSchema();
- assertEquals(dataSchema,
- new DataSchema(new String[]{"maxValue"}, new
ColumnDataType[]{ColumnDataType.DOUBLE}));
+ assertEquals(dataSchema, new DataSchema(new String[]{"maxValue"}, new
ColumnDataType[]{ColumnDataType.DOUBLE}));
List<Object[]> rows = resultTable.getRows();
// The default null ordering is: 'NULLS LAST'. This is why the number of
returned value is 4 and not 5.
assertEquals(rows.size(), 4);
@@ -423,14 +407,13 @@ public class BigDecimalQueriesTest extends
BaseQueriesTest {
}
{
int lowerLimit = 991;
- String query = String.format(
- "SELECT MAX(%s) AS maxValue FROM testTable GROUP BY %s HAVING
maxValue > %s ORDER BY maxValue",
- BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN,
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(lowerLimit)));
+ String query =
+ String.format("SELECT MAX(%s) AS maxValue FROM testTable GROUP BY %s
HAVING maxValue > %s ORDER BY maxValue",
+ BIG_DECIMAL_COLUMN, BIG_DECIMAL_COLUMN,
BASE_BIG_DECIMAL.add(BigDecimal.valueOf(lowerLimit)));
BrokerResponseNative brokerResponse = getBrokerResponse(query,
queryOptions);
ResultTable resultTable = brokerResponse.getResultTable();
DataSchema dataSchema = resultTable.getDataSchema();
- assertEquals(dataSchema,
- new DataSchema(new String[]{"maxValue"}, new
ColumnDataType[]{ColumnDataType.DOUBLE}));
+ assertEquals(dataSchema, new DataSchema(new String[]{"maxValue"}, new
ColumnDataType[]{ColumnDataType.DOUBLE}));
List<Object[]> rows = resultTable.getRows();
assertEquals(rows.size(), 6);
int i = lowerLimit;
diff --git
a/pinot-core/src/test/java/org/apache/pinot/queries/DistinctQueriesTest.java
b/pinot-core/src/test/java/org/apache/pinot/queries/DistinctQueriesTest.java
index 0d01502b21..0f8da45821 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/DistinctQueriesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/DistinctQueriesTest.java
@@ -1251,7 +1251,7 @@ public class DistinctQueriesTest extends BaseQueriesTest {
assertEquals(((Long) row[1]).intValue(), intValue);
assertEquals(((Float) row[2]).intValue(), intValue);
assertEquals(((Double) row[3]).intValue(), intValue);
- assertEquals(((BigDecimal) row[4]).intValue(), intValue);
+ assertEquals(Integer.parseInt((String) row[4]), intValue);
assertEquals(Integer.parseInt((String) row[5]), intValue);
assertEquals(new String(BytesUtils.toBytes((String) row[6]),
UTF_8).trim(), row[5]);
actualValues.add(intValue);
@@ -1319,7 +1319,7 @@ public class DistinctQueriesTest extends BaseQueriesTest {
for (Object[] row : rows) {
int intValue = ((Long) row[0]).intValue();
List<Integer> actualValueList =
- Arrays.asList(intValue, ((BigDecimal) row[1]).intValue(), ((Float)
row[2]).intValue(),
+ Arrays.asList(intValue, Integer.parseInt((String) row[1]),
((Float) row[2]).intValue(),
Integer.parseInt((String) row[3]));
assertEquals((int) actualValueList.get(1), intValue);
List<Integer> expectedMVValues = new ArrayList<>(2);
@@ -1522,7 +1522,7 @@ public class DistinctQueriesTest extends BaseQueriesTest {
for (Object[] row : rows) {
int intValue = ((Long) row[0]).intValue();
List<Integer> actualValueList =
- Arrays.asList(intValue, ((BigDecimal) row[1]).intValue(), ((Float)
row[2]).intValue(),
+ Arrays.asList(intValue, Integer.parseInt((String) row[1]),
((Float) row[2]).intValue(),
Integer.parseInt((String) row[3]));
assertEquals((int) actualValueList.get(1), intValue);
List<Integer> expectedMVValues = new ArrayList<>(2);
diff --git
a/pinot-core/src/test/java/org/apache/pinot/queries/ExprMinMaxTest.java
b/pinot-core/src/test/java/org/apache/pinot/queries/ExprMinMaxTest.java
index 413685dbe8..564574b96d 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/ExprMinMaxTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/ExprMinMaxTest.java
@@ -257,8 +257,8 @@ public class ExprMinMaxTest extends BaseQueriesTest {
assertEquals(rows.get(1)[4], new String[]{"a0", "a01", "a02"});
assertEquals(rows.get(0)[5], 0);
assertEquals(rows.get(1)[5], 0);
- assertEquals(rows.get(0)[6], new BigDecimal(360000));
- assertEquals(rows.get(1)[6], new BigDecimal(360000));
+ assertEquals(rows.get(0)[6], "360000");
+ assertEquals(rows.get(1)[6], "360000");
assertEquals(rows.get(0)[7], 600D);
assertEquals(rows.get(1)[7], 600D);
assertEquals(rows.get(0)[8], 1683138373879L - 1999L);
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 d54eb5b1d0..eb11087c2b 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
@@ -24,7 +24,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
-import java.util.stream.Collectors;
+import org.apache.pinot.common.response.broker.ResultTable;
import org.apache.pinot.core.common.datatable.DataTableBuilderFactory;
import org.apache.pinot.query.QueryEnvironmentTestBase;
import org.apache.pinot.query.QueryServerEnclosure;
@@ -38,6 +38,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.JsonUtils;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
@@ -150,8 +151,8 @@ public class QueryRunnerTest extends QueryRunnerTestBase {
*/
@Test(dataProvider = "testDataWithSqlToFinalRowCount")
public void testSqlWithFinalRowCountChecker(String sql, int expectedRows) {
- List<Object[]> resultRows = queryRunner(sql, null);
- Assert.assertEquals(resultRows.size(), expectedRows);
+ ResultTable resultTable = queryRunner(sql, null);
+ Assert.assertEquals(resultTable.getRows().size(), expectedRows);
}
/**
@@ -163,10 +164,10 @@ public class QueryRunnerTest extends QueryRunnerTestBase {
@Test(dataProvider = "testSql")
public void testSqlWithH2Checker(String sql)
throws Exception {
- List<Object[]> resultRows = queryRunner(sql, null);
+ ResultTable resultTable = queryRunner(sql, null);
// query H2 for data
List<Object[]> expectedRows = queryH2(sql);
- compareRowEquals(resultRows, expectedRows);
+ compareRowEquals(resultTable, expectedRows);
}
/**
@@ -176,10 +177,9 @@ public class QueryRunnerTest extends QueryRunnerTestBase {
public void testSqlWithExceptionMsgChecker(String sql, String exceptionMsg) {
try {
// query pinot
- List<Object[]> resultRows = queryRunner(sql, null);
- Assert.fail(
- "Expected error with message '" + exceptionMsg + "'. But instead
rows were returned: " + resultRows.stream()
- .map(Arrays::toString).collect(Collectors.joining(",\n")));
+ ResultTable resultTable = queryRunner(sql, null);
+ Assert.fail("Expected error with message '" + exceptionMsg + "'. 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:
diff --git
a/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTestBase.java
b/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTestBase.java
index 0c422f3962..b9ee6abfbb 100644
---
a/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTestBase.java
+++
b/pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTestBase.java
@@ -40,10 +40,13 @@ import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;
+import javax.annotation.Nullable;
import org.apache.commons.codec.DecoderException;
import org.apache.commons.codec.binary.Hex;
import org.apache.pinot.common.exception.QueryException;
import org.apache.pinot.common.response.broker.ResultTable;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
import org.apache.pinot.common.utils.config.QueryOptionsUtils;
import org.apache.pinot.core.query.reduce.ExecutionStatsAggregator;
import org.apache.pinot.query.QueryEnvironment;
@@ -60,14 +63,17 @@ import
org.apache.pinot.query.service.dispatch.QueryDispatcher;
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.ByteArray;
import org.apache.pinot.spi.utils.BytesUtils;
import org.apache.pinot.spi.utils.CommonConstants;
import org.apache.pinot.spi.utils.StringUtil;
import org.apache.pinot.sql.parsers.CalciteSqlParser;
import org.apache.pinot.sql.parsers.SqlNodeAndOptions;
import org.h2.jdbc.JdbcArray;
-import org.testng.Assert;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
public abstract class QueryRunnerTestBase extends QueryTestSet {
@@ -95,7 +101,7 @@ public abstract class QueryRunnerTestBase extends
QueryTestSet {
* Dispatch query to each pinot-server. The logic should mimic
QueryDispatcher.submit() but does not actually make
* ser/de dispatches.
*/
- protected List<Object[]> queryRunner(String sql, Map<Integer,
ExecutionStatsAggregator> executionStatsAggregatorMap) {
+ protected ResultTable queryRunner(String sql, Map<Integer,
ExecutionStatsAggregator> executionStatsAggregatorMap) {
long requestId = REQUEST_ID_GEN.getAndIncrement();
SqlNodeAndOptions sqlNodeAndOptions =
CalciteSqlParser.compileToSqlNodeAndOptions(sql);
QueryEnvironment.QueryPlannerResult queryPlannerResult =
@@ -140,10 +146,8 @@ public abstract class QueryRunnerTestBase extends
QueryTestSet {
}
}
// exception will be propagated through for assert purpose on runtime error
- ResultTable resultTable =
- QueryDispatcher.runReducer(requestId, dispatchableSubPlan, timeoutMs,
Collections.emptyMap(),
- executionStatsAggregatorMap, _mailboxService);
- return resultTable.getRows();
+ return QueryDispatcher.runReducer(requestId, dispatchableSubPlan,
timeoutMs, Collections.emptyMap(),
+ executionStatsAggregatorMap, _mailboxService);
}
protected List<CompletableFuture<?>>
processDistributedStagePlans(DispatchableSubPlan dispatchableSubPlan,
@@ -193,167 +197,198 @@ public abstract class QueryRunnerTestBase extends
QueryTestSet {
return result;
}
- protected void compareRowEquals(List<Object[]> resultRows, List<Object[]>
expectedRows) {
- compareRowEquals(resultRows, expectedRows, false);
+ protected void compareRowEquals(ResultTable resultTable, List<Object[]>
expectedRows) {
+ compareRowEquals(resultTable, expectedRows, false);
}
- protected void compareRowEquals(List<Object[]> resultRows, List<Object[]>
expectedRows,
- boolean keepOutputRowsInOrder) {
- 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"))));
+ protected void compareRowEquals(ResultTable resultTable, List<Object[]>
expectedRows, boolean keepOutputRowsInOrder) {
+ List<Object[]> resultRows = resultTable.getRows();
+ int numRows = resultRows.size();
+ assertEquals(numRows, 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) {
- return 0;
- } else if (l == null) {
- return -1;
- } else if (r == null) {
- return 1;
+ DataSchema dataSchema = resultTable.getDataSchema();
+ resultRows.forEach(row -> canonicalizeRow(dataSchema, row));
+ expectedRows.forEach(row -> canonicalizeRow(dataSchema, row));
+ if (!keepOutputRowsInOrder) {
+ sortRows(resultRows);
+ sortRows(expectedRows);
+ }
+ for (int i = 0; i < numRows; i++) {
+ Object[] resultRow = resultRows.get(i);
+ Object[] expectedRow = expectedRows.get(i);
+ assertEquals(resultRow.length, expectedRow.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++) {
+ assertTrue(typeCompatibleFuzzyEquals(dataSchema.getColumnDataType(j),
resultRow[j], expectedRow[j]),
+ "Not match at (" + i + "," + j + ")! Expected: " +
Arrays.toString(expectedRow) + " Actual: "
+ + Arrays.toString(resultRow));
}
- if (l instanceof Integer) {
- return Integer.compare((Integer) l, ((Number) r).intValue());
- } else if (l instanceof Long) {
- return Long.compare((Long) l, ((Number) r).longValue());
- } else if (l instanceof Float) {
- float lf = (Float) l;
- float rf = ((Number) r).floatValue();
- if (DoubleMath.fuzzyEquals(lf, rf, DOUBLE_CMP_EPSILON)) {
- return 0;
- }
- float maxf = Math.max(Math.abs(lf), Math.abs(rf));
- if (DoubleMath.fuzzyEquals(lf / maxf, rf / maxf, DOUBLE_CMP_EPSILON)) {
- return 0;
- }
- return Float.compare(lf, rf);
- } else if (l instanceof Double) {
- double ld = (Double) l;
- double rd = ((Number) r).doubleValue();
- if (DoubleMath.fuzzyEquals(ld, rd, DOUBLE_CMP_EPSILON)) {
- return 0;
- }
- double maxd = Math.max(Math.abs(ld), Math.abs(rd));
- if (DoubleMath.fuzzyEquals(ld / maxd, rd / maxd, DOUBLE_CMP_EPSILON)) {
- return 0;
- }
- return Double.compare(ld, rd);
- } else if (l instanceof String) {
- if (r instanceof byte[]) {
- return ((String) l).compareTo(BytesUtils.toHexString((byte[]) r));
- } else if (r instanceof Timestamp) {
- return ((String) l).compareTo((r).toString());
- }
- 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));
+ }
+ }
+
+ protected static void canonicalizeRow(DataSchema dataSchema, Object[] row) {
+ for (int i = 0; i < row.length; i++) {
+ row[i] = canonicalizeValue(dataSchema.getColumnDataType(i), row[i]);
+ }
+ }
+
+ protected static Object canonicalizeValue(ColumnDataType columnDataType,
Object value) {
+ if (value == null) {
+ return null;
+ }
+ switch (columnDataType) {
+ case INT:
+ return ((Number) value).intValue();
+ case LONG:
+ return ((Number) value).longValue();
+ case FLOAT:
+ return ((Number) value).floatValue();
+ case DOUBLE:
+ return ((Number) value).doubleValue();
+ case BIG_DECIMAL:
+ if (value instanceof String) {
+ return new BigDecimal((String) value);
}
- } else if (l instanceof byte[]) {
- if (r instanceof byte[]) {
- return ByteArray.compare((byte[]) l, (byte[]) r);
- } else {
- return ByteArray.compare((byte[]) l, ((ByteArray) r).getBytes());
+ assertTrue(value instanceof BigDecimal, "Got unexpected value type: "
+ value.getClass()
+ + " for BIG_DECIMAL column, expected: String or BigDecimal");
+ return value;
+ case BOOLEAN:
+ assertTrue(value instanceof Boolean,
+ "Got unexpected value type: " + value.getClass() + " for BOOLEAN
column, expected: Boolean");
+ return value;
+ case TIMESTAMP:
+ if (value instanceof String) {
+ return Timestamp.valueOf((String) value);
}
- } else if (l instanceof ByteArray) {
- if (r instanceof ByteArray) {
- return ((ByteArray) l).compareTo((ByteArray) r);
- } else {
- return ByteArray.compare(((ByteArray) l).getBytes(), (byte[]) r);
+ assertTrue(value instanceof Timestamp,
+ "Got unexpected value type: " + value.getClass() + " for TIMESTAMP
column, expected: String or Timestamp");
+ return value;
+ case STRING:
+ assertTrue(value instanceof String,
+ "Got unexpected value type: " + value.getClass() + " for STRING
column, expected: String");
+ return value;
+ case BYTES:
+ if (value instanceof byte[]) {
+ return BytesUtils.toHexString((byte[]) value);
}
- } else if (l instanceof Timestamp) {
- return ((Timestamp) l).compareTo((Timestamp) r);
- } else if (l instanceof int[]) {
- int[] larray = (int[]) l;
- try {
- if (r instanceof JdbcArray) {
- Object[] rarray = (Object[]) ((JdbcArray) r).getArray();
- for (int idx = 0; idx < larray.length; idx++) {
- Number relement = (Number) rarray[idx];
- if (larray[idx] != relement.intValue()) {
- return -1;
- }
- }
- } else {
- int[] rarray = (int[]) r;
- for (int idx = 0; idx < larray.length; idx++) {
- if (larray[idx] != rarray[idx]) {
- return -1;
- }
+ assertTrue(value instanceof String,
+ "Got unexpected value type: " + value.getClass() + " for BYTES
column, expected: String or byte[]");
+ return value;
+ case INT_ARRAY:
+ if (value instanceof JdbcArray) {
+ try {
+ Object[] array = (Object[]) ((JdbcArray) value).getArray();
+ int[] intArray = new int[array.length];
+ for (int i = 0; i < array.length; i++) {
+ intArray[i] = ((Number) array[i]).intValue();
}
+ return intArray;
+ } catch (SQLException e) {
+ throw new RuntimeException(e);
}
- } catch (SQLException e) {
- throw new RuntimeException(e);
}
- return 0;
- } else if (l instanceof String[]) {
- String[] larray = (String[]) l;
- try {
- if (r instanceof JdbcArray) {
- Object[] rarray = (Object[]) ((JdbcArray) r).getArray();
- for (int idx = 0; idx < larray.length; idx++) {
- if (!larray[idx].equals(rarray[idx])) {
- return -1;
- }
- }
- } else {
- String[] rarray = (r instanceof List) ? ((List<String>)
r).toArray(new String[0]) : (String[]) r;
- for (int idx = 0; idx < larray.length; idx++) {
- if (!larray[idx].equals(rarray[idx])) {
- return -1;
- }
- }
- }
- } catch (SQLException e) {
- throw new RuntimeException(e);
+ assertTrue(value instanceof int[],
+ "Got unexpected value type: " + value.getClass() + " for INT_ARRAY
column, expected: int[] or JdbcArray");
+ return value;
+ case STRING_ARRAY:
+ if (value instanceof List) {
+ return ((List) value).toArray(new String[0]);
}
- return 0;
- } else if (l instanceof JdbcArray) {
- try {
- Object[] larray = (Object[]) ((JdbcArray) l).getArray();
- Object[] rarray = (Object[]) ((JdbcArray) r).getArray();
- for (int idx = 0; idx < larray.length; idx++) {
- if (!larray[idx].equals(rarray[idx])) {
- return -1;
+ if (value instanceof JdbcArray) {
+ try {
+ Object[] array = (Object[]) ((JdbcArray) value).getArray();
+ String[] stringArray = new String[array.length];
+ for (int i = 0; i < array.length; i++) {
+ stringArray[i] = (String) array[i];
}
+ return stringArray;
+ } catch (SQLException e) {
+ throw new RuntimeException(e);
}
- } catch (SQLException e) {
- throw new RuntimeException(e);
}
+ assertTrue(value instanceof String[], "Got unexpected value type: " +
value.getClass()
+ + " for STRING_ARRAY column, expected: String[], List or
JdbcArray");
+ return value;
+ default:
+ throw new UnsupportedOperationException("Unsupported ColumnDataType: "
+ columnDataType);
+ }
+ }
+
+ protected static void sortRows(List<Object[]> rows) {
+ Comparator<Object> valueComparator = (v1, v2) -> {
+ if (v1 == null && v2 == null) {
return 0;
- } else {
- throw new RuntimeException("non supported type " + l.getClass());
+ } else if (v1 == null) {
+ return -1;
+ } else if (v2 == null) {
+ return 1;
}
+ if (v1 instanceof Comparable) {
+ return ((Comparable) v1).compareTo(v2);
+ }
+ if (v1 instanceof int[]) {
+ return Arrays.compare((int[]) v1, (int[]) v2);
+ }
+ if (v1 instanceof String[]) {
+ return Arrays.compare((String[]) v1, (String[]) v2);
+ }
+ throw new UnsupportedOperationException("Unsupported class: " +
v1.getClass());
};
- Comparator<Object[]> rowComp = (l, r) -> {
- int cmp = 0;
- for (int i = 0; i < l.length; i++) {
- cmp = valueComp.compare(l[i], r[i]);
+ rows.sort((r1, r2) -> {
+ for (int i = 0; i < r1.length; i++) {
+ int cmp = valueComparator.compare(r1[i], r2[i]);
if (cmp != 0) {
return cmp;
}
}
return 0;
- };
- if (!keepOutputRowsInOrder) {
- resultRows.sort(rowComp);
- expectedRows.sort(rowComp);
+ });
+ }
+
+ protected static boolean typeCompatibleFuzzyEquals(ColumnDataType
columnDataType, @Nullable Object actual,
+ @Nullable Object expected) {
+ if (actual == null || expected == null) {
+ return actual == expected;
}
- 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) + " Actual: "
- + Arrays.toString(resultRow));
- }
+
+ switch (columnDataType) {
+ case INT:
+ return (int) actual == ((Number) expected).intValue();
+ case LONG:
+ return (long) actual == ((Number) expected).longValue();
+ case FLOAT:
+ float actualFloat = (float) actual;
+ float expectedFloat = ((Number) expected).floatValue();
+ if (DoubleMath.fuzzyEquals(actualFloat, expectedFloat,
DOUBLE_CMP_EPSILON)) {
+ return true;
+ }
+ float maxFloat = Math.max(Math.abs(actualFloat),
Math.abs(expectedFloat));
+ return DoubleMath.fuzzyEquals(actualFloat / maxFloat, expectedFloat /
maxFloat, DOUBLE_CMP_EPSILON);
+ case DOUBLE:
+ double actualDouble = (double) actual;
+ double expectedDouble = ((Number) expected).doubleValue();
+ if (DoubleMath.fuzzyEquals(actualDouble, expectedDouble,
DOUBLE_CMP_EPSILON)) {
+ return true;
+ }
+ double maxDouble = Math.max(Math.abs(actualDouble),
Math.abs(expectedDouble));
+ return DoubleMath.fuzzyEquals(actualDouble / maxDouble, expectedDouble
/ maxDouble, DOUBLE_CMP_EPSILON);
+ case BIG_DECIMAL:
+ // Use compare to handle different scale
+ return ((BigDecimal) actual).compareTo((BigDecimal) expected) == 0;
+ case BOOLEAN:
+ case TIMESTAMP:
+ case STRING:
+ case BYTES:
+ return actual.equals(expected);
+ case INT_ARRAY:
+ return Arrays.equals((int[]) actual, (int[]) expected);
+ case STRING_ARRAY:
+ return Arrays.equals((String[]) actual, (String[]) expected);
+ default:
+ throw new UnsupportedOperationException("Unsupported ColumnDataType: "
+ columnDataType);
}
}
@@ -398,13 +433,13 @@ public abstract class QueryRunnerTestBase extends
QueryTestSet {
protected Connection _h2Connection;
protected Connection getH2Connection() {
- Assert.assertNotNull(_h2Connection, "H2 Connection has not been
initialized");
+ assertNotNull(_h2Connection, "H2 Connection has not been initialized");
return _h2Connection;
}
protected void setH2Connection()
throws Exception {
- Assert.assertNull(_h2Connection);
+ assertNull(_h2Connection);
Class.forName("org.h2.Driver");
_h2Connection = DriverManager.getConnection("jdbc:h2:mem:");
}
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 16599c2bd4..1f48a2827a 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
@@ -26,7 +26,6 @@ import java.io.InputStream;
import java.io.InputStreamReader;
import java.net.URL;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -37,12 +36,12 @@ import java.util.Set;
import java.util.TimeZone;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
-import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.pinot.common.datatable.DataTable;
import org.apache.pinot.common.response.broker.BrokerResponseNativeV2;
import org.apache.pinot.common.response.broker.BrokerResponseStats;
+import org.apache.pinot.common.response.broker.ResultTable;
import org.apache.pinot.core.common.datatable.DataTableBuilderFactory;
import org.apache.pinot.core.query.reduce.ExecutionStatsAggregator;
import org.apache.pinot.query.QueryEnvironmentTestBase;
@@ -57,6 +56,7 @@ import org.apache.pinot.spi.data.readers.GenericRow;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.utils.BooleanUtils;
import org.apache.pinot.spi.utils.CommonConstants;
+import org.apache.pinot.spi.utils.JsonUtils;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
@@ -254,9 +254,9 @@ public class ResourceBasedQueriesTest extends
QueryRunnerTestBase {
boolean keepOutputRowOrder)
throws Exception {
// query pinot
- runQuery(sql, expect, null).ifPresent(rows -> {
+ runQuery(sql, expect, null).ifPresent(resultTable -> {
try {
- compareRowEquals(rows, queryH2(h2Sql), keepOutputRowOrder);
+ compareRowEquals(resultTable, queryH2(h2Sql), keepOutputRowOrder);
} catch (Exception e) {
Assert.fail(e.getMessage(), e);
}
@@ -267,7 +267,8 @@ public class ResourceBasedQueriesTest extends
QueryRunnerTestBase {
public void testQueryTestCasesWithOutput(String testCaseName, boolean
isIgnored, String sql, String h2Sql,
List<Object[]> expectedRows, String expect, boolean keepOutputRowOrder)
throws Exception {
- runQuery(sql, expect, null).ifPresent(rows -> compareRowEquals(rows,
expectedRows, keepOutputRowOrder));
+ runQuery(sql, expect, null).ifPresent(
+ resultTable -> compareRowEquals(resultTable, expectedRows,
keepOutputRowOrder));
}
@Test(dataProvider = "testResourceQueryTestCaseProviderWithMetadata")
@@ -275,7 +276,7 @@ public class ResourceBasedQueriesTest extends
QueryRunnerTestBase {
String expect, int numSegments)
throws Exception {
Map<Integer, ExecutionStatsAggregator> executionStatsAggregatorMap = new
HashMap<>();
- runQuery(sql, expect, executionStatsAggregatorMap).ifPresent(rows -> {
+ runQuery(sql, expect, executionStatsAggregatorMap).ifPresent(resultTable
-> {
BrokerResponseNativeV2 brokerResponseNative = new
BrokerResponseNativeV2();
executionStatsAggregatorMap.get(0).setStats(brokerResponseNative);
Assert.assertFalse(executionStatsAggregatorMap.isEmpty());
@@ -325,17 +326,15 @@ public class ResourceBasedQueriesTest extends
QueryRunnerTestBase {
});
}
- private Optional<List<Object[]>> runQuery(String sql, final String except,
- Map<Integer, ExecutionStatsAggregator> executionStatsAggregatorMap) {
+ private Optional<ResultTable> runQuery(String sql, final String except,
+ Map<Integer, ExecutionStatsAggregator> executionStatsAggregatorMap)
+ throws Exception {
try {
// query pinot
- List<Object[]> resultRows = queryRunner(sql,
executionStatsAggregatorMap);
-
- 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);
+ ResultTable resultTable = queryRunner(sql, executionStatsAggregatorMap);
+ Assert.assertNull(except, "Expected error with message '" + except + "'.
But instead rows were returned: "
+ + JsonUtils.objectToPrettyString(resultTable));
+ return Optional.of(resultTable);
} catch (Exception e) {
if (except == null) {
throw e;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
