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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to