Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21484 )
Change subject: IMPALA-12800: Add cache for isTrueWithNullSlots() evaluation ...................................................................... Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/21484/9//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21484/9//COMMIT_MSG@7 PS9, Line 7: IMPALA-12800: Add null slots cache > nit: Can we use a better title, e.g. "Add cache for isTrueWithNullSlots() e Done http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2981 PS8, Line 2981: return globalState_.nullSlotsCache.get(nullTuplePred, > I think we can improve the hash key (maybe in a follow-up JIRA). Currently, https://gerrit.cloudera.org/c/21483/6 implemented appropriate hashCodes for all Exprs. http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2982 PS8, Line 2982: .booleanValue() > nit: Unnecessary unboxing. Need 4 spaces indent. Done http://gerrit.cloudera.org:8080/#/c/21484/8/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2984 PS8, Line 2984: Preconditions.checkState(e.getCause() instanceof InternalException); > nit: can we add an error message for this? I'd rather not. This is preserving the original error message. guava Cache#get wraps any Throwable thrown by the loader function in an ExecutionException. I'm unwrapping it to throw the exception we originally threw. -- To view, visit http://gerrit.cloudera.org:8080/21484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib63f5553284f21f775d2097b6c5d6bbb63699acd Gerrit-Change-Number: 21484 Gerrit-PatchSet: 10 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Tue, 11 Jun 2024 16:29:11 +0000 Gerrit-HasComments: Yes
