yashmayya commented on code in PR #13255:
URL: https://github.com/apache/pinot/pull/13255#discussion_r1618651872
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/NullHandlingIntegrationTest.java:
##########
@@ -219,97 +213,187 @@ public void
testTotalCountWithNullHandlingQueryOptionEnabled(boolean useMultiSta
public void testNullLiteralSelectionOnlyBroker(boolean
useMultiStageQueryEngine)
throws Exception {
setUseMultiStageQueryEngine(useMultiStageQueryEngine);
+ // V2 handles such queries in the servers where all rows in the table are
matched and hence the number of rows
+ // returned in the result set is equal to the total number of rows in the
table (instead of a single row like in
+ // V1 where it is handled in the broker). The V2 way is more standard
though and matches behavior in other
+ // databases like Postgres.
notSupportedInV2();
+
// Null literal only
String sqlQuery = "SELECT null FROM mytable
OPTION(enableNullHandling=true)";
JsonNode response = postQuery(sqlQuery);
JsonNode rows = response.get("resultTable").get("rows");
assertTrue(response.get("exceptions").isEmpty());
- assertEquals(rows.size(), 1);
- assertEquals(rows.get(0).get(0).asText(), "null");
+ assertEquals(1, rows.size());
+ assertEquals("null", rows.get(0).get(0).asText());
// Null related functions
sqlQuery = "SELECT isNull(null) FROM " + getTableName() + " OPTION
(enableNullHandling=true);";
response = postQuery(sqlQuery);
rows = response.get("resultTable").get("rows");
assertTrue(response.get("exceptions").isEmpty());
- assertEquals(rows.size(), 1);
- assertEquals(rows.get(0).get(0).asBoolean(), true);
+ assertEquals(1, rows.size());
+ assertTrue(rows.get(0).get(0).asBoolean());
sqlQuery = "SELECT isNotNull(null) FROM " + getTableName() + " OPTION
(enableNullHandling=true);";
response = postQuery(sqlQuery);
rows = response.get("resultTable").get("rows");
assertTrue(response.get("exceptions").isEmpty());
- assertEquals(rows.size(), 1);
- assertEquals(rows.get(0).get(0).asBoolean(), false);
+ assertEquals(1, rows.size());
+ assertFalse(rows.get(0).get(0).asBoolean());
sqlQuery = "SELECT coalesce(null, 1) FROM " + getTableName() + " OPTION
(enableNullHandling=true);";
response = postQuery(sqlQuery);
rows = response.get("resultTable").get("rows");
assertTrue(response.get("exceptions").isEmpty());
- assertEquals(rows.size(), 1);
- assertEquals(rows.get(0).get(0).asInt(), 1);
+ assertEquals(1, rows.size());
+ assertEquals(1, rows.get(0).get(0).asInt());
sqlQuery = "SELECT coalesce(null, null) FROM " + getTableName() + "
OPTION (enableNullHandling=true);";
response = postQuery(sqlQuery);
rows = response.get("resultTable").get("rows");
assertTrue(response.get("exceptions").isEmpty());
- assertEquals(rows.size(), 1);
- assertEquals(rows.get(0).get(0).asText(), "null");
+ assertEquals(1, rows.size());
+ assertEquals("null", rows.get(0).get(0).asText());
sqlQuery = "SELECT isDistinctFrom(null, null) FROM " + getTableName() + "
OPTION (enableNullHandling=true);";
response = postQuery(sqlQuery);
rows = response.get("resultTable").get("rows");
assertTrue(response.get("exceptions").isEmpty());
- assertEquals(rows.size(), 1);
- assertEquals(rows.get(0).get(0).asBoolean(), false);
+ assertEquals(1, rows.size());
+ assertFalse(rows.get(0).get(0).asBoolean());
sqlQuery = "SELECT isNotDistinctFrom(null, null) FROM " + getTableName() +
" OPTION (enableNullHandling=true);";
response = postQuery(sqlQuery);
rows = response.get("resultTable").get("rows");
assertTrue(response.get("exceptions").isEmpty());
- assertEquals(rows.size(), 1);
- assertEquals(rows.get(0).get(0).asBoolean(), true);
+ assertEquals(1, rows.size());
+ assertTrue(rows.get(0).get(0).asBoolean());
sqlQuery = "SELECT isDistinctFrom(null, 1) FROM " + getTableName() + "
OPTION (enableNullHandling=true);";
response = postQuery(sqlQuery);
rows = response.get("resultTable").get("rows");
assertTrue(response.get("exceptions").isEmpty());
- assertEquals(rows.size(), 1);
- assertEquals(rows.get(0).get(0).asBoolean(), true);
+ assertEquals(1, rows.size());
+ assertTrue(rows.get(0).get(0).asBoolean());
sqlQuery = "SELECT isNotDistinctFrom(null, 1) FROM " + getTableName() + "
OPTION (enableNullHandling=true);";
response = postQuery(sqlQuery);
rows = response.get("resultTable").get("rows");
assertTrue(response.get("exceptions").isEmpty());
- assertEquals(rows.size(), 1);
- assertEquals(rows.get(0).get(0).asBoolean(), false);
+ assertEquals(1, rows.size());
+ assertFalse(rows.get(0).get(0).asBoolean());
sqlQuery = "SELECT case when true then null end FROM " + getTableName() +
" OPTION (enableNullHandling=true);";
response = postQuery(sqlQuery);
rows = response.get("resultTable").get("rows");
assertTrue(response.get("exceptions").isEmpty());
- assertEquals(rows.size(), 1);
- assertEquals(rows.get(0).get(0).asText(), "null");
+ assertEquals(1, rows.size());
+ assertEquals("null", rows.get(0).get(0).asText());
sqlQuery = "SELECT case when false then 1 end FROM " + getTableName() + "
OPTION (enableNullHandling=true);";
response = postQuery(sqlQuery);
rows = response.get("resultTable").get("rows");
assertTrue(response.get("exceptions").isEmpty());
- assertEquals(rows.size(), 1);
- assertEquals(rows.get(0).get(0).asText(), "null");
+ assertEquals(1, rows.size());
+ assertEquals("null", rows.get(0).get(0).asText());
// Null intolerant functions
sqlQuery = "SELECT add(null, 1) FROM " + getTableName() + " OPTION
(enableNullHandling=true);";
response = postQuery(sqlQuery);
rows = response.get("resultTable").get("rows");
assertTrue(response.get("exceptions").isEmpty());
- assertEquals(rows.size(), 1);
- assertEquals(rows.get(0).get(0).asText(), "null");
+ assertEquals(1, rows.size());
+ assertEquals("null", rows.get(0).get(0).asText());
+ }
+
+ @Test
+ public void testNullLiteralSelectionInV2() throws Exception {
Review Comment:
Makes sense, I've added a data provider and updated both these tests. Let me
know if this is along the lines of what you were thinking about. We could very
easily add another parameter for v1 / v2 and use that to determine the query
engine and the expected number of rows in order to combine both the test
methods into a single one. However, given the notable difference in behavior
between the two engines in this case (v1 simply handles it in the broker and
returns a single result), I preferred keeping them as separate tests using the
same data provider to emphasize the difference.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
