Skip to content

Commit 4e50a85

Browse files
sunshonesunshone
committed
[VL] Address review comments: IS TRUE via flinksql is_true, tests, CI velox4j pin
- Switch the IS TRUE entry in RexCallConverterFactory from IsTrueRexCallConverter (equalto(x, true), which returns NULL for a NULL input) to DefaultRexCallConverter("is_true"): Flink x IS TRUE returns FALSE for UNKNOWN and never NULL (ScalarOperatorGens.scala:771-777, NEVER_NULL). IsTrueRexCallConverter itself is baseline code and stays untouched. - Add SQL-level tests (LogicalFunctionsTest, ScalarFunctionsTest paradigm) over a BOOLEAN column containing NULLs: three-valued truth tables for NOT/IS TRUE/IS FALSE/IS NOT TRUE/IS NOT FALSE (NOT preserves NULL, the IS_* predicates never return NULL), nested forms NOT(c1 IS FALSE) and (NOT(c1)) IS NOT TRUE (IS binds tighter than NOT, so the parenthesized form is the one that applies IS NOT TRUE to NOT c1), constant and constant-foldable operands including CAST(NULL AS BOOLEAN) IS TRUE, and a WHERE-clause filter case. - Point the flink CI velox4j clone at the sunshone fork's flinksql-logical-expressions branch carrying the velox ref bump for this change (fix-velox4j.patch applies cleanly on the pinned head), reverting to bigo-sg gluten-20260829 once the velox-side functions land there.
1 parent 8e5f4a7 commit 4e50a85

4 files changed

Lines changed: 130 additions & 5 deletions

File tree

‎.github/workflows/flink.yml‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,12 @@ jobs:
8787
export VELOX_DEPENDENCY_SOURCE=BUNDLED
8888
export fmt_SOURCE=BUNDLED
8989
export folly_SOURCE=BUNDLED
90-
git clone -b gluten-20260829 https://github.com/bigo-sg/velox4j.git
90+
# TODO: revert to the bigo-sg/velox4j gluten-20260829 branch (pin
91+
# 2d08448f556bff669fe413874d04dd8cd96f65b2) once the velox-side
92+
# flinksql logical functions land there.
93+
git clone -b flinksql-logical-expressions https://github.com/sunshone/velox4j.git
9194
cd velox4j
92-
git reset --hard cc8b56d74baa3863ae0fb8387f5745652370fa7d
95+
git reset --hard d80c64e8e59a40db0807e6c70620f78a78f20ca5
9396
git apply $GITHUB_WORKSPACE/gluten-flink/patches/fix-velox4j.patch
9497
$GITHUB_WORKSPACE/build/mvn clean install -DskipTests -Dgpg.skip -Dspotless.skip=true
9598
cd ..

‎gluten-flink/docs/Flink.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,12 @@ As some features have not been committed to upstream, you have to use the follow
4646

4747
```bash
4848
## fetch velox4j code
49-
git clone -b gluten-20260829 https://github.com/bigo-sg/velox4j.git
49+
# TODO: revert to the bigo-sg/velox4j gluten-20260829 branch (pin
50+
# 2d08448f556bff669fe413874d04dd8cd96f65b2) once the velox-side
51+
# flinksql logical functions land there.
52+
git clone -b flinksql-logical-expressions https://github.com/sunshone/velox4j.git
5053
cd velox4j
51-
git reset --hard cc8b56d74baa3863ae0fb8387f5745652370fa7d
54+
git reset --hard d80c64e8e59a40db0807e6c70620f78a78f20ca5
5255
mvn clean install -DskipTests -Dgpg.skip -Dspotless.skip=true
5356
```
5457
**Get gluten**

‎gluten-flink/planner/src/main/java/org/apache/gluten/rexnode/functions/RexCallConverterFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ public class RexCallConverterFactory {
110110
Map.entry("LOWER", Arrays.asList(() -> new DefaultRexCallConverter("lower"))),
111111
Map.entry("count_char", Arrays.asList(() -> new DefaultRexCallConverter("count_char"))),
112112
Map.entry("EXTRACT", Arrays.asList(() -> new DefaultRexCallConverter("extract"))),
113-
Map.entry("IS TRUE", Arrays.asList(() -> new IsTrueRexCallConverter())),
113+
Map.entry("IS TRUE", Arrays.asList(() -> new DefaultRexCallConverter("is_true"))),
114114
Map.entry("IS FALSE", Arrays.asList(() -> new DefaultRexCallConverter("is_false"))),
115115
Map.entry("IS NOT TRUE", Arrays.asList(() -> new DefaultRexCallConverter("is_not_true"))),
116116
Map.entry(

‎gluten-flink/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/LogicalFunctionsTest.java‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.gluten.table.runtime.stream.custom;
18+
19+
import org.apache.gluten.table.runtime.stream.common.GlutenStreamingTestBase;
20+
21+
import org.apache.flink.types.Row;
22+
23+
import org.junit.jupiter.api.BeforeEach;
24+
import org.junit.jupiter.api.Test;
25+
26+
import java.util.Arrays;
27+
import java.util.List;
28+
29+
/** SQL-level tests for the logical operators over a boolean column containing NULLs. */
30+
class LogicalFunctionsTest extends GlutenStreamingTestBase {
31+
32+
@Override
33+
@BeforeEach
34+
public void before() throws Exception {
35+
super.before();
36+
}
37+
38+
// Three-valued truth-table input: TRUE, FALSE, UNKNOWN.
39+
private List<Row> truthTableRows() {
40+
return Arrays.asList(Row.of(true), Row.of(false), Row.of((Object) null));
41+
}
42+
43+
@Test
44+
void testNot() {
45+
createSimpleBoundedValuesTable("tblLogicalNot", "c boolean", truthTableRows());
46+
String query = "select NOT(c) from tblLogicalNot";
47+
runAndCheck(query, Arrays.asList("+I[false]", "+I[true]", "+I[null]"));
48+
}
49+
50+
@Test
51+
void testIsTrue() {
52+
createSimpleBoundedValuesTable("tblLogicalIsTrue", "c boolean", truthTableRows());
53+
String query = "select c IS TRUE from tblLogicalIsTrue";
54+
runAndCheck(query, Arrays.asList("+I[true]", "+I[false]", "+I[false]"));
55+
}
56+
57+
@Test
58+
void testIsFalse() {
59+
createSimpleBoundedValuesTable("tblLogicalIsFalse", "c boolean", truthTableRows());
60+
String query = "select c IS FALSE from tblLogicalIsFalse";
61+
runAndCheck(query, Arrays.asList("+I[false]", "+I[true]", "+I[false]"));
62+
}
63+
64+
@Test
65+
void testIsNotTrue() {
66+
createSimpleBoundedValuesTable("tblLogicalIsNotTrue", "c boolean", truthTableRows());
67+
String query = "select c IS NOT TRUE from tblLogicalIsNotTrue";
68+
runAndCheck(query, Arrays.asList("+I[false]", "+I[true]", "+I[true]"));
69+
}
70+
71+
@Test
72+
void testIsNotFalse() {
73+
createSimpleBoundedValuesTable("tblLogicalIsNotFalse", "c boolean", truthTableRows());
74+
String query = "select c IS NOT FALSE from tblLogicalIsNotFalse";
75+
runAndCheck(query, Arrays.asList("+I[true]", "+I[false]", "+I[true]"));
76+
}
77+
78+
@Test
79+
void testNestedLogical() {
80+
createSimpleBoundedValuesTable("tblLogicalNested", "c boolean", truthTableRows());
81+
// NOT(c IS FALSE): IS FALSE never returns NULL, so NOT sees
82+
// {false,true,false} -> {true,false,true}.
83+
// (NOT(c)) IS NOT TRUE (parens required: IS binds tighter than NOT, so
84+
// NOT(c) IS NOT TRUE parses as NOT(c IS NOT TRUE)): NOT preserves NULL,
85+
// then IS NOT TRUE maps {false,true,NULL} -> {true,false,true}, never
86+
// NULL.
87+
// NOT(c) IS NOT TRUE parses as NOT(c IS NOT TRUE); c IS NOT TRUE is
88+
// never NULL, so NOT flips {false,true,true} -> {true,false,false}.
89+
String query =
90+
"select NOT(c IS FALSE), (NOT(c)) IS NOT TRUE, NOT(c) IS NOT TRUE from tblLogicalNested";
91+
runAndCheck(
92+
query,
93+
Arrays.asList("+I[true, true, true]", "+I[false, false, false]", "+I[true, true, false]"));
94+
}
95+
96+
@Test
97+
void testConstantOperands() {
98+
createSimpleBoundedValuesTable("tblLogicalConst", "c boolean", truthTableRows());
99+
// Constant and constant-foldable operands: a NULL constant operand still
100+
// yields FALSE under IS TRUE, never NULL.
101+
String query =
102+
"select NOT(true), NOT(false), true IS TRUE, false IS FALSE, "
103+
+ "CAST(NULL AS BOOLEAN) IS TRUE, NOT(1 = 1) from tblLogicalConst";
104+
runAndCheck(
105+
query,
106+
Arrays.asList(
107+
"+I[false, true, true, true, false, false]",
108+
"+I[false, true, true, true, false, false]",
109+
"+I[false, true, true, true, false, false]"));
110+
}
111+
112+
@Test
113+
void testLogicalFilter() {
114+
createSimpleBoundedValuesTable("tblLogicalFilter", "c boolean", truthTableRows());
115+
// IS NOT FALSE passes TRUE and UNKNOWN, rejects FALSE.
116+
String query = "select c from tblLogicalFilter where c IS NOT FALSE";
117+
runAndCheck(query, Arrays.asList("+I[true]", "+I[null]"));
118+
}
119+
}

0 commit comments

Comments
 (0)