From dd6686fb6b0d2b675b4f24eebaa677a10d70ead3 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 11 Oct 2023 13:12:57 -0700 Subject: [PATCH 1/2] fix issue with SQL boolean constants not respecting nulls when strict booleans are enabled --- .../calcite/rule/DruidLogicalValuesRule.java | 4 ++++ .../rule/DruidLogicalValuesRuleTest.java | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRule.java b/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRule.java index 614ffddf5667..0faa9d1befd7 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRule.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRule.java @@ -26,6 +26,7 @@ import org.apache.calcite.rel.logical.LogicalValues; import org.apache.calcite.rex.RexLiteral; import org.apache.druid.error.InvalidSqlInput; +import org.apache.druid.math.expr.ExpressionProcessing; import org.apache.druid.query.InlineDataSource; import org.apache.druid.segment.column.RowSignature; import org.apache.druid.sql.calcite.planner.Calcites; @@ -120,6 +121,9 @@ public static Object getValueFromLiteral(RexLiteral literal, PlannerContext plan } return ((Number) RexLiteral.value(literal)).longValue(); case BOOLEAN: + if (ExpressionProcessing.useStrictBooleans() && literal.isNull()) { + return null; + } return literal.isAlwaysTrue() ? 1L : 0L; case TIMESTAMP: case DATE: diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRuleTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRuleTest.java index ee2e4273f83b..981fe6ae60e6 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRuleTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRuleTest.java @@ -30,6 +30,7 @@ import org.apache.calcite.util.TimestampString; import org.apache.druid.error.DruidExceptionMatcher; import org.apache.druid.java.util.common.DateTimes; +import org.apache.druid.math.expr.ExpressionProcessing; import org.apache.druid.sql.calcite.planner.DruidTypeSystem; import org.apache.druid.sql.calcite.planner.PlannerContext; import org.apache.druid.testing.InitializedNullHandlingTest; @@ -139,6 +140,26 @@ public void testGetValueFromFalseLiteral() Assert.assertEquals(0L, fromLiteral); } + @Test + public void testGetValueFromNullBooleanLiteral() + { + RexLiteral literal = REX_BUILDER.makeLiteral(null, REX_BUILDER.getTypeFactory().createSqlType(SqlTypeName.BOOLEAN)); + + try { + ExpressionProcessing.initializeForStrictBooleansTests(true); + final Object fromLiteral = DruidLogicalValuesRule.getValueFromLiteral(literal, DEFAULT_CONTEXT); + Assert.assertNull(fromLiteral); + + ExpressionProcessing.initializeForStrictBooleansTests(false); + final Object fromLiteralNonStrict = DruidLogicalValuesRule.getValueFromLiteral(literal, DEFAULT_CONTEXT); + Assert.assertSame(Long.class, fromLiteralNonStrict.getClass()); + Assert.assertEquals(0L, fromLiteralNonStrict); + } + finally { + ExpressionProcessing.initializeForTests(); + } + } + @Test public void testGetValueFromTimestampLiteral() { From a8e083a0a2345e6eff2cfad84b5b062cb678dff8 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 11 Oct 2023 19:46:56 -0700 Subject: [PATCH 2/2] require sql compatible null handling and strict booleans --- .../druid/sql/calcite/rule/DruidLogicalValuesRule.java | 3 ++- .../sql/calcite/rule/DruidLogicalValuesRuleTest.java | 10 +++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRule.java b/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRule.java index 0faa9d1befd7..97fa5b86a6a1 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRule.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRule.java @@ -25,6 +25,7 @@ import org.apache.calcite.plan.RelOptRuleCall; import org.apache.calcite.rel.logical.LogicalValues; import org.apache.calcite.rex.RexLiteral; +import org.apache.druid.common.config.NullHandling; import org.apache.druid.error.InvalidSqlInput; import org.apache.druid.math.expr.ExpressionProcessing; import org.apache.druid.query.InlineDataSource; @@ -121,7 +122,7 @@ public static Object getValueFromLiteral(RexLiteral literal, PlannerContext plan } return ((Number) RexLiteral.value(literal)).longValue(); case BOOLEAN: - if (ExpressionProcessing.useStrictBooleans() && literal.isNull()) { + if (ExpressionProcessing.useStrictBooleans() && NullHandling.sqlCompatible() && literal.isNull()) { return null; } return literal.isAlwaysTrue() ? 1L : 0L; diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRuleTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRuleTest.java index 981fe6ae60e6..2ad83563e9ef 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRuleTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/rule/DruidLogicalValuesRuleTest.java @@ -28,6 +28,7 @@ import org.apache.calcite.util.DateString; import org.apache.calcite.util.TimeString; import org.apache.calcite.util.TimestampString; +import org.apache.druid.common.config.NullHandling; import org.apache.druid.error.DruidExceptionMatcher; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.math.expr.ExpressionProcessing; @@ -145,19 +146,14 @@ public void testGetValueFromNullBooleanLiteral() { RexLiteral literal = REX_BUILDER.makeLiteral(null, REX_BUILDER.getTypeFactory().createSqlType(SqlTypeName.BOOLEAN)); - try { - ExpressionProcessing.initializeForStrictBooleansTests(true); + if (NullHandling.sqlCompatible() && ExpressionProcessing.useStrictBooleans()) { final Object fromLiteral = DruidLogicalValuesRule.getValueFromLiteral(literal, DEFAULT_CONTEXT); Assert.assertNull(fromLiteral); - - ExpressionProcessing.initializeForStrictBooleansTests(false); + } else { final Object fromLiteralNonStrict = DruidLogicalValuesRule.getValueFromLiteral(literal, DEFAULT_CONTEXT); Assert.assertSame(Long.class, fromLiteralNonStrict.getClass()); Assert.assertEquals(0L, fromLiteralNonStrict); } - finally { - ExpressionProcessing.initializeForTests(); - } } @Test