Skip to content

Commit

Permalink
[SPARK-37013][SQL][FOLLOWUP] Add legacy flag for the breaking change …
Browse files Browse the repository at this point in the history
…of forbidding %0$ usage in format_string

### What changes were proposed in this pull request?
Adds a legacy flag `spark.sql.legacy.allowZeroIndexInFormatString` for the breaking change introduced in #34313 and #34454 (followup).

The flag is disabled by default. But when it is enabled, restore the pre-change behavior that allows the 0 based index in `format_string`.

### Why are the changes needed?
The original commit is a breaking change, and breaking changes should be encouraged to add a flag to turn it off for smooth migration between versions.

### Does this PR introduce _any_ user-facing change?
With the default value of the conf, there is no user-facing difference.
If users turn this conf on, they can restore the pre-change behavior.

### How was this patch tested?
Through unit tests.

Closes #36101 from anchovYu/flags-format-string-java.

Authored-by: Xinyi Yu <xinyi.yu@databricks.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit b7af2b3)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
  • Loading branch information
anchovYu authored and cloud-fan committed Apr 8, 2022
1 parent b27e884 commit 4d5c85e
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 6 deletions.
Expand Up @@ -1808,7 +1808,9 @@ case class ParseUrl(children: Seq[Expression], failOnError: Boolean = SQLConf.ge
case class FormatString(children: Expression*) extends Expression with ImplicitCastInputTypes {

require(children.nonEmpty, s"$prettyName() should take at least 1 argument")
checkArgumentIndexNotZero(children(0))
if (!SQLConf.get.getConf(SQLConf.ALLOW_ZERO_INDEX_IN_FORMAT_STRING)) {
checkArgumentIndexNotZero(children(0))
}


override def foldable: Boolean = children.forall(_.foldable)
Expand Down
Expand Up @@ -2015,6 +2015,17 @@ object SQLConf {
.booleanConf
.createWithDefault(false)

val ALLOW_ZERO_INDEX_IN_FORMAT_STRING =
buildConf("spark.sql.legacy.allowZeroIndexInFormatString")
.internal()
.doc("When false, the `strfmt` in `format_string(strfmt, obj, ...)` and " +
"`printf(strfmt, obj, ...)` will no longer support to use \"0$\" to specify the first " +
"argument, the first argument should always reference by \"1$\" when use argument index " +
"to indicating the position of the argument in the argument list.")
.version("3.3")
.booleanConf
.createWithDefault(false)

val USE_CURRENT_SQL_CONFIGS_FOR_VIEW =
buildConf("spark.sql.legacy.useCurrentConfigsForView")
.internal()
Expand Down
Expand Up @@ -19,6 +19,7 @@ package org.apache.spark.sql.errors

import org.apache.spark.sql.{AnalysisException, IntegratedUDFTestUtils, QueryTest}
import org.apache.spark.sql.functions.{grouping, grouping_id, sum}
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.test.SharedSparkSession

case class StringLongClass(a: String, b: Long)
Expand Down Expand Up @@ -94,12 +95,14 @@ class QueryCompilationErrorsSuite extends QueryTest with SharedSparkSession {
}

test("ILLEGAL_SUBSTRING: the argument_index of string format is invalid") {
val e = intercept[AnalysisException] {
sql("select format_string('%0$s', 'Hello')")
withSQLConf(SQLConf.ALLOW_ZERO_INDEX_IN_FORMAT_STRING.key -> "false") {
val e = intercept[AnalysisException] {
sql("select format_string('%0$s', 'Hello')")
}
assert(e.errorClass === Some("ILLEGAL_SUBSTRING"))
assert(e.message ===
"The argument_index of string format cannot contain position 0$.")
}
assert(e.errorClass === Some("ILLEGAL_SUBSTRING"))
assert(e.message ===
"The argument_index of string format cannot contain position 0$.")
}

test("CANNOT_USE_MIXTURE: Using aggregate function with grouped aggregate pandas UDF") {
Expand Down

0 comments on commit 4d5c85e

Please sign in to comment.