[GitHub] [spark] dtenedor commented on pull request #41007: [SPARK-43205] IDENTIFIER clause

["dtenedor (via GitHub)" <gi...@apache.org>]

dtenedor commented on PR #41007:
URL: https://github.com/apache/spark/pull/41007#issuecomment-1553732854

   @srielau @gengliangwang @cloud-fan 
   
   The general structure of the PR looks OK.
   
   This PR proposes to add new unresolved nodes for several different locations in the parser. This could be useful in the future if we want to add more custom analysis support for these different areas.
   
   Alternatively, we could leave the multipartIdentifier references where they are, and just update its definition instead:
   
   ```
   multipartIdentifier
       : IDENTIFIER_KW LEFT_PAREN expression RIGHT_PAREN
       | parts+=errorCapturingIdentifier (DOT parts+=errorCapturingIdentifier)*
       ;
   ```
   
   This would reduce the number of changes to the .g4 file, since the proposed namespaceReference and functionNameReference and relationReference are all the same syntax. Then the PR would be easier to merge into different Spark forks out there. But this approach grants us future flexibility, if we ever anticipate the syntax to diverge for these different cases. I am OK with the proposed approach 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