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


##########
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:
   > I mean we can add something like this, and by default we can tune this 
config off.
   
   Maybe we should turn on this config by default for known hints since it 
would be much easier to use this API if allowed string parameter. I was 
suggesting:
   ```scala
     def hint(name: String, parameters: Any*): Dataset[T] = withTypedPlan {
       val parsed = if 
(ResolveHints.builtInHintNames.contains(name.toUpperCase(Locale.ROOT))) {
         parameters.map {
           case s: String => 
sparkSession.sessionState.sqlParser.parseExpression(s)
           case i: Integer | Long => Literal(i)
           // other parameter types are left as it is. The ResolveHints rule 
will throw
           // a query analysis exception if not supported.
         }
       } else {
         parameters
       }
       UnresolvedHint(name, parsed, logicalPlan)
     }
   ```
   ``` scala
   // the builtInHintNames could be defined as follows in the ResolveHints:
     val builtInHintNames =
       JoinStrategyHint.strategies.flatMap(_.hintAliases) ++ 
ResolveCoalesceHints.COALESCE_HINT_NAMES
   ```
   WDYT?
   
   I'm not a big fan of adding new config if we can determine which is better 
to do.



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