fix return type inference of parse_long, which can also be null if st…

…ring is not parseable into a long (#15909) * fix return type inference of parse_long, which can also be null if string is not parseable into a long * fix msq test
apache · Feb 15, 2024 · fe2ba8c · fe2ba8c
1 parent 66f54f2
commit fe2ba8c
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 1 deletion.
diff --git a/.../multi-stage-query/src/test/java/org/apache/druid/msq/test/CalciteSelectQueryMSQTest.java b/.../multi-stage-query/src/test/java/org/apache/druid/msq/test/CalciteSelectQueryMSQTest.java
@@ -24,6 +24,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.inject.Injector;
 import com.google.inject.Module;
+import org.apache.druid.common.config.NullHandling;
 import org.apache.druid.guice.DruidInjectorBuilder;
 import org.apache.druid.java.util.common.ISE;
 import org.apache.druid.msq.exec.WorkerMemoryParameters;
@@ -215,4 +216,18 @@ public void testJoinMultipleTablesWithWhereCondition()
         )
         .run();
   }
+
+  @Override
+  public void testFilterParseLongNullable()
+  {
+    // this isn't really correct in default value mode, the result should be ImmutableList.of(new Object[]{0L})
+    // but MSQ is missing default aggregator values in empty group results. this override can be removed when this
+    // is fixed
+    testBuilder().queryContext(QUERY_CONTEXT_DEFAULT)
+                 .sql("select count(*) from druid.foo where parse_long(dim1, 10) is null")
+                 .expectedResults(
+                     NullHandling.sqlCompatible() ? ImmutableList.of(new Object[]{4L}) : ImmutableList.of()
+                 )
+                 .run();
+  }
 }
diff --git a/...ain/java/org/apache/druid/sql/calcite/expression/builtin/ParseLongOperatorConversion.java b/...ain/java/org/apache/druid/sql/calcite/expression/builtin/ParseLongOperatorConversion.java
@@ -38,7 +38,7 @@ public class ParseLongOperatorConversion implements SqlOperatorConversion
   private static final SqlFunction SQL_FUNCTION = OperatorConversions
       .operatorBuilder(NAME)
       .operandTypes(SqlTypeFamily.CHARACTER, SqlTypeFamily.INTEGER)
-      .returnTypeCascadeNullable(SqlTypeName.BIGINT)
+      .returnTypeNullable(SqlTypeName.BIGINT)
       .functionCategory(SqlFunctionCategory.STRING)
       .requiredOperandCount(1)
       .build();

diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -15292,4 +15292,29 @@ public void testWindowingWithOrderBy()
         )
         .run();
   }
+
+  @Test
+  public void testFilterParseLongNullable()
+  {
+    testQuery(
+        "select count(*) from druid.foo where parse_long(dim1, 10) is null",
+        ImmutableList.of(
+            Druids.newTimeseriesQueryBuilder()
+                  .dataSource(CalciteTests.DATASOURCE1)
+                  .intervals(querySegmentSpec(Filtration.eternity()))
+                  .virtualColumns(
+                      expressionVirtualColumn(
+                          "v0",
+                          "parse_long(\"dim1\",10)",
+                          ColumnType.LONG)
+                  )
+                  .filters(isNull("v0"))
+                  .granularity(Granularities.ALL)
+                  .aggregators(new CountAggregatorFactory("a0"))
+                  .context(QUERY_CONTEXT_DEFAULT)
+                  .build()
+        ),
+        ImmutableList.of(new Object[]{NullHandling.sqlCompatible() ? 4L : 0L})
+    );
+  }
 }