You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Aman Sinha (Code Review)" <ge...@cloudera.org> on 2021/12/06 08:08:19 UTC

[Impala-ASF-CR] IMPALA-11030: Fix incorrect creation of common partition exprs

Aman Sinha has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18072


Change subject: IMPALA-11030: Fix incorrect creation of common partition exprs
......................................................................

IMPALA-11030: Fix incorrect creation of common partition exprs

When there are 2 or more analytic functions in an inline view
and at least one of them does not have a partition-by expr,
we were previously still populating the commonPartitionExprs
list in AnalyticInfo. This common partition expr was then
used during the auxiliary predicate creation when the outer
query has a predicate on partition-by column. This leads to
wrong result because the auxiliary predicate is pushed down
to the table scan. While pushing down predicate on a
partitioning column is okay if all the analytic functions
contain that partitioning column, it is not correct to do
this when at least one analytic function does not have that
partitioning column.

This patch fixes the wrong result by ensuring that the
AnalyticInfo's commonPartitionExprs is empty if at least
one analytic function does not have partitioning exprs.

Testing:
 - Added new planner test and e2e test for analytic fns
 TODO: run all FE and e2e tests

Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
---
M fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
3 files changed, 68 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
Gerrit-Change-Number: 18072
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>

[Impala-ASF-CR] IMPALA-11030: Fix incorrect creation of common partition exprs

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18072 )

Change subject: IMPALA-11030: Fix incorrect creation of common partition exprs
......................................................................

IMPALA-11030: Fix incorrect creation of common partition exprs

When there are 2 or more analytic functions in an inline view
and at least one of them does not have a partition-by expr,
we were previously still populating the commonPartitionExprs
list in AnalyticInfo. This common partition expr was then
used during the auxiliary predicate creation when the outer
query has a predicate on partition-by column. This leads to
wrong result because the auxiliary predicate is pushed down
to the table scan. While pushing down predicate on a
partitioning column is okay if all the analytic functions
contain that partitioning column, it is not correct to do
this when at least one analytic function does not have that
partitioning column.

This patch fixes the wrong result by ensuring that the
AnalyticInfo's commonPartitionExprs is empty if at least
one analytic function does not have partitioning exprs.

Testing:
 - Added new planner test and e2e test for row_num
   analytic function

Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
Reviewed-on: http://gerrit.cloudera.org:8080/18072
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Aman Sinha <am...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
3 files changed, 68 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
Gerrit-Change-Number: 18072
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11030: Fix incorrect creation of common partition exprs

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

Change subject: IMPALA-11030: Fix incorrect creation of common partition exprs
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
Gerrit-Change-Number: 18072
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Dec 2021 08:31:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11030: Fix incorrect creation of common partition exprs

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has removed a vote on this change.

Change subject: IMPALA-11030: Fix incorrect creation of common partition exprs
......................................................................


Removed Verified-1 by Impala Public Jenkins <im...@cloudera.com>
-- 
To view, visit http://gerrit.cloudera.org:8080/18072
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
Gerrit-Change-Number: 18072
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11030: Fix incorrect creation of common partition exprs

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11030: Fix incorrect creation of common partition exprs
......................................................................

IMPALA-11030: Fix incorrect creation of common partition exprs

When there are 2 or more analytic functions in an inline view
and at least one of them does not have a partition-by expr,
we were previously still populating the commonPartitionExprs
list in AnalyticInfo. This common partition expr was then
used during the auxiliary predicate creation when the outer
query has a predicate on partition-by column. This leads to
wrong result because the auxiliary predicate is pushed down
to the table scan. While pushing down predicate on a
partitioning column is okay if all the analytic functions
contain that partitioning column, it is not correct to do
this when at least one analytic function does not have that
partitioning column.

This patch fixes the wrong result by ensuring that the
AnalyticInfo's commonPartitionExprs is empty if at least
one analytic function does not have partitioning exprs.

Testing:
 - Added new planner test and e2e test for row_num
   analytic function

Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
---
M fe/src/main/java/org/apache/impala/analysis/AnalyticInfo.java
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
M testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
3 files changed, 68 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/18072/2
-- 
To view, visit http://gerrit.cloudera.org:8080/18072
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
Gerrit-Change-Number: 18072
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11030: Fix incorrect creation of common partition exprs

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

Change subject: IMPALA-11030: Fix incorrect creation of common partition exprs
......................................................................


Patch Set 2: Code-Review+2

LGTM. Thanks for fixing this!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
Gerrit-Change-Number: 18072
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sun, 09 Jan 2022 00:04:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11030: Fix incorrect creation of common partition exprs

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

Change subject: IMPALA-11030: Fix incorrect creation of common partition exprs
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
Gerrit-Change-Number: 18072
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sun, 09 Jan 2022 02:12:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11030: Fix incorrect creation of common partition exprs

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

Change subject: IMPALA-11030: Fix incorrect creation of common partition exprs
......................................................................


Patch Set 3: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
Gerrit-Change-Number: 18072
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sun, 09 Jan 2022 08:48:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11030: Fix incorrect creation of common partition exprs

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

Change subject: IMPALA-11030: Fix incorrect creation of common partition exprs
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
Gerrit-Change-Number: 18072
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jan 2022 08:34:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11030: Fix incorrect creation of common partition exprs

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

Change subject: IMPALA-11030: Fix incorrect creation of common partition exprs
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
Gerrit-Change-Number: 18072
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sun, 09 Jan 2022 02:12:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11030: Fix incorrect creation of common partition exprs

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

Change subject: IMPALA-11030: Fix incorrect creation of common partition exprs
......................................................................


Patch Set 3: Verified+1

The 1 failure is unrelated.  It shows an error message related to the compiler cache in ubuntu (compiler cache is intended to improve compilation speed, should not affect functionality):
   Build command failed: ccache /home/ubuntu/Impala/toolchain/toolchain-packages-gcc7.5.0/gcc-7.5.0/bin/g++
   ....
   ccache: error: /home/ubuntu/.ccache/ccache.conf: No such file or directory


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iebb51f691e8e5459ffbaf5a49907140f2de212cc
Gerrit-Change-Number: 18072
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sun, 09 Jan 2022 15:38:14 +0000
Gerrit-HasComments: No