You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Paul Rogers (Code Review)" <ge...@cloudera.org> on 2018/11/27 01:56:23 UTC

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Paul Rogers has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11993


Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................

IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Expected results from spillable-bufe-sizing.test and max-row-size.test
included incorrect expressions: the un-anayzed GROUP BY expression with
ordinals represented as (invalid) casts.

SelectStmt is a bit of a mess. There are two copies of the grouping
expressions. Here we want to use the analyzed version with the ordinals
replaced.

Testing: problem found when running PlannerTest. PlannerTest now passes,
with correct results, after this change.

Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
3 files changed, 14 insertions(+), 14 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/11993/1
-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Nov 2018 18:19:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................


Patch Set 4: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Nov 2018 01:27:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................


Patch Set 2: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3496/


-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Nov 2018 22:19:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3503/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Nov 2018 01:27:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3496/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Nov 2018 18:19:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1441/ : 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/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Nov 2018 02:22:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Nov 2018 01:26:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Nov 2018 18:19:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1462/ : 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/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Nov 2018 01:23:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

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/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................

IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Expected results from spillable-bufe-sizing.test and max-row-size.test
included incorrect expressions: the un-anayzed GROUP BY expression with
ordinals represented as (invalid) casts.

SelectStmt is a bit of a mess. There are two copies of the grouping
expressions. Here we want to use the analyzed version with the ordinals
replaced.

Testing:

* Problem found when running PlannerTest. PlannerTest now passes,
  with correct results, after this change.
* Turns out there is another path used when generating SQL for a view
  which does toSql() on an unanalyzed query. Added unit tests for this
  case.

Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Reviewed-on: http://gerrit.cloudera.org:8080/11993
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
A fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
4 files changed, 88 insertions(+), 14 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 5
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................


Patch Set 4: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Nov 2018 05:16:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................


Patch Set 3:

Tim, thanks for the review and for pointing out the test failure.

Tracked down the bug. We have a whole pile of "to sql" code in the FE, called from the BE, without unit tests. This code works with an AST without analysis, which triggered the NPE we saw earlier. The new patch includes a starter test. I'll add others in a separate patch.

Please take a second look when you get a chance. Thanks!


-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Nov 2018 00:55:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11993 )

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................


Patch Set 2:

Looks like test_show_create_table failed with a NPE


-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Nov 2018 23:26:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/11993

to look at the new patch set (#3).

Change subject: IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test
......................................................................

IMPALA-7895: Incorrect expected results for spillable-buffer-sizing.test

Expected results from spillable-bufe-sizing.test and max-row-size.test
included incorrect expressions: the un-anayzed GROUP BY expression with
ordinals represented as (invalid) casts.

SelectStmt is a bit of a mess. There are two copies of the grouping
expressions. Here we want to use the analyzed version with the ordinals
replaced.

Testing:

* Problem found when running PlannerTest. PlannerTest now passes,
  with correct results, after this change.
* Turns out there is another path used when generating SQL for a view
  which does toSql() on an unanalyzed query. Added unit tests for this
  case.

Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
A fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
4 files changed, 88 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/11993/3
-- 
To view, visit http://gerrit.cloudera.org:8080/11993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I413bded920e27fe9f41f0ea989696a0c8f92fe4a
Gerrit-Change-Number: 11993
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pa...@yahoo.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>