You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/11/18 17:05:22 UTC
[GitHub] [spark] srielau commented on a diff in pull request #38713: [SPARK-41195][SQL] Support PIVOT/UNPIVOT with join children
srielau commented on code in PR #38713:
URL: https://github.com/apache/spark/pull/38713#discussion_r1026672022
##########
sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -697,12 +697,12 @@ setQuantifier
;
relation
- : LATERAL? relationPrimary joinRelation*
+ : LATERAL? relationPrimary pivotClause? unpivotClause? joinRelation*
;
joinRelation
- : (joinType) JOIN LATERAL? right=relationPrimary joinCriteria?
- | NATURAL joinType JOIN LATERAL? right=relationPrimary
+ : (joinType) JOIN LATERAL? right=relationPrimary joinCriteria? pivotClause? unpivotClause?
+ | NATURAL joinType JOIN LATERAL? right=relationPrimary pivotClause? unpivotClause?
Review Comment:
Have you tried simply:
```suggestion
: (joinType) JOIN LATERAL? right=relationPrimary joinCriteria? pivotClause? unpivotClause?
| NATURAL joinType JOIN LATERAL? right=relationPrimary
| pivotClause
| unpivotClause
```
Also removing the relation entries above?
##########
sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -697,12 +697,12 @@ setQuantifier
;
relation
- : LATERAL? relationPrimary joinRelation*
+ : LATERAL? relationPrimary pivotClause? unpivotClause? joinRelation*
Review Comment:
This doesn't look right.
As you say PIVOT and UNPIVOT are like JOINs, so why are do you have this strange ordered optional clauses instead of merging them in as if they were another type of join.
I can't put my finger on exactly how it breaks without running a bunch of tests.
For example I should be able to chain a string UNPIVOTs and PIVOTs in any order without the need for a JOIN (or a braces) in between. I don't see how that works here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org