[GitHub] [spark] ulysses-you commented on a diff in pull request #42255: [SPARK-40178][SQL] Support string parameters in hint method

["ulysses-you (via GitHub)" <gi...@apache.org>]

ulysses-you commented on code in PR #42255:
URL: https://github.com/apache/spark/pull/42255#discussion_r1285308250


##########
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'm fine with this idea. However, I think we should treat all input parameters as expression like what AstBuilder does rathen than only handle string type. Besides, we should add one more config, since the hint framework is a kind of extension, developers may have their own resolution rules.



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