Skip to content

Fix nullable AND results in tree-model CASE #18875

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ColinLeeo wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix nullable AND results in tree-model CASE #18875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de140b4
Fix nullable AND results in tree-model CASE
ColinLeeo Oct 14, 2026
3316fa6
Strengthen nullable AND and OR selection regression tests
ColinLeeo Oct 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions integration-test/src/test/java/org/apache/iotdb/db/it/query/IoTDBCaseWhenThenIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -928,4 +928,30 @@ public void testThenWithBinarySameConstant() {
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 "

@JackieTien97 JackieTien97 Oct 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the AND only appears as a WHEN condition, NULL and FALSE both mean "no match", so this query only covers the NULL AND TRUE half of the fix (time 1: 2.0 before the fix, 3.0 after). Time 3 (0 > 0 AND 1 > 0) is 3.0 whether the AND returns FALSE or NULL: with line 73 of LogicAndColumnTransformer changed to builder.appendNull(), this query still returns 3.0, 2.0, 3.0, 1.0.

A query that exposes the AND result itself would cover the FALSE half too. On the same data:

query before fix after fix
CASE WHEN s1 > 100 THEN 1 WHEN NOT (s1 > 0 AND s2 > 0) THEN 2 ELSE 3 END 3.0, 3.0, 3.0, 1.0 3.0, 3.0, 2.0, 1.0
CASE WHEN s1 > 100 THEN false WHEN s2 > 0 THEN (s1 > 0 AND s2 > 0) END true, true, null, false null, true, false, false

Both also fail with the appendNull() change (time 3 becomes 3.0 / null). I got these values by running the expressions through ColumnTransformerVisitor + FilterAndProjectOperator on this PR's code, with and without the fix.

+ "FROM root.sg.case_null",
new String[] {TIMESTAMP_STR, "result"},
new String[] {"1,3.0,", "2,2.0,", "3,3.0,", "4,1.0,"});
resultSetEqualTest(
"SELECT CASE WHEN s1 > 100 THEN 1 WHEN NOT (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,3.0,", "3,2.0,", "4,1.0,"});
resultSetEqualTest(
"SELECT CASE WHEN s1 > 100 THEN false WHEN s2 > 0 THEN (s1 > 0 AND s2 > 0) END AS result "
+ "FROM root.sg.case_null",
new String[] {TIMESTAMP_STR, "result"},
new String[] {"1,null,", "2,true,", "3,false,", "4,false,"});
}
}
2 changes: 1 addition & 1 deletion ...ava/org/apache/iotdb/calc/transformation/dag/column/binary/LogicAndColumnTransformer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public void evaluateWithSelection(boolean[] selection) {
}
}

initializeColumnCache(rightColumn);
initializeColumnCache(builder.build());
this.leftTransformer.clearCache();
this.rightTransformer.clearCache();
}
Expand Down
90 changes: 90 additions & 0 deletions ...tdb/db/queryengine/transformation/dag/column/binary/LogicBinaryColumnTransformerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.iotdb.db.queryengine.transformation.dag.column.binary;

import org.apache.iotdb.calc.transformation.dag.column.ColumnTransformer;
import org.apache.iotdb.calc.transformation.dag.column.binary.BinaryColumnTransformer;
import org.apache.iotdb.calc.transformation.dag.column.binary.LogicAndColumnTransformer;
import org.apache.iotdb.calc.transformation.dag.column.binary.LogicOrColumnTransformer;
Expand All @@ -37,13 +38,22 @@
import org.junit.Test;

import java.util.Arrays;
import java.util.function.BiFunction;

public class LogicBinaryColumnTransformerTest {

private static final Type returnType = TypeFactory.getType(TSDataType.BOOLEAN);

private static final int POSITION_COUNT = 4;

private static final Boolean[] AND_EXPECTED = {
null, false, null, false, false, false, null, false, true
};

private static final Boolean[] OR_EXPECTED = {
null, null, true, null, false, true, true, true, true
};

private static final boolean[] leftInput = new boolean[] {true, false, true, false};

private static final boolean[] rightInput = new boolean[] {true, false, true, false};
Expand Down Expand Up @@ -113,4 +123,84 @@ public void testInputType() {

}
}

@Test
public void testLogicAndWithNullsAndSelection() {
assertSelected(
(left, right) -> new LogicAndColumnTransformer(returnType, left, right),
AND_EXPECTED,
false);
}

@Test
public void testLogicAndWithNullsAndPartialSelection() {
assertSelected(
(left, right) -> new LogicAndColumnTransformer(returnType, left, right),
AND_EXPECTED,
true);
}

@Test
public void testLogicOrWithNullsAndSelection() {
assertSelected(
(left, right) -> new LogicOrColumnTransformer(returnType, left, right), OR_EXPECTED, false);
}

@Test
public void testLogicOrWithNullsAndPartialSelection() {
assertSelected(
(left, right) -> new LogicOrColumnTransformer(returnType, left, right), OR_EXPECTED, true);
}

private void assertSelected(
BiFunction<ColumnTransformer, ColumnTransformer, BinaryColumnTransformer> transformerFactory,
Boolean[] expected,
boolean partial) {
Boolean[] values = {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);
BinaryColumnTransformer transformer = transformerFactory.apply(left, right);
transformer.addReferenceCount();
boolean[] selection = new boolean[9];
for (int i = 0; i < selection.length; i++) {
// Keep the right-FALSE rows (1 and 7) selected while excluding some left-FALSE rows.
selection[i] = !partial || (i != 3 && i != 5);
}
boolean[] selected = selection.clone();
transformer.evaluateWithSelection(selection);
Assert.assertArrayEquals(selected, selection);
Column result = transformer.getColumn();
Assert.assertEquals(expected.length, result.getPositionCount());
for (int i = 0; i < expected.length; i++) {
if (!selected[i] || expected[i] == null) {
Assert.assertTrue(result.isNull(i));
} else {
Assert.assertFalse(result.isNull(i));
Assert.assertEquals(expected[i], result.getBoolean(i));

@JackieTien97 JackieTien97 Oct 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch only checks getBoolean(i), and a NULL position in BooleanColumn reads as false, so the test can't tell FALSE from NULL. That leaves the "left is FALSE, so the result is FALSE" half of the fix unprotected:

  • Changing line 73 of LogicAndColumnTransformer to builder.appendNull() makes rows 3-5 (left = false) return NULL, and both new tests still pass.
  • With the pre-fix code, the test only catches row 2 (NULL AND TRUE, line 166). Rows 3-5 also came back NULL there, which the assertion on this line accepts.

The loop also reads selection after evaluateWithSelection has run, so it wouldn't notice the method mutating it: dropping the selection.clone() on line 36 likewise turns rows 3-5 into NULL, and the tests still pass.

Suggestion: keep a copy of the selection and assert non-null explicitly.

boolean[] selected = selection.clone();
transformer.evaluateWithSelection(selection);
Column result = transformer.getColumn();
Assert.assertEquals(expected.length, result.getPositionCount());
for (int i = 0; i < expected.length; i++) {
  if (!selected[i] || expected[i] == null) {
    Assert.assertTrue(result.isNull(i));
  } else {
    Assert.assertFalse(result.isNull(i));
    Assert.assertEquals(expected[i], result.getBoolean(i));
  }
}

With these assertions, the pre-fix code and both changes above fail, and the current code passes.

Minor: the partial mask i % 2 == 0 leaves rows 1 and 7 unselected, and they are the only rows that reach the "right is false" branch (lines 58-60), so that branch never runs with a partial selection. A mask that keeps row 1 or 7 selected would cover it.

}
}
}
}
Loading