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

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


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

Reply via email to