[GitHub] [spark] anchovYu commented on a diff in pull request #42177: [SPARK-44059] Add better error messages for SQL named argumnts

["anchovYu (via GitHub)" <gi...@apache.org>]

anchovYu commented on code in PR #42177:
URL: https://github.com/apache/spark/pull/42177#discussion_r1280942792


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -115,10 +116,11 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
     )
   }
 
-  def unexpectedPositionalArgument(functionName: String): Throwable = {
+  def unexpectedPositionalArgument(functionName: String, parameterName: String): Throwable = {

Review Comment:
   Can we rename `parameterName` to something like last or preceding named argument?



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/FunctionBuilderBase.scala:
##########
@@ -104,7 +104,8 @@ object NamedParametersSupport {
     val positionalParametersSet = allParameterNames.take(positionalArgs.size).toSet
     val namedParametersSet = collection.mutable.Set[String]()
 
-    for (arg <- namedArgs) {
+    namedArgs.zipWithIndex.foreach { tuple =>

Review Comment:
   ```suggestion
       namedArgs.zipWithIndex.foreach { case (arg, index) =>
   ```



##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -2398,7 +2398,7 @@
   },
   "REQUIRED_PARAMETER_NOT_FOUND" : {
     "message" : [
-      "Cannot invoke function <functionName> because the parameter named <parameterName> is required, but the function call did not supply a value. Please update the function call to supply an argument value (either positionally or by name) and retry the query again."
+      "Cannot invoke function <functionName> because the parameter named <parameterName> is required at index <index>, but the function call did not supply a value. Please update the function call to supply an argument value (either positionally or by name) and retry the query again."

Review Comment:
   Or we can move the index thing to positional context:
   ```suggestion
         "Cannot invoke function <functionName> because the parameter named <parameterName> is required, but the function call did not supply a value. Please update the function call to supply an argument value (either positionally at index <index> or by name) and retry the query again."
   ```



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/FunctionBuilderBase.scala:
##########
@@ -141,11 +143,12 @@ object NamedParametersSupport {
     }.toMap
 
     // We rearrange named arguments to match their positional order.
-    val rearrangedNamedArgs: Seq[Expression] = namedParameters.map { param =>
+    val rearrangedNamedArgs: Seq[Expression] = namedParameters.zipWithIndex.map { tuple =>

Review Comment:
   ditto



##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -2398,7 +2398,7 @@
   },
   "REQUIRED_PARAMETER_NOT_FOUND" : {
     "message" : [
-      "Cannot invoke function <functionName> because the parameter named <parameterName> is required, but the function call did not supply a value. Please update the function call to supply an argument value (either positionally or by name) and retry the query again."
+      "Cannot invoke function <functionName> because the parameter named <parameterName> is required at index <index>, but the function call did not supply a value. Please update the function call to supply an argument value (either positionally or by name) and retry the query again."

Review Comment:
   Does this index make sense? Because named arguments actually can appear in any order.



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