You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "srielau (via GitHub)" <gi...@apache.org> on 2023/05/16 20:58:52 UTC

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

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

   > @srielau I am rethinking the requirement after reading the related docs (especially [the doc from snowflake](https://docs.snowflake.com/en/sql-reference/identifier-literal)) So how important it is to support all kinds of expressions(e.g string concats) within the `IDENTIFIER()` clause? It would be much easier if we limit the requirement only accepts the following:
   > 
   > * quoted identifier
   > * session variable
   > 
   > So, instead of
   > 
   > ```
   > identifierReference
   >     : IDENTIFIER_KW LEFT_PAREN expression RIGHT_PAREN
   >     | multipartIdentifier
   >     ;
   > ```
   > 
   > We can make it
   > 
   > ```
   > identifierReference
   >     : IDENTIFIER_KW LEFT_PAREN '\'' multipartIdentifier  '\'' RIGHT_PAREN    #singleQuotedIdentifier
   >     | IDENTIFIER_KW LEFT_PAREN '"' multipartIdentifier  '"' RIGHT_PAREN      #doubleQuotedIdentifier
   >     | IDENTIFIER_KW LEFT_PAREN '$' multipartIdentifier RIGHT_PAREN           #sessionvariableIdentifier
   >     | multipartIdentifier                                                    #simpleMultipartIdentifier
   >     ;
   > ```
   > 
   > And this requires much less changes in the parser and analyzer.
   This is similar to what I tried in my previous attempt: https://github.com/apache/spark/pull/40884
   We need to support parameter markers as well as "proper" (session) variables which would not have a leading '$'.
   They would just be identifiers which need to be resolved.
   Also we need the ability for users to pre-fix and post-fix. For example a schema-name or a field name.
   Without solid support for parameter markers (and variables) we are still open to SQL injection.
   I was told that I cannot call eval from within the parser.
   
     
   


-- 
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