You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Steve Carlin (Code Review)" <ge...@cloudera.org> on 2024/10/29 02:12:30 UTC
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Steve Carlin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21987
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
IMPALA-13493: Square brackets failing in Calcite planner.
Needed to change the parser to allow hints that have square
brackets. These hints are not being used at the current time,
but it will now compile.
Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
---
M java/calcite-planner/src/main/codegen/templates/Parser.jj
1 file changed, 5 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/21987/1
--
To view, visit http://gerrit.cloudera.org:8080/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21987 )
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21987/1/java/calcite-planner/src/main/codegen/templates/Parser.jj
File java/calcite-planner/src/main/codegen/templates/Parser.jj:
http://gerrit.cloudera.org:8080/#/c/21987/1/java/calcite-planner/src/main/codegen/templates/Parser.jj@5337
PS1, Line 5337: /*
> I was going back and forth on this.
In https://gerrit.cloudera.org/c/21923/4/java/calcite-planner/src/main/codegen/templates/Parser.jj you deleted a bunch. I think we err on not leaving commented-out code around.
--
To view, visit http://gerrit.cloudera.org:8080/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Oct 2024 21:02:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21987 )
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Oct 2024 21:15:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Joe McDonnell, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21987
to look at the new patch set (#2).
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
IMPALA-13493: Square brackets failing in Calcite planner.
Needed to change the parser to allow hints that have square
brackets. These hints are not being used at the current time,
but it will now compile.
Specifically, this was found when testing the following query
with the shuffle hint (in distinct.test):
select count(distinct a.int_col) from functional.alltypes a
inner join [shuffle] functional.alltypes b on
a.int_col = b.int_col;
To minimize the changes made in the parser file, the
BRACKET_QUOTED_IDENTIFIER was reused, but removed from
the part in the parser where it was actually used as an
identifier. It is simply parsed and ignored when following
a join type.
Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
---
M java/calcite-planner/src/main/codegen/templates/Parser.jj
1 file changed, 5 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/21987/2
--
To view, visit http://gerrit.cloudera.org:8080/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21987 )
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21987/1/java/calcite-planner/src/main/codegen/templates/Parser.jj
File java/calcite-planner/src/main/codegen/templates/Parser.jj:
http://gerrit.cloudera.org:8080/#/c/21987/1/java/calcite-planner/src/main/codegen/templates/Parser.jj@5337
PS1, Line 5337: /*
Why is this commented out rather than deleted?
--
To view, visit http://gerrit.cloudera.org:8080/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Oct 2024 20:35:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has abandoned this change. ( http://gerrit.cloudera.org:8080/21987 )
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Joe McDonnell, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21987
to look at the new patch set (#3).
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
IMPALA-13493: Square brackets failing in Calcite planner.
Needed to change the parser to allow hints that have square
brackets. These hints are not being used at the current time,
but it will now compile.
Specifically, this was found when testing the following query
with the shuffle hint (in distinct.test):
select count(distinct a.int_col) from functional.alltypes a
inner join [shuffle] functional.alltypes b on
a.int_col = b.int_col;
To minimize the changes made in the parser file, the
BRACKET_QUOTED_IDENTIFIER was reused, but removed from
the part in the parser where it was actually used as an
identifier. It is simply parsed and ignored when following
a join type.
Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
---
M java/calcite-planner/src/main/codegen/templates/Parser.jj
1 file changed, 3 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/21987/3
--
To view, visit http://gerrit.cloudera.org:8080/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21987 )
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21987/1/java/calcite-planner/src/main/codegen/templates/Parser.jj
File java/calcite-planner/src/main/codegen/templates/Parser.jj:
http://gerrit.cloudera.org:8080/#/c/21987/1/java/calcite-planner/src/main/codegen/templates/Parser.jj@5337
PS1, Line 5337: /*
> In https://gerrit.cloudera.org/c/21923/4/java/calcite-planner/src/main/code
Done
--
To view, visit http://gerrit.cloudera.org:8080/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Oct 2024 21:10:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21987 )
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 4
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Mar 2025 18:32:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21987 )
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/17286/ : 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/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Oct 2024 02:38:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Posted by "Steve Carlin (Code Review)" <ge...@cloudera.org>.
Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21987 )
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21987/1/java/calcite-planner/src/main/codegen/templates/Parser.jj
File java/calcite-planner/src/main/codegen/templates/Parser.jj:
http://gerrit.cloudera.org:8080/#/c/21987/1/java/calcite-planner/src/main/codegen/templates/Parser.jj@2004
PS1, Line 2004: ) [<BRACKET_QUOTED_IDENTIFIER>]
> Can you explain a bit more how this works? The syntax is obscure. Maybe pro
Done
http://gerrit.cloudera.org:8080/#/c/21987/1/java/calcite-planner/src/main/codegen/templates/Parser.jj@5337
PS1, Line 5337: /*
> Why is this commented out rather than deleted?
I was going back and forth on this.
I think it's a good idea to track how this differs from the original Parser.jj file. Commenting things out rather than removing code keeps the original code close at hand.
I don't mind deleting it though if we feel that is the correct thing to do.
--
To view, visit http://gerrit.cloudera.org:8080/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Oct 2024 21:01:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21987 )
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/17299/ : 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/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Steve Carlin <sc...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Oct 2024 21:34:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13493: Square brackets failing in Calcite planner.
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21987 )
Change subject: IMPALA-13493: Square brackets failing in Calcite planner.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21987/1/java/calcite-planner/src/main/codegen/templates/Parser.jj
File java/calcite-planner/src/main/codegen/templates/Parser.jj:
http://gerrit.cloudera.org:8080/#/c/21987/1/java/calcite-planner/src/main/codegen/templates/Parser.jj@2004
PS1, Line 2004: ) [<BRACKET_QUOTED_IDENTIFIER>]
Can you explain a bit more how this works? The syntax is obscure. Maybe provide some examples in the commit message.
--
To view, visit http://gerrit.cloudera.org:8080/21987
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I201a6cecfa1af4af6fd2739904f624b8eb790308
Gerrit-Change-Number: 21987
Gerrit-PatchSet: 1
Gerrit-Owner: Steve Carlin <sc...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Oct 2024 18:42:50 +0000
Gerrit-HasComments: Yes