You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Kim Jin Chul (Code Review)" <ge...@cloudera.org> on 2017/11/21 03:31:36 UTC
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Kim Jin Chul has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8614
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
---
M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/8614/1
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 1
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8614 )
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8614/1/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/8614/1/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@246
PS1, Line 246: LOG.info("Replace " + distinctAggExprs.get(0).toSql()
include this in the exception message, most users will not dig into the logs and many users do not even have access to them
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 1
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Comment-Date: Tue, 21 Nov 2017 22:57:49 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Kim Jin Chul (Code Review)" <ge...@cloudera.org>.
Hello Alex Behm,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8614
to look at the new patch set (#3).
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
---
M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
1 file changed, 12 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/8614/3
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 3
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Kim Jin Chul (Code Review)" <ge...@cloudera.org>.
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8614 )
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/8614/2/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/8614/2/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@250
PS2, Line 250: + "Replace " + distinctAggExprs.get(0).toSql()
> * Just replacing one function may not work, so suggesting to replace a sing
Please see : https://gerrit.cloudera.org/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
http://gerrit.cloudera.org:8080/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@249
PS3, Line 249: + "; deviating function: " + distinctAggExprs.get(i).toSql() + "\n"
: + "Replace
Conversion to NDV() is only available for the case: COUNT(DISTINCT).
http://gerrit.cloudera.org:8080/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@252
PS3, Line 252: + ") if estimated counts are okay. Enable the APPX_COUNT_DISTINCT"
: + " query option to perform this rewrite automatically.");
I think it is better to show a specific object instead of constant.
Do you still prefer the constant general string like COUNT(DISTINCT)?
> select count(distinct x), count(distinct y), count(distinct z) from t3;
ERROR: AnalysisException: All DISTINCT aggregate functions need to have the same set of parameters as count(DISTINCT x); deviating function: count(DISTINCT y)
Consider using NDV(y) instead of count(DISTINCT y) if estimated counts are acceptable. Enable the APPX_COUNT_DISTINCT query option to perform this rewrite automatically.
> select count(distinct x), ndv(distinct y), count(distinct z) from t3;
ERROR: AnalysisException: All DISTINCT aggregate functions need to have the same set of parameters as count(DISTINCT x); deviating function: count(DISTINCT z)
Consider using NDV(z) instead of count(DISTINCT z) if estimated counts are acceptable. Enable the APPX_COUNT_DISTINCT query option to perform this rewrite automatically.
> select count(distinct x), ndv(distinct y), ndv(distinct z) from t3;
--> works
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 2
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Nov 2017 05:20:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Kim Jin Chul (Code Review)" <ge...@cloudera.org>.
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8614 )
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@249
PS3, Line 249: if (distinctAggExprs.get(i).getFnName().getFunction().equalsIgnoreCase(
: "count")) {
> That's correct, but that error message is also thrown for non-count(distinc
1. Do you mean the guidance of NDV is also printed on your example? Why should we do this? If a given query does not have any use case of COUNT(DISTINCT), we don't have to display this. It could make an user confuse. May I know your intention why we should include this?
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 3
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Nov 2017 05:48:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8614 )
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 4
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Nov 2017 20:45:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8614 )
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Reviewed-on: http://gerrit.cloudera.org:8080/8614
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
1 file changed, 4 insertions(+), 1 deletion(-)
Approvals:
Alex Behm: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 5
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8614 )
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8614/2/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/8614/2/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@250
PS2, Line 250: + "Replace " + distinctAggExprs.get(0).toSql()
* Just replacing one function may not work, so suggesting to replace a single specific one does not seem right.
* The aggregate functions might not be count() so the error message might be misleading (e.g. avg(distinct x), avg(distinct y))
* Can you rephrase the error to be more generic and not specific to one function? Something like:
Consider using NDV() instead of COUNT(DISTINCT) if estimated counts are acceptable. Enable the APPX_COUNT_DISTINCT query option to perform this rewrite automatically.
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 2
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Nov 2017 04:49:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8614 )
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 4
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Comment-Date: Thu, 23 Nov 2017 00:21:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8614 )
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1516/
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 4
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Nov 2017 20:46:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8614 )
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@249
PS3, Line 249: if (distinctAggExprs.get(i).getFnName().getFunction().equalsIgnoreCase(
: "count")) {
> Conversion to NDV() is only available for the case: COUNT(DISTINCT).
That's correct, but that error message is also thrown for non-count(distinct) aggregation functions like:
select avg(distinct int_col), group_concat(distinct string_col) from functional.alltypes
http://gerrit.cloudera.org:8080/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@252
PS3, Line 252: + distinctAggExprs.get(i).getChild(0).toSql()
: + ") instead of " + distinctAggExprs.get(i).toSql() + " if estimated "
> I think it is better to show a specific object instead of constant.
What about this:
select avg(distinct x), group_concat(distinct y)
I still prefer my proposed version with a generic count(distinct)
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 3
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Nov 2017 05:30:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Kim Jin Chul (Code Review)" <ge...@cloudera.org>.
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8614 )
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@249
PS3, Line 249: if (distinctAggExprs.get(i).getFnName().getFunction().equalsIgnoreCase(
: "count")) {
> In my example, it does not make sense to print an error message that uses N
Okay, let me reflect your proposal.
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 3
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Nov 2017 08:01:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Kim Jin Chul (Code Review)" <ge...@cloudera.org>.
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8614 )
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8614/1/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/8614/1/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@246
PS1, Line 246: throw new AnalysisException(
> include this in the exception message, most users will not dig into the log
Done
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 2
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Nov 2017 01:27:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Kim Jin Chul (Code Review)" <ge...@cloudera.org>.
Hello Alex Behm,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8614
to look at the new patch set (#2).
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
---
M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
1 file changed, 6 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/8614/2
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 2
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8614 )
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java:
http://gerrit.cloudera.org:8080/#/c/8614/3/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@249
PS3, Line 249: if (distinctAggExprs.get(i).getFnName().getFunction().equalsIgnoreCase(
: "count")) {
> 1. Do you mean the guidance of NDV is also printed on your example? Why sho
In my example, it does not make sense to print an error message that uses NDV() on a specific child of an existing aggregate function like in your current error message.
I see two options:
(1) Only print the NDV() recommendation if one of the distinctAggExprs is a COUNT(). Give a specific recommendation which aggregate expr to rewrite.
(2) Always print the NDV() workaround using a generic error message like the one I proposed
I don't like option (1) because even if you convert all COUNT(DISTINCT) to NDV() you might still not be able to run the query. Example:
select count(distinct a), count(distinct b), avg(distinct c), avg(distinct d) ...
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 3
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Comment-Date: Wed, 22 Nov 2017 06:02:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Posted by "Kim Jin Chul (Code Review)" <ge...@cloudera.org>.
Hello Alex Behm,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8614
to look at the new patch set (#4).
Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
......................................................................
IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
---
M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java
1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/8614/4
--
To view, visit http://gerrit.cloudera.org:8080/8614
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89
Gerrit-Change-Number: 8614
Gerrit-PatchSet: 4
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>