advancedxy commented on code in PR #42255:
URL: https://github.com/apache/spark/pull/42255#discussion_r1298105145
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameHintSuite.scala:
##########
@@ -42,43 +44,53 @@ class DataFrameHintSuite extends AnalysisTest with
SharedSparkSession {
check(
df.hint("hint1", 1, "a"),
- UnresolvedHint("hint1", Seq(1, "a"), df.logicalPlan)
+ UnresolvedHint("hint1", Seq(Literal(1), Literal("a")), df.logicalPlan)
)
check(
df.hint("hint1", 1, $"a"),
- UnresolvedHint("hint1", Seq(1, $"a"),
+ UnresolvedHint("hint1", Seq(Literal(1), $"a".expr),
df.logicalPlan
)
)
check(
- df.hint("hint1", Seq(1, 2, 3), Seq($"a", $"b", $"c")),
- UnresolvedHint("hint1", Seq(Seq(1, 2, 3), Seq($"a", $"b", $"c")),
+ df.hint("hint1", Array(1, 2, 3), array($"a", $"b", $"c")),
Review Comment:
> It's better to avoid breaking change unless it needs a lot of effort.
I do agree that we should avoid breaking change unless necessary.
However if we are going to normalize the input to the hint method, such as
requiring it to be a column/literal, we will bring breaking changes. We can
special-case for `Seq`(not just `Seq[Int]`) to Array, however since the hint
accept any type of input, we will break other inputs potentially.
Also, I didn't see any hint accept a `Seq` as input in the code, are you
aware of such hints exists in the wild?
--
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]
