You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org> on 2018/06/05 10:19:28 UTC
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10605
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
Since IMPALA-6314 on runtime scalar subqueries we set LIMIT 2
in StmtRewriter.mergeExpr(). We do that because later we add a
CardinalityCheckNode on top of such subqueries and with
LIMIT 2 we can still check if they return more than one row.
In the constructor of CardinalityCheckNode there is a
precondition that checks if the child node has LIMIT 2 to
be certain that we've set the limit for all the necessary
cases.
However, some of these subqueries will get a LIMIT 1 later
breaking the precondition in CardinalityCheckNode. I changed
the precondition from
Preconditions.checkState(child.getLimit() == 2);
to
Preconditions.checkState(child.getLimit() <= 2);
in order to be more permissive.
I added tests for such subqueries.
Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
---
M fe/src/main/java/org/apache/impala/planner/CardinalityCheckNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-query/queries/QueryTest/subquery.test
3 files changed, 62 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/10605/1
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10605/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/10605/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1100
PS3, Line 1100: returns a single row
Does this one need to be changed to be inline with 4. above as well?
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Jun 2018 19:37:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Vincent Tran, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10605
to look at the new patch set (#4).
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
Since IMPALA-6314 on runtime scalar subqueries we set LIMIT 2
in StmtRewriter.mergeExpr(). We do that because later we add a
CardinalityCheckNode on top of such subqueries and with
LIMIT 2 we can still check if they return more than one row.
In the constructor of CardinalityCheckNode there is a
precondition that checks if the child node has LIMIT 2 to
be certain that we've set the limit for all the necessary
cases.
However, some subqueries will get a LIMIT 1 later breaking the
precondition in CardinalityCheckNode. An example to these
subqueries is a select stmt that selects from an inline view
that returns a single row:
select * from functional.alltypes
where int_col = (select f.id from (
select * from functional.alltypes limit 1) f);
Note that we shouldn't add a CardinalityCheckNode to the plan
of this query in the first place. To generate a proper plan I
updated SelectStmt.returnsSingleRow() because this method didn't
handle this case well.
I also changed
the precondition from
Preconditions.checkState(child.getLimit() == 2);
to
Preconditions.checkState(child.getLimit() <= 2);
in order to be more permissive.
I added tests for the aforementioned query.
Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/planner/CardinalityCheckNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-query/queries/QueryTest/subquery.test
4 files changed, 67 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/10605/4
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10605/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/10605/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1082
PS3, Line 1082: returns a single row.
I think this bit is still inaccurate.
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Jun 2018 19:15:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun-tarmstrong/33/
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 16:51:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Vincent Tran, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10605
to look at the new patch set (#3).
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
Since IMPALA-6314 on runtime scalar subqueries we set LIMIT 2
in StmtRewriter.mergeExpr(). We do that because later we add a
CardinalityCheckNode on top of such subqueries and with
LIMIT 2 we can still check if they return more than one row.
In the constructor of CardinalityCheckNode there is a
precondition that checks if the child node has LIMIT 2 to
be certain that we've set the limit for all the necessary
cases.
However, some subqueries will get a LIMIT 1 later breaking the
precondition in CardinalityCheckNode. An example to these
subqueries is a select stmt that selects from an inline view
that returns a single row:
select * from functional.alltypes
where int_col = (select f.id from (
select * from functional.alltypes limit 1) f);
Note that we shouldn't add a CardinalityCheckNode to the plan
of this query in the first place. To generate a proper plan I
updated SelectStmt.returnsSingleRow() because this method didn't
handle this case well.
I also changed
the precondition from
Preconditions.checkState(child.getLimit() == 2);
to
Preconditions.checkState(child.getLimit() <= 2);
in order to be more permissive.
I added tests for the aforementioned query.
Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/planner/CardinalityCheckNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-query/queries/QueryTest/subquery.test
4 files changed, 66 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/10605/3
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 2: Code-Review+1
LGTM
Tested against a few other variations of subqueries that return a single scalar values.
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Jun 2018 18:25:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 1:
Did you want to also include your other suggested fix too? It seems like that's more consistent with the original intent of the code.
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Jun 2018 16:21:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10605/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/10605/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1082
PS2, Line 1082: * Check if the stmt returns a single row. This can happen
> Should this be "returns at most one row"?
Fixed the comment.
I have similar comments in the tests but I didn't change them because the used queries do return a single row.
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Jun 2018 09:09:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10605/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/10605/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1082
PS3, Line 1082: returns at most one r
> I think this bit is still inaccurate.
Changed to 'returns at most one row'.
The name of the method is still inaccurate, but maybe it's okay with the rephrased comments.
http://gerrit.cloudera.org:8080/#/c/10605/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1100
PS3, Line 1100: returns at most one
> Does this one need to be changed to be inline with 4. above as well?
Right, thanks!
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 09:55:00 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 2:
Yeah I think it's worth to add both fixes.
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Jun 2018 12:27:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun-tarmstrong/33/
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 16:51:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 16:27:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
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/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
Since IMPALA-6314 on runtime scalar subqueries we set LIMIT 2
in StmtRewriter.mergeExpr(). We do that because later we add a
CardinalityCheckNode on top of such subqueries and with
LIMIT 2 we can still check if they return more than one row.
In the constructor of CardinalityCheckNode there is a
precondition that checks if the child node has LIMIT 2 to
be certain that we've set the limit for all the necessary
cases.
However, some subqueries will get a LIMIT 1 later breaking the
precondition in CardinalityCheckNode. An example to these
subqueries is a select stmt that selects from an inline view
that returns a single row:
select * from functional.alltypes
where int_col = (select f.id from (
select * from functional.alltypes limit 1) f);
Note that we shouldn't add a CardinalityCheckNode to the plan
of this query in the first place. To generate a proper plan I
updated SelectStmt.returnsSingleRow() because this method didn't
handle this case well.
I also changed
the precondition from
Preconditions.checkState(child.getLimit() == 2);
to
Preconditions.checkState(child.getLimit() <= 2);
in order to be more permissive.
I added tests for the aforementioned query.
Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Reviewed-on: http://gerrit.cloudera.org:8080/10605
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/planner/CardinalityCheckNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-query/queries/QueryTest/subquery.test
4 files changed, 67 insertions(+), 2 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10605/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:
http://gerrit.cloudera.org:8080/#/c/10605/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1082
PS2, Line 1082: * Check if the stmt returns a single row. This can happen
Should this be "returns at most one row"?
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Jun 2018 21:18:06 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 20:15:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10605 )
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 16:51:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Vincent Tran, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10605
to look at the new patch set (#2).
Change subject: IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
......................................................................
IMPALA-7108: IllegalStateException hit during CardinalityCheckNode.<init>
Since IMPALA-6314 on runtime scalar subqueries we set LIMIT 2
in StmtRewriter.mergeExpr(). We do that because later we add a
CardinalityCheckNode on top of such subqueries and with
LIMIT 2 we can still check if they return more than one row.
In the constructor of CardinalityCheckNode there is a
precondition that checks if the child node has LIMIT 2 to
be certain that we've set the limit for all the necessary
cases.
However, some subqueries will get a LIMIT 1 later breaking the
precondition in CardinalityCheckNode. An example to these
subqueries is a select stmt that selects from an inline view
that returns a single row:
select * from functional.alltypes
where int_col = (select f.id from (
select * from functional.alltypes limit 1) f);
Note that we shouldn't add a CardinalityCheckNode to the plan
of this query in the first place. To generate a proper plan I
updated SelectStmt.returnsSingleRow() because this method didn't
handle this case well.
I also changed
the precondition from
Preconditions.checkState(child.getLimit() == 2);
to
Preconditions.checkState(child.getLimit() <= 2);
in order to be more permissive.
I added tests for the aforementioned query.
Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/planner/CardinalityCheckNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-query/queries/QueryTest/subquery.test
4 files changed, 66 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/10605/2
--
To view, visit http://gerrit.cloudera.org:8080/10605
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I82a7a3fe26db3e12131c030c4ad055a9c4955407
Gerrit-Change-Number: 10605
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>