Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22322 )
Change subject: IMPALA-13658: Enable tuple caching aggregates ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/22322/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22322/1//COMMIT_MSG@9 PS1, Line 9: Enables tuple caching on aggregates. Caching aggregates requires that > Which aggregates are eligible? Should be any aggregate that does not involv I thought the next sentence covered that. I can try to reword a bit to match how you described it. http://gerrit.cloudera.org:8080/#/c/22322/1/fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java File fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java: http://gerrit.cloudera.org:8080/#/c/22322/1/fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java@227 PS1, Line 227: } > Should we test ineligible grouping and aggregate expressions? The way our testing currently works, I'm verifying caching by counting the number of tuple cache nodes. Some of that testing is implicit in the counts above in that they might be multi-phase aggregates but only 2 tuple cache nodes (above scan and first aggregate). I'll add tests: - where there's no aggregate below an exchange - where the aggregate is above a union I could also look into making the test verification more specific, but I don't have concrete ideas about that yet. -- To view, visit http://gerrit.cloudera.org:8080/22322 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9bd13c2813c90d23eb3a70f98068fdcdab97a885 Gerrit-Change-Number: 22322 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Fri, 10 Jan 2025 21:46:47 +0000 Gerrit-HasComments: Yes
