You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Daniel Becker (Code Review)" <ge...@cloudera.org> on 2024/09/30 12:57:13 UTC

[Impala-ASF-CR] IMPALA-13373: Analytic query with struct from view fails

Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21824


Change subject: IMPALA-13373: Analytic query with struct from view fails
......................................................................

IMPALA-13373: Analytic query with struct from view fails

The following fails with a frontend exception:
  CREATE TABLE tbl (
    s STRUCT<id:STRING,type:STRING, b:BOOLEAN>
  ) STORED AS PARQUET;

  CREATE VIEW view_tbl AS SELECT s FROM tbl;

  SELECT MAX(view_tbl.s.type) OVER (PARTITION BY view_tbl.s.id) AS type
  FROM view_tbl;

The root cause is that in some places, structs are not handled
recursively when they should be. This change introduces recursive
handling in the following places:
 - SlotDescriptor copying constructor
 - AnalyticPlanner.createSortInfo()
 - SortNode.init()

The last case is about adding structs to smaps, which has already been
done recursively for InlineViewRef. To reuse the code, it has been
extracted into a new class, RecursiveExprSubstitutionMapInserter.

Testing:
 - added regression tests in struct-in-select-list.test

Change-Id: I6ecd79ba00396fb557581514462af86f2f450c86
---
M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
A fe/src/main/java/org/apache/impala/analysis/RecursiveExprSubstitutionMapInserter.java
M fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test
9 files changed, 438 insertions(+), 176 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/21824/6
-- 
To view, visit http://gerrit.cloudera.org:8080/21824
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6ecd79ba00396fb557581514462af86f2f450c86
Gerrit-Change-Number: 21824
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13373: Analytic query with struct from view fails

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

Change subject: IMPALA-13373: Analytic query with struct from view fails
......................................................................


Patch Set 7:

(2 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/21824/7/fe/src/main/java/org/apache/impala/analysis/RecursiveExprSubstitutionMapInserter.java
File fe/src/main/java/org/apache/impala/analysis/RecursiveExprSubstitutionMapInserter.java:

http://gerrit.cloudera.org:8080/#/c/21824/7/fe/src/main/java/org/apache/impala/analysis/RecursiveExprSubstitutionMapInserter.java@81
PS7, Line 81:         // https://github.com/apache/impala/blob/58fd45f20c22f8a062f603d6b96f62ee57d85ca9/fe/src/main/java/org/apache/impala/analysis/Analyzer.java#L1640
line too long (153 > 90)


http://gerrit.cloudera.org:8080/#/c/21824/7/fe/src/main/java/org/apache/impala/analysis/RecursiveExprSubstitutionMapInserter.java@88
PS7, Line 88:         Preconditions.checkState(baseTableExpr instanceof SlotRef || baseTableExpr == null);
line too long (92 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ecd79ba00396fb557581514462af86f2f450c86
Gerrit-Change-Number: 21824
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 Sep 2025 14:04:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13373: Analytic query with struct from view fails

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

Change subject: IMPALA-13373: Analytic query with struct from view fails
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ecd79ba00396fb557581514462af86f2f450c86
Gerrit-Change-Number: 21824
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Sep 2024 13:30:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13373: Analytic query with struct from view fails

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/21824 )

Change subject: IMPALA-13373: Analytic query with struct from view fails
......................................................................

IMPALA-13373: Analytic query with struct from view fails

The following fails with a frontend exception:
  CREATE TABLE tbl (
    s STRUCT<id:STRING,type:STRING, b:BOOLEAN>
  ) STORED AS PARQUET;

  CREATE VIEW view_tbl AS SELECT s FROM tbl;

  SELECT MAX(view_tbl.s.type) OVER (PARTITION BY view_tbl.s.id) AS type
  FROM view_tbl;

The root cause is that in some places, structs are not handled
recursively when they should be. This change introduces recursive
handling in the following places:
 - SlotDescriptor copying constructor
 - AnalyticPlanner.createSortInfo()
 - SortNode.init()

The last case is about adding structs to smaps, which has already been
done recursively for InlineViewRef. To reuse the code, it has been
extracted into a new class, RecursiveExprSubstitutionMapInserter.

Testing:
 - added regression tests in struct-in-select-list.test

Change-Id: I6ecd79ba00396fb557581514462af86f2f450c86
---
M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
A fe/src/main/java/org/apache/impala/analysis/RecursiveExprSubstitutionMapInserter.java
M fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M testdata/workloads/functional-query/queries/QueryTest/struct-in-select-list.test
9 files changed, 438 insertions(+), 176 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/24/21824/7
-- 
To view, visit http://gerrit.cloudera.org:8080/21824
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6ecd79ba00396fb557581514462af86f2f450c86
Gerrit-Change-Number: 21824
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13373: Analytic query with struct from view fails

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

Change subject: IMPALA-13373: Analytic query with struct from view fails
......................................................................


Patch Set 6:

(2 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python2 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with virtualenv installed.

http://gerrit.cloudera.org:8080/#/c/21824/6/fe/src/main/java/org/apache/impala/analysis/RecursiveExprSubstitutionMapInserter.java
File fe/src/main/java/org/apache/impala/analysis/RecursiveExprSubstitutionMapInserter.java:

http://gerrit.cloudera.org:8080/#/c/21824/6/fe/src/main/java/org/apache/impala/analysis/RecursiveExprSubstitutionMapInserter.java@81
PS6, Line 81:         // https://github.com/apache/impala/blob/58fd45f20c22f8a062f603d6b96f62ee57d85ca9/fe/src/main/java/org/apache/impala/analysis/Analyzer.java#L1640
line too long (153 > 90)


http://gerrit.cloudera.org:8080/#/c/21824/6/fe/src/main/java/org/apache/impala/analysis/RecursiveExprSubstitutionMapInserter.java@88
PS6, Line 88:         Preconditions.checkState(baseTableExpr instanceof SlotRef || baseTableExpr == null);
line too long (92 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ecd79ba00396fb557581514462af86f2f450c86
Gerrit-Change-Number: 21824
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Sep 2024 13:08:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13373: Analytic query with struct from view fails

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

Change subject: IMPALA-13373: Analytic query with struct from view fails
......................................................................


Patch Set 7:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ecd79ba00396fb557581514462af86f2f450c86
Gerrit-Change-Number: 21824
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 Sep 2025 14:30:30 +0000
Gerrit-HasComments: No