You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/10/25 20:46:33 UTC

[GitHub] [pinot] walterddr opened a new pull request, #9657: adding null integration test

walterddr opened a new pull request, #9657:
URL: https://github.com/apache/pinot/pull/9657

   - null value test for more types
   - add null integration test


-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] walterddr commented on a diff in pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9657:
URL: https://github.com/apache/pinot/pull/9657#discussion_r1005157062


##########
pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java:
##########
@@ -156,131 +179,186 @@ private void setUp(TableConfig tableConfig, DataType dataType)
     _indexSegments = Arrays.asList(immutableSegment, immutableSegment);
   }
 
-  @Test
-  public void testQueriesWithDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 1)
-  public void testQueriesWithNoDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithNoDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 2)
-  public void testQueriesWithDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 3)
-  public void testQueriesWithNoDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithNoDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 4)
-  public void testQueriesWithDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "numberColumnTypeProvider")
+  public Object[][] provideNumberColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), true},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), true},
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), false},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), false},
+    };
   }
 
-  @Test(priority = 5)
-  public void testQueriesWithNoDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    List<String> noDictionaryColumns = new ArrayList<String>();
-    noDictionaryColumns.add(COLUMN_NAME);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .setNoDictionaryColumns(noDictionaryColumns)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "otherColumnTypeProvider")
+  public Object[][] provideOtherColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), true},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), false},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), false},

Review Comment:
   Ah I see thanks for the info. I wasn't aware of the other tests



-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] nizarhejazi commented on a diff in pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
nizarhejazi commented on code in PR #9657:
URL: https://github.com/apache/pinot/pull/9657#discussion_r1005131171


##########
pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java:
##########
@@ -156,131 +179,186 @@ private void setUp(TableConfig tableConfig, DataType dataType)
     _indexSegments = Arrays.asList(immutableSegment, immutableSegment);
   }
 
-  @Test
-  public void testQueriesWithDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 1)
-  public void testQueriesWithNoDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithNoDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 2)
-  public void testQueriesWithDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 3)
-  public void testQueriesWithNoDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithNoDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 4)
-  public void testQueriesWithDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "numberColumnTypeProvider")
+  public Object[][] provideNumberColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), true},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), true},
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), false},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), false},
+    };
   }
 
-  @Test(priority = 5)
-  public void testQueriesWithNoDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    List<String> noDictionaryColumns = new ArrayList<String>();
-    noDictionaryColumns.add(COLUMN_NAME);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .setNoDictionaryColumns(noDictionaryColumns)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "otherColumnTypeProvider")
+  public Object[][] provideOtherColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), true},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), false},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), false},
+    };
   }
 
-  @Test(priority = 6)
-  public void testQueriesWithDictDoubleColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
-  }
+  public void testQueriesGeneric(ColumnDataType dataType, boolean nullValuesExist) {
+    DataTableBuilderFactory.setDataTableVersion(DataTableFactory.VERSION_4);
+    Map<String, String> queryOptions = new HashMap<>();
+    queryOptions.put("enableNullHandling", "true");
+    {
+      String query = "SELECT * FROM testTable";
+      BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+      ResultTable resultTable = brokerResponse.getResultTable();
+      DataSchema dataSchema = resultTable.getDataSchema();
+      assertEquals(dataSchema, new DataSchema(new String[]{COLUMN_NAME, KEY_COLUMN},
+          new ColumnDataType[]{dataType, ColumnDataType.INT}));
+      List<Object[]> rows = resultTable.getRows();
+      assertEquals(rows.size(), 10);
+      for (int i = 0; i < 10; i++) {
+        Object[] row = rows.get(i);
+        assertEquals(row.length, 2);
+        if (row[0] != null) {
+          assertNotNull(row[1]);
+        } else {
+          assertNull(row[1]);
+        }
+      }
+    }
+    {
+      // This test case was added to validate path-code for distinct w/o order by.
+      int limit = 40;
+      String query = String.format("SELECT DISTINCT %s FROM testTable LIMIT %d", COLUMN_NAME, limit);
+      BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+      ResultTable resultTable = brokerResponse.getResultTable();
+      DataSchema dataSchema = resultTable.getDataSchema();
+      assertEquals(dataSchema,
+          new DataSchema(new String[]{COLUMN_NAME}, new ColumnDataType[]{dataType}));
+      List<Object[]> rows = resultTable.getRows();
+      // Boolean there's only true or false.
+      if (dataType != ColumnDataType.BOOLEAN) {
+        assertEquals(rows.size(), limit);
+      }
+    }
+    {
+      String query = String.format(
+          "SELECT COUNT(*) AS count, %s FROM testTable GROUP BY %s ORDER BY %s DESC LIMIT 1000", COLUMN_NAME,
+          COLUMN_NAME, COLUMN_NAME);
+      BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+      ResultTable resultTable = brokerResponse.getResultTable();
+      DataSchema dataSchema = resultTable.getDataSchema();
+      assertEquals(dataSchema, new DataSchema(new String[]{"count", COLUMN_NAME},
+          new ColumnDataType[]{ColumnDataType.LONG, dataType}));
+      List<Object[]> rows = resultTable.getRows();
 
-  @Test(priority = 7)
-  public void testQueriesWithNoDictDoubleColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    List<String> noDictionaryColumns = new ArrayList<String>();
-    noDictionaryColumns.add(COLUMN_NAME);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .setNoDictionaryColumns(noDictionaryColumns)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+      if (dataType != ColumnDataType.BOOLEAN) {
+        int rowsCount = nullValuesExist ? 501 : 500;
+        assertEquals(rows.size(), rowsCount);
+        int i = 0;
+        for (int index = 0; index < 500; index++) {
+          Object[] row = rows.get(index);
+          assertEquals(row.length, 2);
+          if ((NUM_RECORDS - i - 1) % 2 == 1) {
+            // Null values are inserted at: index % 2 == 1. All null values are grouped into a single null.
+            i++;
+          }
+          assertEquals(row[0], 4L);
+          i++;
+        }
+      }
+      // The default null ordering is 'NULLS LAST'.
+      if (nullValuesExist) {
+        Object[] row = rows.get(rows.size() - 1);
+        assertEquals(row[0], 2000L);
+        assertNull(row[1]);
+      }
+    }
+    // TODO fix String null value handling
+    if (dataType != ColumnDataType.STRING) {

Review Comment:
   @walterddr There is a reason this query does not work w/ String, right? We cannot pass String column to aggregation functions (Count, Min, Max, Avg, etc.) and we cannot use inequality operators w/ Strings.



-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] jackjlli commented on a diff in pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
jackjlli commented on code in PR #9657:
URL: https://github.com/apache/pinot/pull/9657#discussion_r1005045318


##########
pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java:
##########
@@ -317,16 +395,17 @@ public void testQueries(Number baseValue, ColumnDataType dataType, boolean nullV
           //(3 rows)
           assertEquals(row[3], 0L);
         } else if ((int) row[keyColumnIdx] == 1) {
-          assertTrue(Math.abs(((Double) row[0]) - 4 * _sumKey1) < 1e-1);
-          assertTrue(Math.abs(((Double) row[1]) - baseValue.doubleValue()) < 1e-1);
+          assertTrue(Math.abs(((Double) row[0]) - 4 * _sumKey1) < PRECISION);

Review Comment:
   I saw the value of `PRECISION` constant is `1` in this PR, isn't it greater than `1e-1`?



-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] nizarhejazi commented on a diff in pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
nizarhejazi commented on code in PR #9657:
URL: https://github.com/apache/pinot/pull/9657#discussion_r1005131171


##########
pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java:
##########
@@ -156,131 +179,186 @@ private void setUp(TableConfig tableConfig, DataType dataType)
     _indexSegments = Arrays.asList(immutableSegment, immutableSegment);
   }
 
-  @Test
-  public void testQueriesWithDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 1)
-  public void testQueriesWithNoDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithNoDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 2)
-  public void testQueriesWithDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 3)
-  public void testQueriesWithNoDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithNoDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 4)
-  public void testQueriesWithDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "numberColumnTypeProvider")
+  public Object[][] provideNumberColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), true},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), true},
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), false},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), false},
+    };
   }
 
-  @Test(priority = 5)
-  public void testQueriesWithNoDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    List<String> noDictionaryColumns = new ArrayList<String>();
-    noDictionaryColumns.add(COLUMN_NAME);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .setNoDictionaryColumns(noDictionaryColumns)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "otherColumnTypeProvider")
+  public Object[][] provideOtherColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), true},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), false},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), false},
+    };
   }
 
-  @Test(priority = 6)
-  public void testQueriesWithDictDoubleColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
-  }
+  public void testQueriesGeneric(ColumnDataType dataType, boolean nullValuesExist) {
+    DataTableBuilderFactory.setDataTableVersion(DataTableFactory.VERSION_4);
+    Map<String, String> queryOptions = new HashMap<>();
+    queryOptions.put("enableNullHandling", "true");
+    {
+      String query = "SELECT * FROM testTable";
+      BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+      ResultTable resultTable = brokerResponse.getResultTable();
+      DataSchema dataSchema = resultTable.getDataSchema();
+      assertEquals(dataSchema, new DataSchema(new String[]{COLUMN_NAME, KEY_COLUMN},
+          new ColumnDataType[]{dataType, ColumnDataType.INT}));
+      List<Object[]> rows = resultTable.getRows();
+      assertEquals(rows.size(), 10);
+      for (int i = 0; i < 10; i++) {
+        Object[] row = rows.get(i);
+        assertEquals(row.length, 2);
+        if (row[0] != null) {
+          assertNotNull(row[1]);
+        } else {
+          assertNull(row[1]);
+        }
+      }
+    }
+    {
+      // This test case was added to validate path-code for distinct w/o order by.
+      int limit = 40;
+      String query = String.format("SELECT DISTINCT %s FROM testTable LIMIT %d", COLUMN_NAME, limit);
+      BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+      ResultTable resultTable = brokerResponse.getResultTable();
+      DataSchema dataSchema = resultTable.getDataSchema();
+      assertEquals(dataSchema,
+          new DataSchema(new String[]{COLUMN_NAME}, new ColumnDataType[]{dataType}));
+      List<Object[]> rows = resultTable.getRows();
+      // Boolean there's only true or false.
+      if (dataType != ColumnDataType.BOOLEAN) {
+        assertEquals(rows.size(), limit);
+      }
+    }
+    {
+      String query = String.format(
+          "SELECT COUNT(*) AS count, %s FROM testTable GROUP BY %s ORDER BY %s DESC LIMIT 1000", COLUMN_NAME,
+          COLUMN_NAME, COLUMN_NAME);
+      BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+      ResultTable resultTable = brokerResponse.getResultTable();
+      DataSchema dataSchema = resultTable.getDataSchema();
+      assertEquals(dataSchema, new DataSchema(new String[]{"count", COLUMN_NAME},
+          new ColumnDataType[]{ColumnDataType.LONG, dataType}));
+      List<Object[]> rows = resultTable.getRows();
 
-  @Test(priority = 7)
-  public void testQueriesWithNoDictDoubleColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    List<String> noDictionaryColumns = new ArrayList<String>();
-    noDictionaryColumns.add(COLUMN_NAME);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .setNoDictionaryColumns(noDictionaryColumns)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+      if (dataType != ColumnDataType.BOOLEAN) {
+        int rowsCount = nullValuesExist ? 501 : 500;
+        assertEquals(rows.size(), rowsCount);
+        int i = 0;
+        for (int index = 0; index < 500; index++) {
+          Object[] row = rows.get(index);
+          assertEquals(row.length, 2);
+          if ((NUM_RECORDS - i - 1) % 2 == 1) {
+            // Null values are inserted at: index % 2 == 1. All null values are grouped into a single null.
+            i++;
+          }
+          assertEquals(row[0], 4L);
+          i++;
+        }
+      }
+      // The default null ordering is 'NULLS LAST'.
+      if (nullValuesExist) {
+        Object[] row = rows.get(rows.size() - 1);
+        assertEquals(row[0], 2000L);
+        assertNull(row[1]);
+      }
+    }
+    // TODO fix String null value handling
+    if (dataType != ColumnDataType.STRING) {

Review Comment:
   @walterddr There is a reason this query does not work w/ String, right? We cannot pass String column to aggregation functions (Min, Max, Avg, etc.) and we cannot use inequality operators w/ Strings.



-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] walterddr closed pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
walterddr closed pull request #9657: adding null integration test
URL: https://github.com/apache/pinot/pull/9657


-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] nizarhejazi commented on a diff in pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
nizarhejazi commented on code in PR #9657:
URL: https://github.com/apache/pinot/pull/9657#discussion_r1005130406


##########
pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java:
##########
@@ -156,131 +179,186 @@ private void setUp(TableConfig tableConfig, DataType dataType)
     _indexSegments = Arrays.asList(immutableSegment, immutableSegment);
   }
 
-  @Test
-  public void testQueriesWithDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 1)
-  public void testQueriesWithNoDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithNoDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 2)
-  public void testQueriesWithDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 3)
-  public void testQueriesWithNoDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithNoDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 4)
-  public void testQueriesWithDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "numberColumnTypeProvider")
+  public Object[][] provideNumberColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), true},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), true},
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), false},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), false},
+    };
   }
 
-  @Test(priority = 5)
-  public void testQueriesWithNoDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    List<String> noDictionaryColumns = new ArrayList<String>();
-    noDictionaryColumns.add(COLUMN_NAME);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .setNoDictionaryColumns(noDictionaryColumns)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "otherColumnTypeProvider")
+  public Object[][] provideOtherColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), true},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), false},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), false},

Review Comment:
   @walterddr NullHandling support was tested against all data types. Please check the following files:
   
   NullEnabledQueriesTest (Dict and non-dict Float and Double types).
   BooleanNullEnabledQueriesTest (Dict and non-dict Boolean type).
   BigDecimalQueriesTest (Dict and non-dict type).
   AllNullQueriesTest (Dict and non-dict Long, Float, Double, Int, String and BigDecimal types).



-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] codecov-commenter commented on pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9657:
URL: https://github.com/apache/pinot/pull/9657#issuecomment-1291181441

   # [Codecov](https://codecov.io/gh/apache/pinot/pull/9657?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#9657](https://codecov.io/gh/apache/pinot/pull/9657?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2605368) into [master](https://codecov.io/gh/apache/pinot/commit/3de1c54f072a14a981c11eb3a2cea41bec282210?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3de1c54) will **decrease** coverage by `7.30%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #9657      +/-   ##
   ============================================
   - Coverage     68.63%   61.33%   -7.31%     
   - Complexity     4920     5156     +236     
   ============================================
     Files          1947     1933      -14     
     Lines        104168   103900     -268     
     Branches      15796    15770      -26     
   ============================================
   - Hits          71496    63723    -7773     
   - Misses        27556    35365    +7809     
   + Partials       5116     4812     -304     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `25.88% <ø> (+0.01%)` | :arrow_up: |
   | unittests1 | `67.43% <ø> (+0.09%)` | :arrow_up: |
   | unittests2 | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9657?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...pinot/controller/recommender/io/ConfigManager.java](https://codecov.io/gh/apache/pinot/pull/9657/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9pby9Db25maWdNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../org/apache/pinot/client/AggregationResultSet.java](https://codecov.io/gh/apache/pinot/pull/9657/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0FnZ3JlZ2F0aW9uUmVzdWx0U2V0LmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ntroller/recommender/rules/impl/JsonIndexRule.java](https://codecov.io/gh/apache/pinot/pull/9657/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy9pbXBsL0pzb25JbmRleFJ1bGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...routing/adaptiveserverselector/HybridSelector.java](https://codecov.io/gh/apache/pinot/pull/9657/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9hZGFwdGl2ZXNlcnZlcnNlbGVjdG9yL0h5YnJpZFNlbGVjdG9yLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...troller/recommender/io/metadata/FieldMetadata.java](https://codecov.io/gh/apache/pinot/pull/9657/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9pby9tZXRhZGF0YS9GaWVsZE1ldGFkYXRhLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...outing/adaptiveserverselector/LatencySelector.java](https://codecov.io/gh/apache/pinot/pull/9657/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9hZGFwdGl2ZXNlcnZlcnNlbGVjdG9yL0xhdGVuY3lTZWxlY3Rvci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...roller/recommender/rules/impl/BloomFilterRule.java](https://codecov.io/gh/apache/pinot/pull/9657/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy9pbXBsL0Jsb29tRmlsdGVyUnVsZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...oller/api/resources/PinotControllerAppConfigs.java](https://codecov.io/gh/apache/pinot/pull/9657/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90Q29udHJvbGxlckFwcENvbmZpZ3MuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ler/recommender/data/generator/BytesGenerator.java](https://codecov.io/gh/apache/pinot/pull/9657/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9kYXRhL2dlbmVyYXRvci9CeXRlc0dlbmVyYXRvci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...er/recommender/io/metadata/SchemaWithMetaData.java](https://codecov.io/gh/apache/pinot/pull/9657/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9pby9tZXRhZGF0YS9TY2hlbWFXaXRoTWV0YURhdGEuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [292 more](https://codecov.io/gh/apache/pinot/pull/9657/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] walterddr commented on a diff in pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9657:
URL: https://github.com/apache/pinot/pull/9657#discussion_r1005102999


##########
pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java:
##########
@@ -156,131 +179,186 @@ private void setUp(TableConfig tableConfig, DataType dataType)
     _indexSegments = Arrays.asList(immutableSegment, immutableSegment);
   }
 
-  @Test
-  public void testQueriesWithDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 1)
-  public void testQueriesWithNoDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithNoDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 2)
-  public void testQueriesWithDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 3)
-  public void testQueriesWithNoDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithNoDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 4)
-  public void testQueriesWithDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "numberColumnTypeProvider")
+  public Object[][] provideNumberColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), true},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), true},
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), false},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), false},

Review Comment:
   @nizarhejazi i was trying to add additional numeric test types here but the assert logic is a bit hard to generalized. please kindly take a look and see if you have better ideas. 



##########
pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java:
##########
@@ -156,131 +179,186 @@ private void setUp(TableConfig tableConfig, DataType dataType)
     _indexSegments = Arrays.asList(immutableSegment, immutableSegment);
   }
 
-  @Test
-  public void testQueriesWithDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 1)
-  public void testQueriesWithNoDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithNoDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 2)
-  public void testQueriesWithDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 3)
-  public void testQueriesWithNoDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithNoDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 4)
-  public void testQueriesWithDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "numberColumnTypeProvider")
+  public Object[][] provideNumberColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), true},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), true},
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), false},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), false},
+    };
   }
 
-  @Test(priority = 5)
-  public void testQueriesWithNoDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    List<String> noDictionaryColumns = new ArrayList<String>();
-    noDictionaryColumns.add(COLUMN_NAME);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .setNoDictionaryColumns(noDictionaryColumns)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "otherColumnTypeProvider")
+  public Object[][] provideOtherColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), true},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), false},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), false},

Review Comment:
   this list is not at all a full coverage, but good to add STRING BOOLEAN and TIMESTAMP as representatives. can add more later



##########
pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java:
##########
@@ -156,131 +179,186 @@ private void setUp(TableConfig tableConfig, DataType dataType)
     _indexSegments = Arrays.asList(immutableSegment, immutableSegment);
   }
 
-  @Test
-  public void testQueriesWithDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 1)
-  public void testQueriesWithNoDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithNoDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 2)
-  public void testQueriesWithDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 3)
-  public void testQueriesWithNoDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithNoDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 4)
-  public void testQueriesWithDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "numberColumnTypeProvider")
+  public Object[][] provideNumberColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), true},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), true},
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), false},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), false},
+    };
   }
 
-  @Test(priority = 5)
-  public void testQueriesWithNoDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    List<String> noDictionaryColumns = new ArrayList<String>();
-    noDictionaryColumns.add(COLUMN_NAME);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .setNoDictionaryColumns(noDictionaryColumns)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "otherColumnTypeProvider")
+  public Object[][] provideOtherColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), true},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), false},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), false},
+    };
   }
 
-  @Test(priority = 6)
-  public void testQueriesWithDictDoubleColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
-  }
+  public void testQueriesGeneric(ColumnDataType dataType, boolean nullValuesExist) {
+    DataTableBuilderFactory.setDataTableVersion(DataTableFactory.VERSION_4);
+    Map<String, String> queryOptions = new HashMap<>();
+    queryOptions.put("enableNullHandling", "true");
+    {
+      String query = "SELECT * FROM testTable";
+      BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+      ResultTable resultTable = brokerResponse.getResultTable();
+      DataSchema dataSchema = resultTable.getDataSchema();
+      assertEquals(dataSchema, new DataSchema(new String[]{COLUMN_NAME, KEY_COLUMN},
+          new ColumnDataType[]{dataType, ColumnDataType.INT}));
+      List<Object[]> rows = resultTable.getRows();
+      assertEquals(rows.size(), 10);
+      for (int i = 0; i < 10; i++) {
+        Object[] row = rows.get(i);
+        assertEquals(row.length, 2);
+        if (row[0] != null) {
+          assertNotNull(row[1]);
+        } else {
+          assertNull(row[1]);
+        }
+      }
+    }
+    {
+      // This test case was added to validate path-code for distinct w/o order by.
+      int limit = 40;
+      String query = String.format("SELECT DISTINCT %s FROM testTable LIMIT %d", COLUMN_NAME, limit);
+      BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+      ResultTable resultTable = brokerResponse.getResultTable();
+      DataSchema dataSchema = resultTable.getDataSchema();
+      assertEquals(dataSchema,
+          new DataSchema(new String[]{COLUMN_NAME}, new ColumnDataType[]{dataType}));
+      List<Object[]> rows = resultTable.getRows();
+      // Boolean there's only true or false.
+      if (dataType != ColumnDataType.BOOLEAN) {
+        assertEquals(rows.size(), limit);
+      }
+    }
+    {
+      String query = String.format(
+          "SELECT COUNT(*) AS count, %s FROM testTable GROUP BY %s ORDER BY %s DESC LIMIT 1000", COLUMN_NAME,
+          COLUMN_NAME, COLUMN_NAME);
+      BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+      ResultTable resultTable = brokerResponse.getResultTable();
+      DataSchema dataSchema = resultTable.getDataSchema();
+      assertEquals(dataSchema, new DataSchema(new String[]{"count", COLUMN_NAME},
+          new ColumnDataType[]{ColumnDataType.LONG, dataType}));
+      List<Object[]> rows = resultTable.getRows();
 
-  @Test(priority = 7)
-  public void testQueriesWithNoDictDoubleColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    List<String> noDictionaryColumns = new ArrayList<String>();
-    noDictionaryColumns.add(COLUMN_NAME);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .setNoDictionaryColumns(noDictionaryColumns)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+      if (dataType != ColumnDataType.BOOLEAN) {
+        int rowsCount = nullValuesExist ? 501 : 500;
+        assertEquals(rows.size(), rowsCount);
+        int i = 0;
+        for (int index = 0; index < 500; index++) {
+          Object[] row = rows.get(index);
+          assertEquals(row.length, 2);
+          if ((NUM_RECORDS - i - 1) % 2 == 1) {
+            // Null values are inserted at: index % 2 == 1. All null values are grouped into a single null.
+            i++;
+          }
+          assertEquals(row[0], 4L);
+          i++;
+        }
+      }
+      // The default null ordering is 'NULLS LAST'.
+      if (nullValuesExist) {
+        Object[] row = rows.get(rows.size() - 1);
+        assertEquals(row[0], 2000L);
+        assertNull(row[1]);
+      }
+    }
+    // TODO fix String null value handling
+    if (dataType != ColumnDataType.STRING) {

Review Comment:
   looks like this query doesn't work with STRING type. might be we can take another look @nizarhejazi 



-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] jackjlli commented on a diff in pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
jackjlli commented on code in PR #9657:
URL: https://github.com/apache/pinot/pull/9657#discussion_r1005050198


##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java:
##########
@@ -132,6 +132,9 @@ protected void testQuery(String pinotQuery, String h2Query)
   @AfterClass
   public void tearDown()
       throws Exception {
+    // Setting data table version back
+    DataTableBuilderFactory.setDataTableVersion(DataTableBuilderFactory.DEFAULT_VERSION);

Review Comment:
   Do we need to set the following to the `setUp` method of this integration test class as well?
   ```
       // Setting data table version to 4
       DataTableBuilderFactory.setDataTableVersion(DataTableFactory.VERSION_4);
   ```



-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] nizarhejazi commented on pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
nizarhejazi commented on PR #9657:
URL: https://github.com/apache/pinot/pull/9657#issuecomment-1291364114

   @walterddr NullHandling support was tested against all data types. Please check the following files:
   - NullEnabledQueriesTest (Dict and non-dict Float and Double types).
   - BooleanNullEnabledQueriesTest (Dict and non-dict Boolean type).
   - BigDecimalQueriesTest (Dict and non-dict type).
   - AllNullQueriesTest (Dict and non-dict Long, Float, Double, Int, String and BigDecimal types).


-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] walterddr commented on a diff in pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9657:
URL: https://github.com/apache/pinot/pull/9657#discussion_r1005102045


##########
pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java:
##########
@@ -317,16 +395,17 @@ public void testQueries(Number baseValue, ColumnDataType dataType, boolean nullV
           //(3 rows)
           assertEquals(row[3], 0L);
         } else if ((int) row[keyColumnIdx] == 1) {
-          assertTrue(Math.abs(((Double) row[0]) - 4 * _sumKey1) < 1e-1);
-          assertTrue(Math.abs(((Double) row[1]) - baseValue.doubleValue()) < 1e-1);
+          assertTrue(Math.abs(((Double) row[0]) - 4 * _sumKey1) < PRECISION);

Review Comment:
   yeah this was for integer and long in the future. but yes i can make it 1e-1 for now until we add other types



-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] walterddr commented on pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
walterddr commented on PR #9657:
URL: https://github.com/apache/pinot/pull/9657#issuecomment-1291445520

   looks like as @nizarhejazi mentioned the issue is not on null handling (e.g. select out null column) but on function interpretation of null values. closing this and creating a new PR for fixing the issue


-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] walterddr commented on a diff in pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9657:
URL: https://github.com/apache/pinot/pull/9657#discussion_r1005102594


##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java:
##########
@@ -132,6 +132,9 @@ protected void testQuery(String pinotQuery, String h2Query)
   @AfterClass
   public void tearDown()
       throws Exception {
+    // Setting data table version back
+    DataTableBuilderFactory.setDataTableVersion(DataTableBuilderFactory.DEFAULT_VERSION);

Review Comment:
   yeah this is a hotfix, multistage test was already setting it in setup but it wasn't set back in teardown i just happen to notice



-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] nizarhejazi commented on a diff in pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
nizarhejazi commented on code in PR #9657:
URL: https://github.com/apache/pinot/pull/9657#discussion_r1005130406


##########
pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java:
##########
@@ -156,131 +179,186 @@ private void setUp(TableConfig tableConfig, DataType dataType)
     _indexSegments = Arrays.asList(immutableSegment, immutableSegment);
   }
 
-  @Test
-  public void testQueriesWithDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 1)
-  public void testQueriesWithNoDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithNoDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 2)
-  public void testQueriesWithDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 3)
-  public void testQueriesWithNoDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithNoDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 4)
-  public void testQueriesWithDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "numberColumnTypeProvider")
+  public Object[][] provideNumberColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), true},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), true},
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), false},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), false},
+    };
   }
 
-  @Test(priority = 5)
-  public void testQueriesWithNoDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    List<String> noDictionaryColumns = new ArrayList<String>();
-    noDictionaryColumns.add(COLUMN_NAME);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .setNoDictionaryColumns(noDictionaryColumns)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "otherColumnTypeProvider")
+  public Object[][] provideOtherColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), true},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), false},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), false},

Review Comment:
   @walterddr NullHandling support was tested against all data types. Please check the following files:
   
   - NullEnabledQueriesTest (Dict and non-dict Float and Double types).
   - BooleanNullEnabledQueriesTest (Dict and non-dict Boolean type).
   - BigDecimalQueriesTest (Dict and non-dict BigDecimal type).
   - AllNullQueriesTest (Dict and non-dict Long, Float, Double, Int, String and BigDecimal types).



-- 
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-unsubscribe@pinot.apache.org

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


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


[GitHub] [pinot] walterddr commented on a diff in pull request #9657: adding null integration test

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9657:
URL: https://github.com/apache/pinot/pull/9657#discussion_r1005158021


##########
pinot-core/src/test/java/org/apache/pinot/queries/NullEnabledQueriesTest.java:
##########
@@ -156,131 +179,186 @@ private void setUp(TableConfig tableConfig, DataType dataType)
     _indexSegments = Arrays.asList(immutableSegment, immutableSegment);
   }
 
-  @Test
-  public void testQueriesWithDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 1)
-  public void testQueriesWithNoDictFloatColumn()
+  @Test(dataProvider = "numberColumnTypeProvider")
+  public void testQueriesWithNoDictColumn(ColumnDataType columnDataType, Number baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createNumericRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesNumeric(baseValue, columnDataType, generateNulls);
   }
 
-  @Test(priority = 2)
-  public void testQueriesWithDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 3)
-  public void testQueriesWithNoDictDoubleColumn()
+  @Test(dataProvider = "otherColumnTypeProvider")
+  public void testQueriesByTypeWithNoDictColumn(ColumnDataType columnDataType, Object baseValue, boolean generateNulls)
       throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = true;
-    createRecords(baseValue, generateNulls);
+    createOtherRecords(baseValue, columnDataType, generateNulls);
     List<String> noDictionaryColumns = new ArrayList<String>();
     noDictionaryColumns.add(COLUMN_NAME);
     TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
         .setTableName(RAW_TABLE_NAME)
         .setNoDictionaryColumns(noDictionaryColumns)
         .build();
     setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+    testQueriesGeneric(columnDataType, generateNulls);
   }
 
-  @Test(priority = 4)
-  public void testQueriesWithDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "numberColumnTypeProvider")
+  public Object[][] provideNumberColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), true},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), true},
+        new Object[]{ColumnDataType.FLOAT, RANDOM.nextFloat(), false},
+        new Object[]{ColumnDataType.DOUBLE, RANDOM.nextDouble(), false},
+    };
   }
 
-  @Test(priority = 5)
-  public void testQueriesWithNoDictFloatColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.FLOAT;
-    float baseValue = RANDOM.nextFloat();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    List<String> noDictionaryColumns = new ArrayList<String>();
-    noDictionaryColumns.add(COLUMN_NAME);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .setNoDictionaryColumns(noDictionaryColumns)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+  @DataProvider(name = "otherColumnTypeProvider")
+  public Object[][] provideOtherColumnTypeAndRandomVal() {
+    return new Object[][]{
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), true},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), true},
+        new Object[]{ColumnDataType.INT, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.LONG, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.BOOLEAN, RANDOM.nextInt(2), false},
+        new Object[]{ColumnDataType.STRING, RANDOM.nextInt(), false},
+        new Object[]{ColumnDataType.TIMESTAMP, RANDOM.nextInt(), false},
+    };
   }
 
-  @Test(priority = 6)
-  public void testQueriesWithDictDoubleColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
-  }
+  public void testQueriesGeneric(ColumnDataType dataType, boolean nullValuesExist) {
+    DataTableBuilderFactory.setDataTableVersion(DataTableFactory.VERSION_4);
+    Map<String, String> queryOptions = new HashMap<>();
+    queryOptions.put("enableNullHandling", "true");
+    {
+      String query = "SELECT * FROM testTable";
+      BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+      ResultTable resultTable = brokerResponse.getResultTable();
+      DataSchema dataSchema = resultTable.getDataSchema();
+      assertEquals(dataSchema, new DataSchema(new String[]{COLUMN_NAME, KEY_COLUMN},
+          new ColumnDataType[]{dataType, ColumnDataType.INT}));
+      List<Object[]> rows = resultTable.getRows();
+      assertEquals(rows.size(), 10);
+      for (int i = 0; i < 10; i++) {
+        Object[] row = rows.get(i);
+        assertEquals(row.length, 2);
+        if (row[0] != null) {
+          assertNotNull(row[1]);
+        } else {
+          assertNull(row[1]);
+        }
+      }
+    }
+    {
+      // This test case was added to validate path-code for distinct w/o order by.
+      int limit = 40;
+      String query = String.format("SELECT DISTINCT %s FROM testTable LIMIT %d", COLUMN_NAME, limit);
+      BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+      ResultTable resultTable = brokerResponse.getResultTable();
+      DataSchema dataSchema = resultTable.getDataSchema();
+      assertEquals(dataSchema,
+          new DataSchema(new String[]{COLUMN_NAME}, new ColumnDataType[]{dataType}));
+      List<Object[]> rows = resultTable.getRows();
+      // Boolean there's only true or false.
+      if (dataType != ColumnDataType.BOOLEAN) {
+        assertEquals(rows.size(), limit);
+      }
+    }
+    {
+      String query = String.format(
+          "SELECT COUNT(*) AS count, %s FROM testTable GROUP BY %s ORDER BY %s DESC LIMIT 1000", COLUMN_NAME,
+          COLUMN_NAME, COLUMN_NAME);
+      BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+      ResultTable resultTable = brokerResponse.getResultTable();
+      DataSchema dataSchema = resultTable.getDataSchema();
+      assertEquals(dataSchema, new DataSchema(new String[]{"count", COLUMN_NAME},
+          new ColumnDataType[]{ColumnDataType.LONG, dataType}));
+      List<Object[]> rows = resultTable.getRows();
 
-  @Test(priority = 7)
-  public void testQueriesWithNoDictDoubleColumnNoNullValues()
-          throws Exception {
-    ColumnDataType columnDataType = ColumnDataType.DOUBLE;
-    double baseValue = RANDOM.nextDouble();
-    boolean generateNulls = false;
-    createRecords(baseValue, generateNulls);
-    List<String> noDictionaryColumns = new ArrayList<String>();
-    noDictionaryColumns.add(COLUMN_NAME);
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE)
-            .setTableName(RAW_TABLE_NAME)
-            .setNoDictionaryColumns(noDictionaryColumns)
-            .build();
-    setUp(tableConfig, columnDataType.toDataType());
-    testQueries(baseValue, columnDataType, generateNulls);
+      if (dataType != ColumnDataType.BOOLEAN) {
+        int rowsCount = nullValuesExist ? 501 : 500;
+        assertEquals(rows.size(), rowsCount);
+        int i = 0;
+        for (int index = 0; index < 500; index++) {
+          Object[] row = rows.get(index);
+          assertEquals(row.length, 2);
+          if ((NUM_RECORDS - i - 1) % 2 == 1) {
+            // Null values are inserted at: index % 2 == 1. All null values are grouped into a single null.
+            i++;
+          }
+          assertEquals(row[0], 4L);
+          i++;
+        }
+      }
+      // The default null ordering is 'NULLS LAST'.
+      if (nullValuesExist) {
+        Object[] row = rows.get(rows.size() - 1);
+        assertEquals(row[0], 2000L);
+        assertNull(row[1]);
+      }
+    }
+    // TODO fix String null value handling
+    if (dataType != ColumnDataType.STRING) {

Review Comment:
   Oh I see. So we are not using comparison for min/max and null checker for count. But actually fallback to the numeric handling. Is this correct?



-- 
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-unsubscribe@pinot.apache.org

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


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