yadavay-amzn commented on code in PR #57211:
URL: https://github.com/apache/spark/pull/57211#discussion_r3584094038
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/JsonExpressionsSuite.scala:
##########
@@ -909,6 +909,34 @@ class JsonExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
}
}
+ test("json_valid") {
+ Seq(
+ // null input returns null
+ (null, null),
+ // valid JSON values
+ ("""{"a":1}""", true),
+ ("""{"a": "b", "c": [1, 2, 3]}""", true),
+ ("[1, 2, 3]", true),
+ ("[]", true),
+ ("{}", true),
+ ("\"a string\"", true),
+ ("123", true),
+ ("true", true),
+ ("null", true),
+ // invalid JSON values
+ ("", false),
+ (" ", false),
+ ("invalid", false),
+ ("""{"a":1} garbage""", false),
+ ("[1, 2, 3", false),
+ ("""{"a": }""", false)
+ ).foreach {
+ case (input, expected) =>
+ val literal = if (input == null) Literal.create(null, StringType) else
Literal(input)
+ checkEvaluation(JsonValid(literal), expected)
Review Comment:
Since the single-quote leniency is the one intentional departure from strict
JSON, it might be worth a case here that pins it, e.g. `("{'a':1}", true)`, so
the decision doesn't silently regress later.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/json/JsonExpressionUtils.java:
##########
@@ -54,6 +54,26 @@ public static Integer lengthOfJsonArray(UTF8String json) {
}
}
+ public static Boolean isJsonValid(UTF8String json) {
+ if (json == null) {
Review Comment:
I think this null check might be unreachable, since the `replacement` builds
the `StaticInvoke` with the default `propagateNull = true`, so a null input
gets short-circuited to null before this method runs. `lengthOfJsonArray` and
`jsonObjectKeys` rely on that and skip the check, so it could go for
consistency with its neighbors. Not harmful either way.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -693,3 +693,52 @@ case class JsonObjectKeys(child: Expression)
override protected def withNewChildInternal(newChild: Expression):
JsonObjectKeys =
copy(child = newChild)
}
+
+/**
+ * A function that returns true if the input is a valid JSON string, false
otherwise.
+ */
+@ExpressionDescription(
+ usage = "_FUNC_(jsonString) - Returns true if `jsonString` is a valid JSON
string, " +
Review Comment:
Since this reuses `SharedFactory`, it inherits `ALLOW_SINGLE_QUOTES` and
`ALLOW_UNESCAPED_CONTROL_CHARS`, so something like `json_valid("{'a':1}")`
returns true. That seems like a reasonable choice for consistency with
`get_json_object` and the rest of the JSON family, but "valid JSON string"
reads as strict RFC to most people. Might be worth a sentence here (and in the
Scala/PySpark docstrings) noting it follows the same lenient parsing as the
other JSON functions, so the single-quote / control-char behavior isn't a
surprise.
--
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]
