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 2022/11/02 16:20:26 UTC
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19190
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
IMPALA-11692: Struct slot memory sharing involving select * not working properly
With EXPAND_COMPLEX_TYPES=1, if there are structs coming from the star
expansion and members of the structs are also given explicitly, slot
memory sharing does not work in some cases:
explain select * from functional_orc_def.complextypes_nested_structs;
row-size=64B
explain select *, outer_struct.inner_struct1 from
functional_orc_def.complextypes_nested_structs;
row-size=80B
The row size should be the same in both cases as
outer_struct.inner_struct1 is part of outer_struct which is included in
the star.
This change modifies how star select list items are analysed. First,
before 'SelectAnalyzer.analyzeSelectClause()' is called, all star items
are expanded to paths which are stored in the 'starExpandedPaths_' map.
The function 'SelectAnalyzer.registerStructSlotRefPathsWithAnalyzer()',
which makes struct slot memory sharing possible, takes these star
expanded paths into account. Then in
'SelectAnalyzer.analyzeSelectClause()' the paths expanded from the star
items are retrieved and and normal analysis takes place.
Testing:
- Added the test function
PlannerTest.testStructFieldSlotSharedWithStructFromStarExpansion()
that verifies that struct slot memory sharing takes place.
Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
2 files changed, 181 insertions(+), 77 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/19190/1
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11793/ : 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/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 04 Nov 2022 12:37:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11816/ : 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/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 08 Nov 2022 16:11:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 11:14:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
IMPALA-11692: Struct slot memory sharing involving select * not working properly
With EXPAND_COMPLEX_TYPES=1, if there are structs coming from the star
expansion and members of the structs are also given explicitly, slot
memory sharing does not work in some cases:
explain select * from functional_orc_def.complextypes_nested_structs;
row-size=64B
explain select *, outer_struct.inner_struct1 from
functional_orc_def.complextypes_nested_structs;
row-size=80B
The row size should be the same in both cases as
outer_struct.inner_struct1 is part of outer_struct which is included in
the star.
This change modifies how star select list items are analysed. First,
before 'SelectAnalyzer.analyzeSelectClause()' is called, all star items
are expanded to paths which are stored in the 'starExpandedPaths_' map.
The function 'SelectAnalyzer.registerStructSlotRefPathsWithAnalyzer()',
which makes struct slot memory sharing possible, takes these star
expanded paths into account. Then in
'SelectAnalyzer.analyzeSelectClause()' the paths expanded from the star
items are retrieved and and normal analysis takes place.
Testing:
- Added the test function
PlannerTest.testStructFieldSlotSharedWithStructFromStarExpansion()
that verifies that struct slot memory sharing takes place.
Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
2 files changed, 222 insertions(+), 81 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/19190/5
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 16:18:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8791/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 11:14:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
IMPALA-11692: Struct slot memory sharing involving select * not working properly
With EXPAND_COMPLEX_TYPES=1, if there are structs coming from the star
expansion and members of the structs are also given explicitly, slot
memory sharing does not work in some cases:
explain select * from functional_orc_def.complextypes_nested_structs;
row-size=64B
explain select *, outer_struct.inner_struct1 from
functional_orc_def.complextypes_nested_structs;
row-size=80B
The row size should be the same in both cases as
outer_struct.inner_struct1 is part of outer_struct which is included in
the star.
This change modifies how star select list items are analysed. First,
before 'SelectAnalyzer.analyzeSelectClause()' is called, all star items
are expanded to paths which are stored in the 'starExpandedPaths_' map.
The function 'SelectAnalyzer.registerStructSlotRefPathsWithAnalyzer()',
which makes struct slot memory sharing possible, takes these star
expanded paths into account. Then in
'SelectAnalyzer.analyzeSelectClause()' the paths expanded from the star
items are retrieved and and normal analysis takes place.
Testing:
- Added the test function
PlannerTest.testStructFieldSlotSharedWithStructFromStarExpansion()
that verifies that struct slot memory sharing takes place.
Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
2 files changed, 193 insertions(+), 77 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/19190/2
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11808/ : 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/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Nov 2022 15:10:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11771/ : 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/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Nov 2022 16:41:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19190/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/19190/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@301
PS1, Line 301: collectStarExpandedPaths();
: registerStructSlotRefPathsWithAnalyzer();
Can you add a few comments about the reason why this is done before analyzeSelectClause()?
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 02 Nov 2022 20:55:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
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/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
IMPALA-11692: Struct slot memory sharing involving select * not working properly
With EXPAND_COMPLEX_TYPES=1, if there are structs coming from the star
expansion and members of the structs are also given explicitly, slot
memory sharing does not work in some cases:
explain select * from functional_orc_def.complextypes_nested_structs;
row-size=64B
explain select *, outer_struct.inner_struct1 from
functional_orc_def.complextypes_nested_structs;
row-size=80B
The row size should be the same in both cases as
outer_struct.inner_struct1 is part of outer_struct which is included in
the star.
This change modifies how star select list items are analysed. First,
before 'SelectAnalyzer.analyzeSelectClause()' is called, all star items
are expanded to paths which are stored in the 'starExpandedPaths_' map.
The function 'SelectAnalyzer.registerStructSlotRefPathsWithAnalyzer()',
which makes struct slot memory sharing possible, takes these star
expanded paths into account. Then in
'SelectAnalyzer.analyzeSelectClause()' the paths expanded from the star
items are retrieved and and normal analysis takes place.
Testing:
- Added the test function
PlannerTest.testStructFieldSlotSharedWithStructFromStarExpansion()
that verifies that struct slot memory sharing takes place.
Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Reviewed-on: http://gerrit.cloudera.org:8080/19190
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/test/java/org/apache/impala/planner/PlannerTest.java
2 files changed, 222 insertions(+), 81 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 7
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/11780/ : 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/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 03 Nov 2022 11:31:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 4: Code-Review+1
(2 comments)
The logic looks good to me, only some comments about naming/comments.
http://gerrit.cloudera.org:8080/#/c/19190/4/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/19190/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@287
PS4, Line 287: private class ExpandedPathInfo {
This is only about expanded star expressions, right? It would be nice to include "star" in the name or at least note this in a comment.
http://gerrit.cloudera.org:8080/#/c/19190/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@307
PS4, Line 307:
: private final Path expandedPath_;
: private final Path originalRootPath_;
Can you add a small comment for these, e.g. path of expanded item, path of original star expression?
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 08 Nov 2022 13:07:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 09:26:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
IMPALA-11692: Struct slot memory sharing involving select * not working properly
With EXPAND_COMPLEX_TYPES=1, if there are structs coming from the star
expansion and members of the structs are also given explicitly, slot
memory sharing does not work in some cases:
explain select * from functional_orc_def.complextypes_nested_structs;
row-size=64B
explain select *, outer_struct.inner_struct1 from
functional_orc_def.complextypes_nested_structs;
row-size=80B
The row size should be the same in both cases as
outer_struct.inner_struct1 is part of outer_struct which is included in
the star.
This change modifies how star select list items are analysed. First,
before 'SelectAnalyzer.analyzeSelectClause()' is called, all star items
are expanded to paths which are stored in the 'starExpandedPaths_' map.
The function 'SelectAnalyzer.registerStructSlotRefPathsWithAnalyzer()',
which makes struct slot memory sharing possible, takes these star
expanded paths into account. Then in
'SelectAnalyzer.analyzeSelectClause()' the paths expanded from the star
items are retrieved and and normal analysis takes place.
Testing:
- Added the test function
PlannerTest.testStructFieldSlotSharedWithStructFromStarExpansion()
that verifies that struct slot memory sharing takes place.
Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
2 files changed, 213 insertions(+), 78 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/19190/4
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/19190/4/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/19190/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@287
PS4, Line 287: private class StarExpandedPathIn
> This is only about expanded star expressions, right? It would be nice to in
Yes, it is only about star paths, changed the name to ExpandedPathInfo.
Also renamed the parameters of addStarExpandedPath() to be more informative.
http://gerrit.cloudera.org:8080/#/c/19190/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@297
PS4, Line 297: // types will have the complex type so it's not empty.
> There are some TODOs here, I think we should address them in a separate cha
Created a JIRA issue and referenced it here.
http://gerrit.cloudera.org:8080/#/c/19190/4/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@307
PS4, Line 307: }
:
: // The path expanded from a star sele
> Can you add a small comment for these, e.g. path of expanded item, path of
Done
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Tue, 08 Nov 2022 15:51:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
IMPALA-11692: Struct slot memory sharing involving select * not working properly
With EXPAND_COMPLEX_TYPES=1, if there are structs coming from the star
expansion and members of the structs are also given explicitly, slot
memory sharing does not work in some cases:
explain select * from functional_orc_def.complextypes_nested_structs;
row-size=64B
explain select *, outer_struct.inner_struct1 from
functional_orc_def.complextypes_nested_structs;
row-size=80B
The row size should be the same in both cases as
outer_struct.inner_struct1 is part of outer_struct which is included in
the star.
This change modifies how star select list items are analysed. First,
before 'SelectAnalyzer.analyzeSelectClause()' is called, all star items
are expanded to paths which are stored in the 'starExpandedPaths_' map.
The function 'SelectAnalyzer.registerStructSlotRefPathsWithAnalyzer()',
which makes struct slot memory sharing possible, takes these star
expanded paths into account. Then in
'SelectAnalyzer.analyzeSelectClause()' the paths expanded from the star
items are retrieved and and normal analysis takes place.
Testing:
- Added the test function
PlannerTest.testStructFieldSlotSharedWithStructFromStarExpansion()
that verifies that struct slot memory sharing takes place.
Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
2 files changed, 193 insertions(+), 77 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/19190/3
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-11692: Struct slot memory sharing involving select * not working properly
Posted by "Peter Rozsa (Code Review)" <ge...@cloudera.org>.
Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/19190 )
Change subject: IMPALA-11692: Struct slot memory sharing involving select * not working properly
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19190/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/19190/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@290
PS2, Line 290: private Map<SelectListItem, List<Pair<Path, Boolean>>> starExpandedPaths_
Could be final.
List<Pair<Path, Boolean>>> could be a class called ExpandedPaths or something similar.
--
To view, visit http://gerrit.cloudera.org:8080/19190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I346c2808c1aa5e77e3cdf3593f7f48ac96516c00
Gerrit-Change-Number: 19190
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Peter Rozsa <pr...@cloudera.com>
Gerrit-Comment-Date: Fri, 04 Nov 2022 14:48:27 +0000
Gerrit-HasComments: Yes