You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Anonymous Coward (Code Review)" <ge...@cloudera.org> on 2023/04/27 15:22:47 UTC

[Impala-ASF-CR] IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

lipenglin@apache.org has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19811


Change subject: IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table
......................................................................

IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

When optimizing the simple count start query for the Iceberg table, the
WITH CLAUSE should be skipped, but that doesn't mean the SQL can't be
optimized, because when the WITH CLAUSE is inlined, the final statement
is optimized by the CountStarToConstRule.

Testing:
 * Add e2e tests

Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-plain-count-star-optimization.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-plain-count-star-optimization.test
3 files changed, 59 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Gerrit-Change-Number: 19811
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <li...@apache.org>

[Impala-ASF-CR] IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

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

Change subject: IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Gerrit-Change-Number: 19811
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Apr 2023 15:43:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
lipenglin@apache.org has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/19811 )

Change subject: IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table
......................................................................

IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

When optimizing the simple count star query for the Iceberg table, the
WITH CLAUSE should be skipped, but that doesn't mean the SQL can't be
optimized, because when the WITH CLAUSE is inlined, the final statement
is optimized by the CountStarToConstRule.

Testing:
 * Add e2e tests

Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-plain-count-star-optimization.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-plain-count-star-optimization.test
4 files changed, 173 insertions(+), 3 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Gerrit-Change-Number: 19811
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

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

Change subject: IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table
......................................................................


Patch Set 1: Code-Review+1

(3 comments)

A few minor comments below.

http://gerrit.cloudera.org:8080/#/c/19811/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19811/1//COMMIT_MSG@9
PS1, Line 9: start
nit: 'star' instead of 'start'


http://gerrit.cloudera.org:8080/#/c/19811/1//COMMIT_MSG@15
PS1, Line 15:  * Add e2e tests
Were you also planning to add a planner test for the WITH clause ?  It would be useful to have 1 such test.


http://gerrit.cloudera.org:8080/#/c/19811/1/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/19811/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1437
PS1, Line 1437: optimizePlainCountStarQuery
nit: Since this function is specifically meant for Iceberg tables, can we rename this to optimizePlainCountStarQueryForIceberg() ? It is slightly verbose but makes the intent clearer to avoid confusion.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Gerrit-Change-Number: 19811
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Apr 2023 19:01:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
lipenglin@apache.org has posted comments on this change. ( http://gerrit.cloudera.org:8080/19811 )

Change subject: IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table
......................................................................


Patch Set 1:

(4 comments)

Thanks for comments!

http://gerrit.cloudera.org:8080/#/c/19811/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19811/1//COMMIT_MSG@9
PS1, Line 9: start
> nit: 'star' instead of 'start'
Done


http://gerrit.cloudera.org:8080/#/c/19811/1//COMMIT_MSG@15
PS1, Line 15:  * Add e2e tests
> Were you also planning to add a planner test for the WITH clause ?  It woul
The v1 table according to NumRowGroups or NumOrcStripes=0 is fine, I added the planner test for the v2 table.


http://gerrit.cloudera.org:8080/#/c/19811/1/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/19811/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1437
PS1, Line 1437: optimizePlainCountStarQuery
> nit: Since this function is specifically meant for Iceberg tables, can we r
Done


http://gerrit.cloudera.org:8080/#/c/19811/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1438
PS1, Line 1438: start
> nit: star
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Gerrit-Change-Number: 19811
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 13:42:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

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

Change subject: IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Gerrit-Change-Number: 19811
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 29 Apr 2023 02:35:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

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

Change subject: IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Gerrit-Change-Number: 19811
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 21:15:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

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

Change subject: IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Gerrit-Change-Number: 19811
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 12:02:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

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

Change subject: IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table
......................................................................

IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

When optimizing the simple count star query for the Iceberg table, the
WITH CLAUSE should be skipped, but that doesn't mean the SQL can't be
optimized, because when the WITH CLAUSE is inlined, the final statement
is optimized by the CountStarToConstRule.

Testing:
 * Add e2e tests

Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Reviewed-on: http://gerrit.cloudera.org:8080/19811
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 testdata/workloads/functional-planner/queries/PlannerTest/iceberg-v2-tables.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-plain-count-star-optimization.test
M testdata/workloads/functional-query/queries/QueryTest/iceberg-v2-plain-count-star-optimization.test
4 files changed, 173 insertions(+), 3 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Gerrit-Change-Number: 19811
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/19811 )

Change subject: IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

Thanks for fixing this so quickly!

http://gerrit.cloudera.org:8080/#/c/19811/1/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/19811/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1438
PS1, Line 1438: start
nit: star



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Gerrit-Change-Number: 19811
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 10:07:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

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

Change subject: IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Gerrit-Change-Number: 19811
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 21:14:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table

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

Change subject: IMPALA-12097: WITH CLAUSE should be skipped when optimizing COUNT(*) query on Iceberg table
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b21cbea79be77f2ea8490bd7f7b2f62063eb0e4
Gerrit-Change-Number: 19811
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <li...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Apr 2023 21:15:48 +0000
Gerrit-HasComments: No