Skip to content

[Bug] Tree-model CASE selects a branch when nullable AND should be NULL #18864

Description

@ColinLeeo

Scope

Track this pre-existing tree-model AND defect independently of PR #18826's CASE cache-lifecycle/selection refactor, as requested in the review's out-of-scope note.

The standalone correction applies to master at a498d7d2a4 and changes only the result published by LogicAndColumnTransformer.evaluateWithSelection():

- initializeColumnCache(rightColumn);
+ initializeColumnCache(builder.build());

It does not require the cache-lifecycle redesign or the new caller-owned selection API.

Standalone fix: PR #18875, based on master 8effe5d6ce. It closes this issue when merged. PR #18826 currently retains the equivalent correction while adapting the selection interface; after the standalone fix lands, synchronizing that PR with master will remove the duplicated correction/test diff. This issue is not configured to be closed by #18826.

Reproduction

For a row where s1 is NULL and s2 is 1:

SELECT CASE
  WHEN s1 > 100 THEN 1
  WHEN s1 > 0 AND s2 > 0 THEN 2
  ELSE 3
END FROM root.p;

Expected: 3. Actual on the original code: 2. Both WHEN conditions are NULL, but selected AND returns the right operand (true) rather than the constructed AND result (NULL).

Standalone regression coverage and evidence

The patch below adds two selection-mode unit tests covering all nine combinations of NULL/false/true, with full and partial masks, plus the tree CASE SQL regression above.

The standalone branch passed Maven reactor verify from master 8effe5d6ce, selecting only LogicBinaryColumnTransformerTest (all 5 methods) and IoTDBCaseWhenThenIT#testNullableAndInLaterCaseBranch (1 SQL test against rebuilt 1C1D nodes). Spotless, Checkstyle, and license checks also passed. A separate class-isolated comparison using the same unit-test runtime and the original master AND implementation confirms that both newly added unit tests fail before the one-line fix.

Standalone patch against master a498d7d
diff --git a/integration-test/src/test/java/org/apache/iotdb/db/it/query/IoTDBCaseWhenThenIT.java b/integration-test/src/test/java/org/apache/iotdb/db/it/query/IoTDBCaseWhenThenIT.java
--- a/integration-test/src/test/java/org/apache/iotdb/db/it/query/IoTDBCaseWhenThenIT.java
+++ b/integration-test/src/test/java/org/apache/iotdb/db/it/query/IoTDBCaseWhenThenIT.java
@@ -928,4 +928,19 @@
         new String[] {"0,0.0,", "1000000,11.0,", "20000000,22.0,", "210000000,33.0,"};
     resultSetEqualTest(sql, expectedHeader, retArray);
   }
+  @Test
+  public void testNullableAndInLaterCaseBranch() {
+    prepareData(
+        new String[] {
+          "CREATE TIMESERIES root.sg.case_null.s1 WITH DATATYPE=INT32, ENCODING=PLAIN",
+          "CREATE TIMESERIES root.sg.case_null.s2 WITH DATATYPE=INT32, ENCODING=PLAIN",
+          "INSERT INTO root.sg.case_null(time, s2) VALUES (1, 1)",
+          "INSERT INTO root.sg.case_null(time, s1, s2) VALUES (2, 1, 1), (3, 0, 1), (4, 101, 1)"
+        });
+    resultSetEqualTest(
+        "SELECT CASE WHEN s1 > 100 THEN 1 WHEN s1 > 0 AND s2 > 0 THEN 2 ELSE 3 END AS result "
+            + "FROM root.sg.case_null",
+        new String[] {TIMESTAMP_STR, "result"},
+        new String[] {"1,3.0,", "2,2.0,", "3,3.0,", "4,1.0,"});
+  }
 }
diff --git a/iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/transformation/dag/column/binary/LogicAndColumnTransformer.java b/iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/transformation/dag/column/binary/LogicAndColumnTransformer.java
--- a/iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/transformation/dag/column/binary/LogicAndColumnTransformer.java
+++ b/iotdb-core/calc-commons/src/main/java/org/apache/iotdb/calc/transformation/dag/column/binary/LogicAndColumnTransformer.java
@@ -77,7 +77,7 @@
       }
     }
 
-    initializeColumnCache(rightColumn);
+    initializeColumnCache(builder.build());
     this.leftTransformer.clearCache();
     this.rightTransformer.clearCache();
   }
diff --git a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/transformation/dag/column/binary/LogicBinaryColumnTransformerTest.java b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/transformation/dag/column/binary/LogicBinaryColumnTransformerTest.java
--- a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/transformation/dag/column/binary/LogicBinaryColumnTransformerTest.java
+++ b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/transformation/dag/column/binary/LogicBinaryColumnTransformerTest.java
@@ -113,4 +113,60 @@
 
     }
   }
+
+  @Test
+  public void testLogicAndWithNullsAndSelection() {
+    assertSelectedAnd(false);
+  }
+
+  @Test
+  public void testLogicAndWithNullsAndPartialSelection() {
+    assertSelectedAnd(true);
+  }
+
+  private void assertSelectedAnd(boolean partial) {
+    Boolean[] values = {null, false, true};
+    Boolean[] expected = {null, false, null, false, false, false, null, false, true};
+    TsBlockBuilder builder =
+        new TsBlockBuilder(9, Arrays.asList(TSDataType.BOOLEAN, TSDataType.BOOLEAN));
+    for (Boolean left : values) {
+      for (Boolean right : values) {
+        builder.getTimeColumnBuilder().writeLong(0);
+        if (left == null) {
+          builder.getColumnBuilder(0).appendNull();
+        } else {
+          builder.getColumnBuilder(0).writeBoolean(left);
+        }
+        if (right == null) {
+          builder.getColumnBuilder(1).appendNull();
+        } else {
+          builder.getColumnBuilder(1).writeBoolean(right);
+        }
+        builder.declarePosition();
+      }
+    }
+    TsBlock block = builder.build();
+    IdentityColumnTransformer left = new IdentityColumnTransformer(returnType, 0);
+    IdentityColumnTransformer right = new IdentityColumnTransformer(returnType, 1);
+    left.addReferenceCount();
+    right.addReferenceCount();
+    left.initFromTsBlock(block);
+    right.initFromTsBlock(block);
+    LogicAndColumnTransformer transformer = new LogicAndColumnTransformer(returnType, left, right);
+    transformer.addReferenceCount();
+    boolean[] selection = new boolean[9];
+    for (int i = 0; i < selection.length; i++) {
+      selection[i] = !partial || i % 2 == 0;
+    }
+    transformer.evaluateWithSelection(selection);
+    Column result = transformer.getColumn();
+    Assert.assertEquals(expected.length, result.getPositionCount());
+    for (int i = 0; i < expected.length; i++) {
+      if (!selection[i] || expected[i] == null) {
+        Assert.assertTrue(result.isNull(i));
+      } else {
+        Assert.assertEquals(expected[i], result.getBoolean(i));
+      }
+    }
+  }
 }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions