SQL: Fix null handling for AND and OR in SELECT (#35277)

Override `process()` in `BinaryLogicProcessor` which doesn't immediately
return null if left or right argument is null, which is the behaviour of
`process()` of the parent class `BinaryProcessor`.

Also, add more tests for `AND` and `OR` in SELECT clause with literal.

Fixes: #35240

Author: Marios Trivyzas

x-pack/plugin/sql/qa/src/main/resources/select.csv-spec

@@ -1,6 +1,7 @@
 //
 // SELECT with = and !=
 //
+// Need to CAST as STRING since for boolean types jdbc CSV translates null -> false
 equalsSelectClause
 SELECT CAST(4 = 4 AS STRING), CAST(NOT 4 = 4 AS STRING), CAST(3 = 4 AS STRING), CAST(NOT 3 = 4 AS STRING), CAST(1 = null AS STRING), CAST(NOT null = 1 AS STRING);
 
@@ -40,6 +41,37 @@ null                                  |null
 ;
 
 
+//
+// SELECT with OR and AND and NULL handling
+//
+// Need to CAST as STRING since for boolean types jdbc CSV translates null -> false
+selectWithOrAndNullHandling
+SELECT CAST(true OR null AS STRING), CAST(null OR true AS STRING), CAST(false OR null AS STRING), CAST(null OR false AS STRING), CAST(null OR null AS STRING);
+
+  CAST(true OR null AS VARCHAR):s |  CAST(null OR true AS VARCHAR):s |  CAST(false OR null AS VARCHAR):s |  CAST(null OR false AS VARCHAR):s |  CAST(null OR null AS VARCHAR):s
+----------------------------------+----------------------------------+-----------------------------------+-----------------------------------+---------------------------------
+true                              |true                              |null                               |null                               |null
+;
+
+selectWithAndAndNullHandling
+SELECT CAST(true AND null AS STRING), CAST(null AND true AS STRING), CAST(false AND null AS STRING), CAST(null AND false AS STRING), CAST(null AND null AS STRING);
+
+  CAST(true AND null AS VARCHAR):s |  CAST(null AND true AS VARCHAR):s |  CAST(false AND null AS VARCHAR):s |  CAST(null AND false AS VARCHAR):s |  CAST(null AND null AS VARCHAR):s
+-----------------------------------+-----------------------------------+------------------------------------+------------------------------------+----------------------------------
+null                               |null                               |false                               |false                               |null
+;
+
+selectWithOrAndAndAndNullHandling_WithTableColumns
+SELECT CAST(languages = 2 OR null AS STRING), CAST(languages = 2 AND null AS STRING) FROM test_emp WHERE emp_no BETWEEN 10018 AND 10020 ORDER BY emp_no;
+
+ CAST(((languages) == 2) OR null AS VARCHAR):s |  CAST(((languages) == 2) AND null AS VARCHAR):s
+-----------------------------------------------+------------------------------------------------
+true                                           |null
+null                                           |false
+null                                           |null
+;
+
+
 //
 // SELECT with IN
 //

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/BinaryLogicProcessor.java

@@ -84,8 +84,18 @@ public String getWriteableName() {
 
     @Override
     protected void checkParameter(Object param) {
-        if (!(param instanceof Boolean)) {
+        if (param != null && !(param instanceof Boolean)) {
             throw new SqlIllegalArgumentException("A boolean is required; received {}", param);
         }
     }
+
+    @Override
+    public Object process(Object input) {
+        Object l = left().process(input);
+        checkParameter(l);
+        Object r = right().process(input);
+        checkParameter(r);
+
+        return doProcess(l, r);
+    }
 }

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/logical/BinaryLogicProcessorTests.java

@@ -0,0 +1,72 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.sql.expression.predicate.logical;
+
+import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.io.stream.Writeable.Reader;
+import org.elasticsearch.test.AbstractWireSerializingTestCase;
+import org.elasticsearch.xpack.sql.expression.function.scalar.Processors;
+import org.elasticsearch.xpack.sql.expression.gen.processor.ConstantProcessor;
+import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
+
+public class BinaryLogicProcessorTests extends AbstractWireSerializingTestCase<BinaryLogicProcessor> {
+
+    private static final Processor FALSE = new ConstantProcessor(false);
+    private static final Processor TRUE = new ConstantProcessor(true);
+    private static final Processor NULL = new ConstantProcessor((Object) null);
+
+    public static BinaryLogicProcessor randomProcessor() {
+        return new BinaryLogicProcessor(
+                new ConstantProcessor(randomFrom(Boolean.FALSE, Boolean.TRUE, null)),
+                new ConstantProcessor(randomFrom(Boolean.FALSE, Boolean.TRUE, null)),
+                randomFrom(BinaryLogicProcessor.BinaryLogicOperation.values()));
+    }
+
+    @Override
+    protected BinaryLogicProcessor createTestInstance() {
+        return randomProcessor();
+    }
+
+    @Override
+    protected Reader<BinaryLogicProcessor> instanceReader() {
+        return BinaryLogicProcessor::new;
+    }
+
+    @Override
+    protected NamedWriteableRegistry getNamedWriteableRegistry() {
+        return new NamedWriteableRegistry(Processors.getNamedWriteables());
+    }
+
+    public void testOR() {
+        assertEquals(true, new BinaryLogicProcessor(TRUE, FALSE, BinaryLogicProcessor.BinaryLogicOperation.OR).process(null));
+        assertEquals(true, new BinaryLogicProcessor(FALSE, TRUE, BinaryLogicProcessor.BinaryLogicOperation.OR).process(null));
+        assertEquals(false, new BinaryLogicProcessor(FALSE, FALSE, BinaryLogicProcessor.BinaryLogicOperation.OR).process(null));
+        assertEquals(true, new BinaryLogicProcessor(TRUE, TRUE, BinaryLogicProcessor.BinaryLogicOperation.OR).process(null));
+    }
+
+    public void testORNullHandling() {
+        assertEquals(true, new BinaryLogicProcessor(TRUE, NULL, BinaryLogicProcessor.BinaryLogicOperation.OR).process(null));
+        assertEquals(true, new BinaryLogicProcessor(NULL, TRUE, BinaryLogicProcessor.BinaryLogicOperation.OR).process(null));
+        assertNull(new BinaryLogicProcessor(FALSE, NULL, BinaryLogicProcessor.BinaryLogicOperation.OR).process(null));
+        assertNull(new BinaryLogicProcessor(NULL, FALSE, BinaryLogicProcessor.BinaryLogicOperation.OR).process(null));
+        assertNull(new BinaryLogicProcessor(NULL, NULL, BinaryLogicProcessor.BinaryLogicOperation.OR).process(null));
+    }
+
+    public void testAnd() {
+        assertEquals(false, new BinaryLogicProcessor(TRUE, FALSE, BinaryLogicProcessor.BinaryLogicOperation.AND).process(null));
+        assertEquals(false, new BinaryLogicProcessor(FALSE, TRUE, BinaryLogicProcessor.BinaryLogicOperation.AND).process(null));
+        assertEquals(false, new BinaryLogicProcessor(FALSE, FALSE, BinaryLogicProcessor.BinaryLogicOperation.AND).process(null));
+        assertEquals(true, new BinaryLogicProcessor(TRUE, TRUE, BinaryLogicProcessor.BinaryLogicOperation.AND).process(null));
+    }
+
+    public void testAndNullHandling() {
+        assertNull(new BinaryLogicProcessor(TRUE, NULL, BinaryLogicProcessor.BinaryLogicOperation.AND).process(null));
+        assertNull(new BinaryLogicProcessor(NULL, TRUE, BinaryLogicProcessor.BinaryLogicOperation.AND).process(null));
+        assertEquals(false, new BinaryLogicProcessor(FALSE, NULL, BinaryLogicProcessor.BinaryLogicOperation.AND).process(null));
+        assertEquals(false, new BinaryLogicProcessor(NULL, FALSE, BinaryLogicProcessor.BinaryLogicOperation.AND).process(null));
+        assertNull(new BinaryLogicProcessor(NULL, NULL, BinaryLogicProcessor.BinaryLogicOperation.AND).process(null));
+    }
+}