You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2025/03/25 23:17:07 UTC
[Impala-ASF-CR] IMPALA-13901: Make histogram/sample/group concat ineligible for tuple caching
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22668
Change subject: IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
......................................................................
IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
"histogram" and "sample" rely on the same code as "appx_median"
and incorporate randomness. "group_concat" is entirely dependent
on the order of inputs. This expands tuple caching's list of
ineligible functions to include these variable aggregate functions.
Testing:
- Added test cases to TupleCacheTest for these functions
Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java
2 files changed, 14 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/22668/1
--
To view, visit http://gerrit.cloudera.org:8080/22668
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Gerrit-Change-Number: 22668
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-13901: Make histogram/sample/group concat ineligible for tuple caching
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/22668 )
Change subject: IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22668/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22668/1//COMMIT_MSG@12
PS1, Line 12: ineligible functions to include these variable aggregate functions.
Why are these cases problematic? We should provide a way to enable caching for non-deterministic queries that have a legal exemplar result.
--
To view, visit http://gerrit.cloudera.org:8080/22668
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Gerrit-Change-Number: 22668
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 08 Apr 2025 13:50:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13901: Make histogram/sample/group concat ineligible for tuple caching
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22668 )
Change subject: IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/22668
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Gerrit-Change-Number: 22668
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 18 Jun 2025 23:31:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13901: Make histogram/sample/group concat ineligible for tuple caching
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22668 )
Change subject: IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/22668
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Gerrit-Change-Number: 22668
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 18 Jun 2025 17:27:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13901: Make histogram/sample/group concat ineligible for tuple caching
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22668 )
Change subject: IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/19531/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22668
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Gerrit-Change-Number: 22668
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 18 Jun 2025 17:54:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13901: Make histogram/sample/group concat ineligible for tuple caching
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/22668 )
Change subject: IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
......................................................................
IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
"histogram" and "sample" rely on the same code as "appx_median"
and incorporate randomness. "group_concat" is entirely dependent
on the order of inputs. This expands tuple caching's list of
ineligible functions to include these variable aggregate functions.
Testing:
- Added test cases to TupleCacheTest for these functions
Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Reviewed-on: http://gerrit.cloudera.org:8080/22668
Reviewed-by: Michael Smith <mi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Yida Wu <wy...@gmail.com>
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java
2 files changed, 13 insertions(+), 4 deletions(-)
Approvals:
Michael Smith: Looks good to me, but someone else must approve
Impala Public Jenkins: Verified
Yida Wu: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/22668
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Gerrit-Change-Number: 22668
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-13901: Make histogram/sample/group concat ineligible for tuple caching
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22668 )
Change subject: IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/12051/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/22668
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Gerrit-Change-Number: 22668
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 18 Jun 2025 17:26:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13901: Make histogram/sample/group concat ineligible for tuple caching
Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/22668 )
Change subject: IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/22668
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Gerrit-Change-Number: 22668
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Jun 2025 06:34:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13901: Make histogram/sample/group concat ineligible for tuple caching
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22668 )
Change subject: IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18695/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22668
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Gerrit-Change-Number: 22668
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Mar 2025 23:45:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13901: Make histogram/sample/group concat ineligible for tuple caching
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22668 )
Change subject: IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/22668
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Gerrit-Change-Number: 22668
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Mar 2025 18:07:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13901: Make histogram/sample/group concat ineligible for tuple caching
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Yida Wu, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22668
to look at the new patch set (#3).
Change subject: IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
......................................................................
IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
"histogram" and "sample" rely on the same code as "appx_median"
and incorporate randomness. "group_concat" is entirely dependent
on the order of inputs. This expands tuple caching's list of
ineligible functions to include these variable aggregate functions.
Testing:
- Added test cases to TupleCacheTest for these functions
Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
---
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/test/java/org/apache/impala/planner/TupleCacheTest.java
2 files changed, 13 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/22668/3
--
To view, visit http://gerrit.cloudera.org:8080/22668
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Gerrit-Change-Number: 22668
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
[Impala-ASF-CR] IMPALA-13901: Make histogram/sample/group concat ineligible for tuple caching
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/22668 )
Change subject: IMPALA-13901: Make histogram/sample/group_concat ineligible for tuple caching
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22668/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22668/1//COMMIT_MSG@12
PS1, Line 12: ineligible functions to include these variable aggregate functions.
> Why are these cases problematic? We should provide a way to enable caching
There are two problems:
1. When a non-deterministic result passes through an exchange, it can be an input into cache entries on all the nodes past the exchange. Nodes can evict cache entries at different times, so we could end up having a cache hit on node A but a cache miss on node B. In order to get a sensible result, the recomputed result used on node B needs to match what A had. Non-deterministic functions can't do that.
2. These functions create noise for automated correctness checking. We don't have a good way to handle them other than turning it off for those locations.
I will file a JIRA to do more sophisticated handling for non-determinism, so that we can cache in legal locations with a non-deterministic result (i.e. locations that aren't subject to the concern in #1). Until that gets done, the functions should be ineligible to avoid impacts to correctness checking.
--
To view, visit http://gerrit.cloudera.org:8080/22668
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb017dd4297a3bb219ffed2bd9abc94e6c5b2605
Gerrit-Change-Number: 22668
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Thu, 22 May 2025 00:25:38 +0000
Gerrit-HasComments: Yes