soumyava commented on code in PR #15086:
URL: https://github.com/apache/druid/pull/15086#discussion_r1353981372


##########
sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java:
##########
@@ -1055,6 +1064,115 @@ public Map<String, Object> baseQueryContext()
     }
   }
 
+  public enum ResultMatchMode
+  {
+    EQUALS {
+      @Override
+      void validate(int row, int column, ValueType type, Object expectedCell, 
Object resultCell)
+      {
+        assertEquals(
+            mismatchMessage(row, column),
+            expectedCell,
+            resultCell);
+      }
+    },
+    RELAX_NULLS {
+      @Override
+      void validate(int row, int column, ValueType type, Object expectedCell, 
Object resultCell)
+      {
+        if (expectedCell == null) {
+          if (resultCell == null) {
+            return;
+          }
+          expectedCell = NullHandling.defaultValueForType(type);
+        }
+        EQUALS.validate(row, column, type, expectedCell, resultCell);
+      }
+    },
+    EQUALS_EPS {
+      @Override
+      void validate(int row, int column, ValueType type, Object expectedCell, 
Object resultCell)
+      {
+        if (expectedCell instanceof Float) {
+          assertEquals(
+              mismatchMessage(row, column),
+              (Float) expectedCell,
+              (Float) resultCell,
+              1e-5);

Review Comment:
   we should make this a static variable and set it to 1e-5. Also is there any 
rationale behind chosing this value ?



##########
sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java:
##########
@@ -1055,6 +1064,115 @@ public Map<String, Object> baseQueryContext()
     }
   }
 
+  public enum ResultMatchMode
+  {
+    EQUALS {
+      @Override
+      void validate(int row, int column, ValueType type, Object expectedCell, 
Object resultCell)
+      {
+        assertEquals(
+            mismatchMessage(row, column),
+            expectedCell,
+            resultCell);
+      }
+    },
+    RELAX_NULLS {
+      @Override
+      void validate(int row, int column, ValueType type, Object expectedCell, 
Object resultCell)
+      {
+        if (expectedCell == null) {
+          if (resultCell == null) {
+            return;
+          }
+          expectedCell = NullHandling.defaultValueForType(type);
+        }
+        EQUALS.validate(row, column, type, expectedCell, resultCell);
+      }
+    },
+    EQUALS_EPS {
+      @Override
+      void validate(int row, int column, ValueType type, Object expectedCell, 
Object resultCell)
+      {
+        if (expectedCell instanceof Float) {
+          assertEquals(
+              mismatchMessage(row, column),
+              (Float) expectedCell,
+              (Float) resultCell,
+              1e-5);
+        } else if (expectedCell instanceof Double) {
+          assertEquals(
+              mismatchMessage(row, column),
+              (Double) expectedCell,
+              (Double) resultCell,
+              1e-5);
+        } else {
+          EQUALS.validate(row, column, type, expectedCell, resultCell);
+        }
+      }
+    };
+
+    abstract void validate(int row, int column, ValueType type, Object 
expectedCell, Object resultCell);
+
+    private static String mismatchMessage(int row, int column)
+    {
+      return String.format(Locale.ENGLISH, "column content mismatch at %d,%d", 
row, column);
+    }
+
+  }
+
+  /**
+   * Validates the results with slight loosening in case {@link NullHandling} 
is not sql compatible.
+   *
+   * In case {@link NullHandling#replaceWithDefault()} is true, if the 
expected result is <code>null</code> it accepts
+   * both <code>null</code> and the default value for that column as actual 
result.
+   */
+  public void assertResultsValid(ResultMatchMode matchMode, List<Object[]> 
expected, QueryResults queryResults)
+  {
+    List<Object[]> results = queryResults.results;
+    Assert.assertEquals("Result count mismatch", expected.size(), 
results.size());
+
+    final List<ValueType> types = new ArrayList<>();
+
+    boolean isMSQ = isMSQRowType(queryResults.signature);
+
+    if (!isMSQ) {
+      for (int i = 0; i < queryResults.signature.getColumnNames().size(); i++) 
{
+        Optional<ColumnType> columnType = 
queryResults.signature.getColumnType(i);
+        if (columnType.isPresent()) {
+          types.add(columnType.get().getType());
+        } else {
+          types.add(null);
+        }
+      }
+    }
+
+    int numRows = results.size();
+    for (int row = 0; row < numRows; row++) {
+      Object[] expectedRow = expected.get(row);

Review Comment:
   nit. could make these allocations final



##########
sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java:
##########
@@ -1055,6 +1064,115 @@ public Map<String, Object> baseQueryContext()
     }
   }
 
+  public enum ResultMatchMode
+  {
+    EQUALS {
+      @Override
+      void validate(int row, int column, ValueType type, Object expectedCell, 
Object resultCell)
+      {
+        assertEquals(
+            mismatchMessage(row, column),
+            expectedCell,
+            resultCell);
+      }
+    },
+    RELAX_NULLS {
+      @Override
+      void validate(int row, int column, ValueType type, Object expectedCell, 
Object resultCell)
+      {
+        if (expectedCell == null) {
+          if (resultCell == null) {
+            return;
+          }
+          expectedCell = NullHandling.defaultValueForType(type);
+        }
+        EQUALS.validate(row, column, type, expectedCell, resultCell);
+      }
+    },
+    EQUALS_EPS {
+      @Override
+      void validate(int row, int column, ValueType type, Object expectedCell, 
Object resultCell)
+      {
+        if (expectedCell instanceof Float) {
+          assertEquals(
+              mismatchMessage(row, column),
+              (Float) expectedCell,
+              (Float) resultCell,
+              1e-5);
+        } else if (expectedCell instanceof Double) {
+          assertEquals(
+              mismatchMessage(row, column),
+              (Double) expectedCell,
+              (Double) resultCell,
+              1e-5);

Review Comment:
   same as above



##########
sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java:
##########
@@ -1055,6 +1064,115 @@ public Map<String, Object> baseQueryContext()
     }
   }
 
+  public enum ResultMatchMode
+  {
+    EQUALS {
+      @Override
+      void validate(int row, int column, ValueType type, Object expectedCell, 
Object resultCell)
+      {
+        assertEquals(
+            mismatchMessage(row, column),
+            expectedCell,
+            resultCell);
+      }
+    },
+    RELAX_NULLS {
+      @Override
+      void validate(int row, int column, ValueType type, Object expectedCell, 
Object resultCell)
+      {
+        if (expectedCell == null) {
+          if (resultCell == null) {
+            return;
+          }
+          expectedCell = NullHandling.defaultValueForType(type);
+        }
+        EQUALS.validate(row, column, type, expectedCell, resultCell);
+      }
+    },
+    EQUALS_EPS {
+      @Override
+      void validate(int row, int column, ValueType type, Object expectedCell, 
Object resultCell)
+      {
+        if (expectedCell instanceof Float) {
+          assertEquals(
+              mismatchMessage(row, column),
+              (Float) expectedCell,
+              (Float) resultCell,
+              1e-5);
+        } else if (expectedCell instanceof Double) {
+          assertEquals(
+              mismatchMessage(row, column),
+              (Double) expectedCell,
+              (Double) resultCell,
+              1e-5);
+        } else {
+          EQUALS.validate(row, column, type, expectedCell, resultCell);
+        }
+      }
+    };
+
+    abstract void validate(int row, int column, ValueType type, Object 
expectedCell, Object resultCell);
+
+    private static String mismatchMessage(int row, int column)
+    {
+      return String.format(Locale.ENGLISH, "column content mismatch at %d,%d", 
row, column);

Review Comment:
   StringUtils.format might be a better choice



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to