advancedxy commented on code in PR #42255:
URL: https://github.com/apache/spark/pull/42255#discussion_r1285326065


##########
sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala:
##########
@@ -1407,7 +1407,15 @@ class Dataset[T] private[sql](
    */
   @scala.annotation.varargs
   def hint(name: String, parameters: Any*): Dataset[T] = withTypedPlan {
-    UnresolvedHint(name, parameters, logicalPlan)
+    // parse string parameters into Expressions as ResolveHint requires all 
the parameters to be
+    // expressions except the first one could be numeric. This logic matches 
how sql hint is parsed
+    // and makes caller easier to pass string parameters in hint 
specification, especially for
+    // other language bindings, such as PySpark.
+    val pars = parameters.map {
+      case s: String => sparkSession.sessionState.sqlParser.parseExpression(s)

Review Comment:
   >  Besides, we should add one more config, since the hint framework is a 
kind of extension, developers may have their own resolution rules.
   
   It's possible to extend the SparkSQLExtension to inject other resolve hint 
rules. It's good idea to handle the built-in hints specially. So for this 
suggestion, I would check the hint name is in the built-in hints or not rather 
than add one more config. 
   
   > However, I think we should treat all input parameters as expression like 
what AstBuilder does rathen than only handle string type.
   
   It might not be that easy. For `AstBuilder`, all the input params are 
string, so they can easily be parsed. For the hint method, the parameters could 
be any type: int/float/string/Column/UnresolvedAttribute etc. It's possible to 
pass a parameter that is not an expression neither is possible to parsed as an 
expression. 
   
   Let's parse int/long/String as expression and left others as it is? WDYT
   



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