You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/04/01 22:41:10 UTC

[GitHub] [druid] clintropolis commented on a change in pull request #11060: fix lookup nullable

clintropolis commented on a change in pull request #11060:
URL: https://github.com/apache/druid/pull/11060#discussion_r605983922



##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -17086,4 +17086,32 @@ public void testGroupingSetsWithLimitOrderByGran() throws Exception
         ).build()
     );
   }
+
+
+  @Test
+  public void testLookupWithNull() throws Exception
+  {
+    testQuery(
+        "SELECT dim2 ,lookup(dim2,'lookyloo') from foo where dim2 is null",
+        ImmutableList.of(
+            new Druids.ScanQueryBuilder()
+            .dataSource(CalciteTests.DATASOURCE1)
+            .intervals(querySegmentSpec(Filtration.eternity()))
+            .virtualColumns(
+                expressionVirtualColumn("v0", "null", ValueType.STRING)
+            )
+            .columns("v0")
+            .legacy(false)
+            .filters(new SelectorDimFilter("dim2", "", null))
+            .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
+            .context(QUERY_CONTEXT_DEFAULT)
+            .build()
+        ),
+        ImmutableList.<Object[]>builder().add(
+            new Object[]{"", NULL_STRING},
+            new Object[]{"", NULL_STRING},
+            new Object[]{"", NULL_STRING}

Review comment:
       It looks like this is failing CI in SQL compatible null handling mode because the expected results are different, where there are only 2 null valued `dim2` rows instead of 3. The data for the 'foo' datasource is here: https://github.com/apache/druid/blob/master/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java#L368. In default mode (`druid.generic.useDefaultValueForNull=true`), the empty string is equivalent to null, so https://github.com/apache/druid/blob/master/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java#L390 is treated as null, but when `druid.generic.useDefaultValueForNull=false`, only https://github.com/apache/druid/blob/master/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java#L382 and https://github.com/apache/druid/blob/master/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java#L409 have null valued `dim2`. Additionally, `dim2` would be `null` instead of the empty string in the results.




-- 
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.

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



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