Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-49987][SQL] Fix the error prompt when seedExpression is non-foldable in randstr #48490

Closed
wants to merge 4 commits into from

Conversation

panbingkun
Copy link
Contributor

@panbingkun panbingkun commented Oct 16, 2024

What changes were proposed in this pull request?

The pr aims to

  • fix the error prompt when seedExpression is non-foldable in randstr.
  • use toSQLId to set the parameter value inputName for randstr and uniform of NON_FOLDABLE_INPUT.

Why are the changes needed?

  • Let me take an example
val df = Seq(1.1).toDF("a")
df.createOrReplaceTempView("t")
sql("SELECT randstr(1, a) from t").show(false)
  • Before
image
[DATATYPE_MISMATCH.NON_FOLDABLE_INPUT] Cannot resolve "randstr(1, a)" due to data type mismatch: the input seedExpression should be a foldable INT or SMALLINT expression; however, got "a". SQLSTATE: 42K09; line 1 pos 7;
'Project [unresolvedalias(randstr(1, a#5, false))]
+- SubqueryAlias t
   +- View (`t`, [a#5])
      +- Project [value#1 AS a#5]
         +- LocalRelation [value#1]
  • After
[DATATYPE_MISMATCH.NON_FOLDABLE_INPUT] Cannot resolve "randstr(1, a)" due to data type mismatch: the input seed should be a foldable INT or SMALLINT expression; however, got "a". SQLSTATE: 42K09; line 1 pos 7;
'Project [unresolvedalias(randstr(1, a#5, false))]
+- SubqueryAlias t
   +- View (`t`, [a#5])
      +- Project [value#1 AS a#5]
         +- LocalRelation [value#1]
  • The parameter name (seedExpression) in the error message does not match the parameter name (seed) seen in docs by the end-user.
image

Does this PR introduce any user-facing change?

Yes, When seed is non-foldable , the end-user will get a consistent experience in the error prompt.

How was this patch tested?

Update existed UT.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the SQL label Oct 16, 2024
@panbingkun
Copy link
Contributor Author

cc @dtenedor @MaxGekk

@panbingkun panbingkun marked this pull request as ready for review October 16, 2024 02:55
@@ -374,7 +374,7 @@ case class RandStr(
var result: TypeCheckResult = TypeCheckResult.TypeCheckSuccess
def requiredType = "INT or SMALLINT"
Seq((length, "length", 0),
(seedExpression, "seedExpression", 1)).foreach {
(seedExpression, "seed", 1)).foreach {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it is an id, could you quote it using to toSQLId(), please at:

                "inputName" -> toSQLId(name),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this, let me update Uniform's name in this PR as well:

Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for CI.

@MaxGekk
Copy link
Member

MaxGekk commented Oct 16, 2024

It seems the failed tests is related:

[info] - randstr function *** FAILED *** (181 milliseconds)
[info]   Map("inputName" -> "`length`", "inputType" -> "INT or SMALLINT", "inputExpr" -> ""a"", "sqlExpr" -> ""randstr(a, 10)"") did not equal Map("inputName" -> "length", "inputType" -> "INT or SMALLINT", "inputExpr" -> ""a"", "sqlExpr" -> ""randstr(a, 10)"") (SparkFunSuite.scala:362)

@panbingkun
Copy link
Contributor Author

It seems the failed tests is related:

[info] - randstr function *** FAILED *** (181 milliseconds)
[info]   Map("inputName" -> "`length`", "inputType" -> "INT or SMALLINT", "inputExpr" -> ""a"", "sqlExpr" -> ""randstr(a, 10)"") did not equal Map("inputName" -> "length", "inputType" -> "INT or SMALLINT", "inputExpr" -> ""a"", "sqlExpr" -> ""randstr(a, 10)"") (SparkFunSuite.scala:362)

Thanks!
Yes, I have fixed it, waiting for CI.

@MaxGekk
Copy link
Member

MaxGekk commented Oct 16, 2024

+1, LGTM. Merging to master.
Thank you, @panbingkun.

@MaxGekk MaxGekk closed this in 8d1cb76 Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants