walterddr commented on code in PR #11716:
URL: https://github.com/apache/pinot/pull/11716#discussion_r1342896741


##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/queries/QueryRunnerTestBase.java:
##########
@@ -193,167 +197,198 @@ protected List<Object[]> queryH2(String sql)
     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) {

Review Comment:
   we might need to expose sortRows and typeCompatibleFuzzyCompare to 
integration test b/c some tests are failing b/c of the ordering doesn't match 
between H2 and Pinot



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to