vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4474120161
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/JsonExpressionsSuite.scala:
##########
@@ -1138,4 +1139,144 @@ class JsonExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
StructType(StructField("f", FileType.external) :: Nil))
}
}
+
+ test("SPARK-58793: IS JSON evaluates its operand exactly once") {
+ // Regression test: a non-deterministic operand (e.g. `IF(rand() < 0.5,
NULL, '{}')`) must be
+ // evaluated a single time, otherwise the predicate could observe two
different values and
+ // return a wrong result (false, when only NULL or true are valid). The
replacement must
+ // therefore reference the operand exactly once.
+ val child = Literal.create("{}", StringType)
+ Seq(IsJsonShape.Any, IsJsonShape.Value, IsJsonShape.Object,
IsJsonShape.Array,
+ IsJsonShape.Scalar).foreach { shape =>
+ val references = IsJson(child, shape).replacement.collect { case e if e
eq child => e }
+ assert(references.length == 1,
+ s"operand referenced ${references.length} times for shape $shape,
expected exactly once")
+ }
+ }
+
+ test("SPARK-58793: IS JSON requires strict ANSI JSON (no Hive leniency)") {
+ // Single quotes and unescaped control characters are allowed by the
Hive-compatible parser
+ // shared by json_typeof etc., but the ANSI SQL IS JSON predicate must
reject them.
+ checkEvaluation(IsJson(Literal("{'a':1}"), IsJsonShape.Any), false)
+ checkEvaluation(IsJson(Literal("['a', 'b']"), IsJsonShape.Any), false)
+ // A raw (unescaped) control character inside a JSON string is not valid
ANSI JSON.
+ checkEvaluation(IsJson(Literal("\"line1\nline2\""), IsJsonShape.Any),
false)
+ // The double-quoted equivalents are well-formed ANSI JSON.
+ checkEvaluation(IsJson(Literal("""{"a":1}"""), IsJsonShape.Any), true)
+ checkEvaluation(IsJson(Literal("""["a", "b"]"""), IsJsonShape.Any), true)
+ // Other parser extensions are rejected too: comments, unquoted field
names, non-numeric
+ // numbers, leading zeros, and whitespace-only input.
+ Seq("/* c */ {}", "{} // c", "# c\n{}", "{a:1}", "NaN", "Infinity",
"-Infinity", "01",
+ "-01", "[01]", " ", "\n\t").foreach { json =>
+ checkEvaluation(IsJson(Literal(json), IsJsonShape.Any), false)
+ }
+ // Duplicate field names are accepted (no WITH UNIQUE KEYS support).
+ checkEvaluation(IsJson(Literal("""{"a":1,"a":2}"""), IsJsonShape.Object),
true)
+ }
+
+ test("SPARK-58793: IS JSON rejects structurally malformed containers") {
+ // Containers that only turn malformed after a valid first token:
truncated input, trailing
+ // commas, missing values, a missing colon, and mismatched or unmatched
closers. Pinning these
+ // guards against a strict factory accidentally built with
JsonReadFeature.ALLOW_TRAILING_COMMA
+ // or ALLOW_MISSING_VALUES, or against a token walk that accepts a
container ending early.
+ Seq("{", "[", "[1,", "{\"a\"", "{\"a\":", "[1,]", "{\"a\":1,}", "[1,,2]",
"{\"a\"}", "[1}",
+ "{]", "}", "]").foreach { json =>
+ checkEvaluation(IsJson(Literal(json), IsJsonShape.Any), false)
+ }
+ // The matching shape qualifier must reject them too, not only bare IS
JSON: the first token
+ // matches the shape, so only the malformed rest can reject the input.
+ checkEvaluation(IsJson(Literal("[1,]"), IsJsonShape.Array), false)
+ checkEvaluation(IsJson(Literal("{\"a\":1,}"), IsJsonShape.Object), false)
+ checkEvaluation(IsJson(Literal("[1,,2]"), IsJsonShape.Array), false)
+ checkEvaluation(IsJson(Literal("[1}"), IsJsonShape.Array), false)
+ }
+
+ test("SPARK-58793: IS JSON fails on input beyond the parser limits, like
parse_json") {
+ def checkLimit(
+ json: String,
+ shape: IsJsonShape,
+ condition: String,
+ parameters: Map[String, String]): Unit = {
+ // Only the first 128 characters of the input are echoed in the error.
+ checkErrorInExpression[SparkRuntimeException](
+ IsJson(Literal(json), shape),
+ InternalRow.empty,
+ condition,
+ parameters + ("inputPrefix" -> s"'${json.take(128)}...'"))
+ }
+ // Inputs at the limits are classified as usual.
+ checkEvaluation(IsJson(Literal("[" * 1000 + "]" * 1000),
IsJsonShape.Array), true)
+ checkEvaluation(IsJson(Literal("1" * 1000), IsJsonShape.Scalar), true)
+ // One past the nesting depth, number length, or field name length limit
is an error rather
Review Comment:
Fixed.
Added a case with a string value of DEFAULT_MAX_STRING_LEN + 1 characters
(same construction as JsonFunctionsSuite), which pins both the message
recognition (STRING_LENGTH rather than UNKNOWN) and the token walk decoding
every string value.
--
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]
