cloud-fan commented on a change in pull request #31754:
URL: https://github.com/apache/spark/pull/31754#discussion_r601040939



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala
##########
@@ -347,8 +348,9 @@ case class AttributeReference(
   }
 
   override def sql: String = {
-    val qualifierPrefix = if (qualifier.nonEmpty) qualifier.mkString(".") + 
"." else ""
-    s"$qualifierPrefix${quoteIdentifier(name)}"
+    val qualifierPrefix =
+      if (qualifier.nonEmpty) qualifier.map(quoteIfNeeded).mkString(".") + "." 
else ""
+    s"$qualifierPrefix${quoteIfNeeded(name)}"

Review comment:
       This is a good point. For places that only display the table/column 
name, like in the error message `Table not found: a b`, it's a bit weird but 
users can still understand the meaning. But in an expression sql string, `a b + 
1` is ambiguous.
   
   I think we shoud follow the parser definition of identifiers `(LETTER | 
DIGIT | '_')+`, and only omit the quoting if the name only contains letter, 
digit or underscore.




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

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