advancedxy commented on code in PR #42255:
URL: https://github.com/apache/spark/pull/42255#discussion_r1280210536
##########
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:
There are two reasons why I didn't handle other types.
1. for existing hints such as rebalance/repartition, the first parameter
could be integer. We can wrap it as a `Literal`. However some tests needs to be
modified to reflect that, it introduce some overhead.
2. PySpark side allowed parameter types: ` allowed_types = (str, list,
float, int)`, I'm not sure why float and list are allowed in the first place.
But to be conservative, I suppose it might be float/list parameters in some
cases.
In summary, it would be safe to parse Strings only.
> If all params need to be expressions, can't we just wrap it with Literal?
To be concise, all params except the first one, need to be
UnresolvedAttribute
https://github.com/apache/spark/commit/6958d7e62959313fb6a1288527aaa611aebee087#diff-f62cd2d0dbd3f1bd4f58f9ab451e53e98f87866b861a20fd9739642764088821R160
--
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]
