Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/24062 )
Change subject: IMPALA-14116: Skip NULL in an IN-list against a column of an ORC table ...................................................................... Patch Set 8: (3 comments) > Patch Set 8: > > > Patch Set 7: > > > > > Patch Set 7: > > > > > > > Patch Set 7: > > > > > > > > (1 comment) > > > > > > > > I have addressed the comment on patch set 5. Let me know if there are > > > > additional suggestions. Thanks! > > > > > > Thanks a lot for the detailed analysis at > > > https://issues.apache.org/jira/browse/IMPALA-14116?focusedCommentId=18063966&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-18063966 > > > Quanlong! > > > > > > Let me know how you would like to fix the issue (via the change to > > > HdfsScanNode.java, or the change to > > > HdfsOrcScanner::GetSearchArgumentLiteral()). > > > > I think both sides should be fixed. GetSearchArgumentLiteral() is used in > > many other places, not just for in-list predicate. Probably we can add more > > regression tests on it. > > Thanks Quanlong! > > I looked around and found that GetSearchArgumentLiteral() seems to be used in > PrepareBinaryPredicate() and PrepareInListPredicate() only, and that Impala's > front-end already filters out the NULL literal in > tryComputeBinaryStatsPredicate() at > https://github.com/apache/impala/blob/ef2d50e/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java#L720. > So I guess we could just focus on the IN-list in the patch? > > Let me know if I missed something important. > > private void tryComputeBinaryStatsPredicate(Analyzer analyzer, > BinaryPredicate binaryPred) { > ... > if (Expr.IS_NULL_VALUE.apply(constExpr)) return; > ... > } Yeah, it seems we can't test them in e2e tests now since NULLs are also skipped in the IN-list. I'm OK with the current test coverage. Just have some minor comments. http://gerrit.cloudera.org:8080/#/c/24062/8/be/src/exec/orc/hdfs-orc-scanner.cc File be/src/exec/orc/hdfs-orc-scanner.cc: http://gerrit.cloudera.org:8080/#/c/24062/8/be/src/exec/orc/hdfs-orc-scanner.cc@1124 PS8, Line 1124: nit: pls remove extra space http://gerrit.cloudera.org:8080/#/c/24062/8/be/src/exec/orc/hdfs-orc-scanner.cc@1183 PS8, Line 1183: return orc::Literal(predicate_type); Let's fix this as well. http://gerrit.cloudera.org:8080/#/c/24062/8/be/src/exec/orc/hdfs-orc-scanner.cc@1213 PS8, Line 1213: if (dst_ptr == nullptr) return orc::Literal(predicate_type); This also need a fix. -- To view, visit http://gerrit.cloudera.org:8080/24062 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id62a631e5aa97132afbe0b184d427ad6bc1a4ad0 Gerrit-Change-Number: 24062 Gerrit-PatchSet: 8 Gerrit-Owner: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Mon, 23 Mar 2026 05:16:58 +0000 Gerrit-HasComments: Yes
