apache · Jackie-Jiang · Oct 2, 2023 · Sep 29, 2023
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
@@ -449,10 +449,13 @@ public Serializable convert(Object value) {
     }
 
     /**
-     * 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 Serializable convertAndFormat(Object value) {
         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
@@ -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 void testColumnDataType() {
     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
@@ -138,9 +138,7 @@ private void setUp(TableConfig tableConfig)
   @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 void testQueriesWithNoDictColumn()
       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 void testQueries() {
         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 void testQueries() {
           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,16 +224,16 @@ public void testQueries() {
         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.
       assertNull(rows.get(rows.size() - 1)[0]);
     }
     {
       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 void testQueries() {
         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 void testQueries() {
       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 void testQueries() {
         }
         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 void testQueries() {
           // 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 void testQueries() {
     {
       // 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 void testQueries() {
           // 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 void testQueries() {
       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 void testQueries() {
     }
     {
       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
@@ -1251,7 +1251,7 @@ private void testDistinctInterSegmentHelper(String[] queries) {
         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 @@ private void testDistinctInterSegmentHelper(String[] queries) {
       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 @@ private void testDistinctInterSegmentHelper(String[] queries) {
       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
@@ -257,8 +257,8 @@ public void testAggregationInterSegment() {
     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/...t-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTest.java b/...t-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTest.java
@@ -24,7 +24,7 @@
 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.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 void tearDown() {
    */
   @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 void testSqlWithFinalRowCountChecker(String sql, int expectedRows) {
   @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 void testSqlWithH2Checker(String sql)
   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: