This is an automated email from the ASF dual-hosted git repository.

maxgekk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new 9dcc24c36f6 [SPARK-39047][SQL] Replace the error class 
ILLEGAL_SUBSTRING by INVALID_PARAMETER_VALUE
9dcc24c36f6 is described below

commit 9dcc24c36f6fcdf43bf66fe50415be575f7b2918
Author: Max Gekk <max.g...@gmail.com>
AuthorDate: Thu Apr 28 07:46:44 2022 +0300

    [SPARK-39047][SQL] Replace the error class ILLEGAL_SUBSTRING by 
INVALID_PARAMETER_VALUE
    
    ### What changes were proposed in this pull request?
    In the PR, I propose to remove the `ILLEGAL_SUBSTRING` error class, and use 
`INVALID_PARAMETER_VALUE` in the case when the `strfmt` parameter of the 
`format_string()` function contains `%0$`. The last value is handled 
differently by JDKs:  _"... Java 8 and Java 11 uses it as "%1$", and Java 17 
throws IllegalFormatArgumentIndexException(Illegal format argument index = 0)"_.
    
    ### Why are the changes needed?
    To improve code maintenance and user experience with Spark SQL by reducing 
the number of user-facing error classes.
    
    ### Does this PR introduce _any_ user-facing change?
    Yes, it changes user-facing error message.
    
    Before:
    ```sql
    spark-sql> select format_string('%0$s', 'Hello');
    Error in query: [ILLEGAL_SUBSTRING] The argument_index of string format 
cannot contain position 0$.; line 1 pos 7
    ```
    
    After:
    ```sql
    spark-sql> select format_string('%0$s', 'Hello');
    Error in query: [INVALID_PARAMETER_VALUE] The value of parameter(s) 
'strfmt' in `format_string` is invalid: expects %1$, %2$ and so on, but got 
%0$.; line 1 pos 7
    ```
    
    ### How was this patch tested?
    By running the affected test suites:
    ```
    $ build/sbt "test:testOnly *SparkThrowableSuite"
    $ build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z 
text.sql"
    $ build/sbt "test:testOnly *QueryCompilationErrorsSuite"
    ```
    
    Closes #36380 from MaxGekk/error-class-ILLEGAL_SUBSTRING.
    
    Authored-by: Max Gekk <max.g...@gmail.com>
    Signed-off-by: Max Gekk <max.g...@gmail.com>
---
 core/src/main/resources/error/error-classes.json                   | 3 ---
 .../apache/spark/sql/catalyst/expressions/stringExpressions.scala  | 3 +--
 .../scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala | 7 ++++---
 .../src/test/resources/sql-tests/results/postgreSQL/text.sql.out   | 2 +-
 .../org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala  | 7 ++++---
 5 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/core/src/main/resources/error/error-classes.json 
b/core/src/main/resources/error/error-classes.json
index 673866e6c35..4738599685b 100644
--- a/core/src/main/resources/error/error-classes.json
+++ b/core/src/main/resources/error/error-classes.json
@@ -71,9 +71,6 @@
   "GROUPING_SIZE_LIMIT_EXCEEDED" : {
     "message" : [ "Grouping sets size cannot be greater than <maxSize>" ]
   },
-  "ILLEGAL_SUBSTRING" : {
-    "message" : [ "<subject> cannot contain <content>." ]
-  },
   "INCOMPARABLE_PIVOT_COLUMN" : {
     "message" : [ "Invalid pivot column '<columnName>'. Pivot columns must be 
comparable." ],
     "sqlState" : "42000"
diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
index 976caeb3502..9089ff46637 100755
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
@@ -1898,8 +1898,7 @@ case class FormatString(children: Expression*) extends 
Expression with ImplicitC
    */
   private def checkArgumentIndexNotZero(expression: Expression): Unit = 
expression match {
     case StringLiteral(pattern) if pattern.contains("%0$") =>
-      throw QueryCompilationErrors.illegalSubstringError(
-        "The argument_index of string format", "position 0$")
+      throw QueryCompilationErrors.zeroArgumentIndexError()
     case _ => // do nothing
   }
 }
diff --git 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
index 7f212ed5891..3d379fb4f71 100644
--- 
a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
+++ 
b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
@@ -66,10 +66,11 @@ object QueryCompilationErrors extends QueryErrorsBase {
       messageParameters = Array(sizeLimit.toString))
   }
 
-  def illegalSubstringError(subject: String, illegalContent: String): 
Throwable = {
+  def zeroArgumentIndexError(): Throwable = {
     new AnalysisException(
-      errorClass = "ILLEGAL_SUBSTRING",
-      messageParameters = Array(subject, illegalContent))
+      errorClass = "INVALID_PARAMETER_VALUE",
+      messageParameters = Array(
+        "strfmt", toSQLId("format_string"), "expects %1$, %2$ and so on, but 
got %0$."))
   }
 
   def unorderablePivotColError(pivotCol: Expression): Throwable = {
diff --git 
a/sql/core/src/test/resources/sql-tests/results/postgreSQL/text.sql.out 
b/sql/core/src/test/resources/sql-tests/results/postgreSQL/text.sql.out
index 3605d54bc33..769b2b92d80 100755
--- a/sql/core/src/test/resources/sql-tests/results/postgreSQL/text.sql.out
+++ b/sql/core/src/test/resources/sql-tests/results/postgreSQL/text.sql.out
@@ -285,7 +285,7 @@ select format_string('%0$s', 'Hello')
 struct<>
 -- !query output
 org.apache.spark.sql.AnalysisException
-[ILLEGAL_SUBSTRING] The argument_index of string format cannot contain 
position 0$.; line 1 pos 7
+[INVALID_PARAMETER_VALUE] The value of parameter(s) 'strfmt' in 
`format_string` is invalid: expects %1$, %2$ and so on, but got %0$.; line 1 
pos 7
 
 
 -- !query
diff --git 
a/sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
 
b/sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
index a30521e5baf..2d1e6f94925 100644
--- 
a/sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
+++ 
b/sql/core/src/test/scala/org/apache/spark/sql/errors/QueryCompilationErrorsSuite.scala
@@ -106,15 +106,16 @@ class QueryCompilationErrorsSuite
     }
   }
 
-  test("ILLEGAL_SUBSTRING: the argument_index of string format is invalid") {
+  test("INVALID_PARAMETER_VALUE: the argument_index of string format is 
invalid") {
     withSQLConf(SQLConf.ALLOW_ZERO_INDEX_IN_FORMAT_STRING.key -> "false") {
       val e = intercept[AnalysisException] {
         sql("select format_string('%0$s', 'Hello')")
       }
       checkErrorClass(
         exception = e,
-        errorClass = "ILLEGAL_SUBSTRING",
-        msg = "The argument_index of string format cannot contain position 
0$.; line 1 pos 7")
+        errorClass = "INVALID_PARAMETER_VALUE",
+        msg = "The value of parameter(s) 'strfmt' in `format_string` is 
invalid: " +
+          "expects %1$, %2$ and so on, but got %0$.; line 1 pos 7")
     }
   }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org

Reply via email to