sunshone commented on PR #13461:
URL: https://github.com/apache/gluten/pull/13461#issuecomment-6495587526
@ggjh-159 Thanks for the thorough review!
1.Added LogicalFunctionsTest in gluten-flink/ut (the ScalarFunctionsTest
paradigm: bounded values table + runAndCheck changelog comparison), covering
a BOOLEAN column containing NULLs:
- three-valued truth tables for all operators: NOT preserves NULL; IS TRUE /
IS FALSE / IS NOT TRUE / IS NOT FALSE never return NULL;
- nested forms NOT(c1 IS FALSE) and (NOT(c1)) IS NOT TRUE. Note that IS binds
tighter than NOT in SQL, so NOT(c1) IS NOT TRUE parses as
NOT(c1 IS NOT TRUE); both nested forms are covered and commented in the
test;
- constant and constant-foldable operands, including
CAST(NULL AS BOOLEAN) IS TRUE (asserting FALSE, never NULL);
- WHERE-clause filter cases (c IS TRUE, NOT(c)).
8/8 green; ScalarFunctionsTest regression 20/20. Additionally, a cluster
dual-run (native Flink vs GlutenFlink, 13 cases including regressions for
already-mapped expressions) folds the print-sink changelogs back to final
result sets and matches exactly on every case.
The inline comment on RexCallConverterFactory.java is covered as well: the
IS TRUE entry maps to the flinksql is_true function now, and IS TRUE is
included in the tests above with NULL input asserting FALSE, never NULL —
see the inline reply for details. Commit: d98b36423.
2.The workflow now clones the velox4j branch carrying the velox ref bump for
this change — https://github.com/sunshone/velox4j.git at
flinksql-logical-expressions — pinned to
3eb04bd378097390225d2957faf789afdff11f36. git apply --check confirms
gluten-flink/patches/fix-velox4j.patch applies cleanly on that head.
A TODO comment records the agreed revert to bigo-sg/velox4j's gluten-20260829
branch (pin 2d08448f556bff669fe413874d04dd8cd96f65b2, the branch head after
the upstream IFNULL ref bump) once the velox-side flinksql functions land
there.
The inline comment on RexCallConverterFactory.java is addressed in the same
commit as well: the IS TRUE entry now maps to the flinksql is_true function
instead of equalto(x, true), so a NULL input evaluates to FALSE rather than
NULL — see the inline reply for details. Commit: d98b36423.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
