You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "vinodkc (via GitHub)" <gi...@apache.org> on 2026/08/16 03:34:48 UTC
[PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
vinodkc opened a new pull request, #58024:
URL: https://github.com/apache/spark/pull/58024
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'common/utils/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'common/utils/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Implements the ANSI SQL `IS [NOT] JSON` predicate:
```sql
expr IS [NOT] JSON [VALUE | ARRAY | OBJECT | SCALAR]
Shape qualifiers:
- VALUE (or no qualifier) — any well-formed JSON value (object, array, or scalar)
- OBJECT — top-level JSON object
- ARRAY — top-level JSON array
- SCALAR — JSON primitive only (string, number, boolean, null)
```
`IS NOT JSON `is the negation and accepts the same optional qualifier.
New keywords: `OBJECT` and `SCALAR`
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
`IS [NOT] JSON `is a standard SQL predicate (ANSI SQL) widely supported by other databases (PostgreSQL, Oracle, DB2, MariaDB).
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as new features, bug fixes, or other behavior changes. Documentation-only updates are not considered user-facing changes.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
Yes. New SQL syntax:
```
SELECT '{"a":1}' IS JSON; -- true
SELECT '{"a":1}' IS JSON VALUE; -- true (VALUE = any valid JSON)
SELECT '{"a":1}' IS JSON OBJECT; -- true
SELECT '[1,2]' IS JSON ARRAY; -- true
SELECT '42' IS JSON SCALAR; -- true
SELECT '{"a":1}' IS JSON SCALAR; -- false
SELECT 'bad' IS NOT JSON; -- true
SELECT NULL IS JSON; -- NULL
```
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
Added tests in `SparkConnectDatabaseMetaDataSuite.scala` , `json-functions.sql`
### Was this patch authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
Yes, Generated-by: Claude Code
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #58024:
URL: https://github.com/apache/spark/pull/58024#issuecomment-5771655975
I reviewed the latest revision. The earlier duplicate-evaluation and Hive-leniency issues are addressed; I have left the remaining current-source feedback in this review.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"conversation:5492971361","thread_id":"conversation:5492971361","verdict_sha256":"d5f761afb9fc8c047214cb73f5ed1a0187c486902ed91a6ab6db7e397fb14e04"} -->
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub)" <gi...@apache.org>.
vinodkc commented on PR #58024:
URL: https://github.com/apache/spark/pull/58024#issuecomment-6352229512
Thanks @cloud-fan .
Addressed all four items: see inline replies, added the docs page `docs/sql-ref-is-json.md`, and updated the PR description (only SCALAR is new)
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub)" <gi...@apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4392542354
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala:
##########
@@ -291,6 +291,31 @@ class ExpressionParserSuite extends AnalysisTest {
assertEqual("a is not distinct from b", $"a" <=> $"b")
}
+ test("SPARK-58793: is json expressions") {
+ assertEqual("a is json", IsJson($"a", IsJsonShape.Any))
+ assertEqual("a is not json", !IsJson($"a", IsJsonShape.Any))
+ assertEqual("a is json value", IsJson($"a", IsJsonShape.Value))
+ assertEqual("a is json object", IsJson($"a", IsJsonShape.Object))
+ assertEqual("a is not json array", !IsJson($"a", IsJsonShape.Array))
+ assertEqual("a is json scalar", IsJson($"a", IsJsonShape.Scalar))
+ // A non-type ARRAY/MAP/STRUCT keyword must not leave the lexer in complex-type mode, which
+ // would make a later `>>` or `>>>` lex as two `>` tokens and fail to parse.
+ assertEqual("(a is json array) = (b >> 1)",
+ IsJson($"a", IsJsonShape.Array) === ShiftRight($"b", 1))
+ assertEqual("(a is json array) = (b >>> 1)",
+ IsJson($"a", IsJsonShape.Array) === ShiftRightUnsigned($"b", 1))
+ assertEqual("array(a) = array(b >> 1)",
+ $"array".function($"a") === $"array".function(ShiftRight($"b", 1)))
+ assertEqual("map(a, b >> 1)", $"map".function($"a", ShiftRight($"b", 1)))
+ assertEqual("struct(a, b >>> 1)", CreateStruct(Seq($"a", ShiftRightUnsigned($"b", 1))))
+ // Angle-bracketed complex types still split consecutive closing brackets, including when
+ // whitespace or comments separate the keyword from '<'.
+ assertEqual("cast(a as array<array<int>>) = b >> 1",
+ Cast($"a", ArrayType(ArrayType(IntegerType))) === ShiftRight($"b", 1))
+ assertEqual("cast(a as map /* c */ <int, array -- c\n <int>>) = b >> 1",
Review Comment:
Fixed.
A loop over six comment forms, each placed before every keyword, plus the bitwise.sql change.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub)" <gi...@apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4353908443
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/json/JsonExpressionUtils.java:
##########
@@ -117,4 +118,62 @@ public static UTF8String jsonTypeof(UTF8String json) {
return null;
}
}
+
+ // Shape codes for the SQL `IS [NOT] JSON` predicate, kept in sync with IsJsonShape on the
+ // Scala side. SHAPE_ANY covers both bare `IS JSON` and `IS JSON VALUE` (any well-formed value).
+ public static final int SHAPE_ANY = 0;
+ public static final int SHAPE_OBJECT = 1;
+ public static final int SHAPE_ARRAY = 2;
+ public static final int SHAPE_SCALAR = 3;
+
+ // A strict JSON factory for the ANSI SQL `IS [NOT] JSON` predicate. Unlike the shared factory
+ // used by the Hive-compatible JSON functions, it leaves ALLOW_SINGLE_QUOTES and
+ // ALLOW_UNESCAPED_CONTROL_CHARS disabled, so only well-formed ANSI JSON is accepted
+ // (e.g. `{'a':1}` with single quotes is rejected).
+ private static final JsonFactory STRICT_JSON_FACTORY = new JsonFactory();
+
+ /**
+ * Evaluates the SQL `IS JSON` predicate for a single, already-evaluated operand: returns whether
+ * {@code json} is a well-formed ANSI JSON value of the requested {@code shape}. The operand is
+ * read exactly once here, so a non-deterministic input is evaluated a single time (unlike a
+ * plan that references the child in more than one place). NULL handling (NULL input yields a
+ * NULL result) is left to the calling expression; this method is only invoked for non-null
+ * input and always returns a definite true/false.
+ */
+ public static boolean isJson(UTF8String json, int shape) {
+ try (JsonParser jsonParser =
+ CreateJacksonParser.utf8String(STRICT_JSON_FACTORY, json)) {
Review Comment:
Fixed.
isJson now checks UTF8String.isValid() before parsing, so invalid UTF-8 returns false. Added tests.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub) via reviews" <re...@spark.apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4474125314
##########
sql/core/src/test/resources/sql-tests/inputs/json-functions.sql:
##########
@@ -335,3 +335,133 @@ select json_object('a' VALUE '{"b":1}');
select json_object('a' VALUE 1 RETURNING INT);
-- JSON_OBJECT error: malformed FORMAT JSON value
select json_object('a' VALUE '{bad' FORMAT JSON);
+
+-- IS [NOT] JSON
+-- basic well-formedness
+select '{"a":1}' IS JSON;
+select '[1,2]' IS JSON;
+select '42' IS JSON;
+select '"hello"' IS JSON;
+select 'not json' IS JSON;
+select '' IS JSON;
+-- IS NOT JSON
+select '{"a":1}' IS NOT JSON;
+select 'bad' IS NOT JSON;
+-- NULL propagates
+select cast(null as string) IS JSON;
+select cast(null as string) IS NOT JSON;
+-- shape qualifiers
+select '{"a":1}' IS JSON OBJECT;
+select '[1,2]' IS JSON OBJECT;
+select '[1,2]' IS JSON ARRAY;
+select '{"a":1}' IS JSON ARRAY;
+-- IS JSON VALUE: any valid JSON value (including objects and arrays)
+select '42' IS JSON VALUE;
+select '"hello"' IS JSON VALUE;
+select '{"a":1}' IS JSON VALUE;
+select '[1,2]' IS JSON VALUE;
+select 'bad' IS JSON VALUE;
+-- IS JSON SCALAR: only primitives (string, number, boolean, null -- not object/array)
+select '42' IS JSON SCALAR;
+select '"hello"' IS JSON SCALAR;
+select 'true' IS JSON SCALAR;
+select 'false' IS JSON SCALAR;
+select '[1]' IS JSON SCALAR;
+select '{"a":1}' IS JSON SCALAR;
+-- IS NOT JSON with shape qualifiers
+select '{"a":1}' IS NOT JSON OBJECT;
+select '[1,2]' IS NOT JSON ARRAY;
+select '{"a":1}' IS NOT JSON VALUE;
+select 'bad' IS NOT JSON VALUE;
+select '{"a":1}' IS NOT JSON SCALAR;
+select '42' IS NOT JSON SCALAR;
+-- JSON null literal is valid JSON and counts as a scalar
+select 'null' IS JSON;
+select 'null' IS JSON SCALAR;
+select 'null' IS JSON VALUE;
+select 'null' IS JSON OBJECT;
+-- strict ANSI parsing: Hive-style leniency (single quotes) is rejected
+select '{''a'':1}' IS JSON;
+select '[''a'', ''b'']' IS JSON;
+-- trailing content after a well-formed value is rejected
+select '1 2' IS JSON;
+select '{} []' IS JSON;
+-- other parser extensions are rejected: comments, unquoted names, NaN/Infinity, leading zeros
+select '/* c */ {}' IS JSON, '{a:1}' IS JSON, 'NaN' IS JSON, 'Infinity' IS JSON, '01' IS JSON;
+-- whitespace-only input is not a JSON value
+select ' ' IS JSON;
+-- duplicate field names are accepted
+select '{"a":1,"a":2}' IS JSON OBJECT;
+-- input beyond the parser limits raises an error, like parse_json
+select concat(repeat('[', 1001), repeat(']', 1001)) IS JSON;
+-- a shape mismatch is decided by the first token even for over-limit input: an over-long root
+-- number is neither an object nor an array, so those shapes return false instead of raising
+select repeat('9', 1001) IS JSON OBJECT, repeat('9', 1001) IS JSON ARRAY;
+-- structurally malformed containers are rejected: truncated input, trailing commas, missing
+-- values, and a mismatched closer
+select '{' IS JSON, '[1,' IS JSON, '{"a"' IS JSON;
+select '[1,]' IS JSON ARRAY, '{"a":1,}' IS JSON OBJECT, '[1,,2]' IS JSON ARRAY, '[1}' IS JSON ARRAY;
+-- input that is not valid UTF-8 is rejected (lone continuation byte, encoded surrogate)
+select cast(X'228022' AS STRING) IS JSON;
+select cast(X'22EDA08022' AS STRING) IS JSON;
+-- unpaired surrogate escapes are rejected in values and field names, also when nested
+select '"\\uD835"' IS JSON;
+select '"\\uDC00"' IS JSON SCALAR;
+select '{"\\uD835":1}' IS JSON OBJECT;
+select '[{"a":["\\uDC00"]}]' IS JSON ARRAY;
+select '"\\uD835\\uDC00"' IS JSON;
+-- shift operators after the ARRAY qualifier
+select '[1]' IS JSON ARRAY, 8 >> 1, 8 >>> 1;
+-- collated and trim-collated operands behave like STRING
+select '{"a":1}' COLLATE UTF8_LCASE IS JSON OBJECT, '[1] ' COLLATE UTF8_LCASE_RTRIM IS JSON ARRAY;
+select 'bad' COLLATE UNICODE IS JSON;
+-- by default a CHAR/VARCHAR cast target is rewritten to STRING at parse time, so this case
+-- exercises a STRING operand
+select cast('[1]' AS VARCHAR(10)) IS JSON ARRAY;
+-- with first-class CHAR/VARCHAR types (preserveCharVarcharTypeInfo or standard semantics), the
+-- cast keeps its type, so IS JSON gets a real VARCHAR or CHAR operand
Review Comment:
Fixed.
The comments now describe each config separately: preserveCharVarcharTypeInfo keeps the CHAR/VARCHAR type, while standard semantics keeps the type at parse time but type coercion promotes the operand to STRING. A CHAR case was added to the standard-semantics block, and the analyzer golden pins the cast(cast(... as char(10)) as string) promotion.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub)" <gi...@apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4353936165
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/json/JsonExpressionUtils.java:
##########
@@ -117,4 +118,62 @@ public static UTF8String jsonTypeof(UTF8String json) {
return null;
}
}
+
+ // Shape codes for the SQL `IS [NOT] JSON` predicate, kept in sync with IsJsonShape on the
+ // Scala side. SHAPE_ANY covers both bare `IS JSON` and `IS JSON VALUE` (any well-formed value).
+ public static final int SHAPE_ANY = 0;
+ public static final int SHAPE_OBJECT = 1;
+ public static final int SHAPE_ARRAY = 2;
+ public static final int SHAPE_SCALAR = 3;
+
+ // A strict JSON factory for the ANSI SQL `IS [NOT] JSON` predicate. Unlike the shared factory
+ // used by the Hive-compatible JSON functions, it leaves ALLOW_SINGLE_QUOTES and
+ // ALLOW_UNESCAPED_CONTROL_CHARS disabled, so only well-formed ANSI JSON is accepted
+ // (e.g. `{'a':1}` with single quotes is rejected).
+ private static final JsonFactory STRICT_JSON_FACTORY = new JsonFactory();
+
+ /**
+ * Evaluates the SQL `IS JSON` predicate for a single, already-evaluated operand: returns whether
+ * {@code json} is a well-formed ANSI JSON value of the requested {@code shape}. The operand is
+ * read exactly once here, so a non-deterministic input is evaluated a single time (unlike a
+ * plan that references the child in more than one place). NULL handling (NULL input yields a
+ * NULL result) is left to the calling expression; this method is only invoked for non-null
+ * input and always returns a definite true/false.
+ */
+ public static boolean isJson(UTF8String json, int shape) {
+ try (JsonParser jsonParser =
+ CreateJacksonParser.utf8String(STRICT_JSON_FACTORY, json)) {
+ JsonToken token = jsonParser.nextToken();
Review Comment:
Fixed.
`isJson` now walks all tokens and rejects unpaired surrogates in string values and field names, same as VariantBuilder.
Added tests.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "spark-test (via GitHub) via reviews" <re...@spark.apache.org>.
spark-test commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4472316912
##########
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:
**Nit (P3):** This block covers three of the four documented limits. Nothing reaches `IS_JSON_INPUT_LIMIT_EXCEEDED.STRING_LENGTH`. That branch rests on two premises no test checks. First, `limitError` must recognize Jackson's "String value length" message. Second, the token walk must keep decoding every string value through `getText()`, which is what makes Jackson enforce the limit.
If a Jackson upgrade rewords the message, the error silently becomes `UNKNOWN`. If the walk stops decoding strings, an over-long string returns true. A case with a string value of `StreamReadConstraints.DEFAULT_MAX_STRING_LEN + 1` characters, as `JsonFunctionsSuite` already builds, would pin both.
**Verification:**
- **Regression:** The new case fails if the string-length message stops being recognized, which would yield the UNKNOWN sub-condition, or if string values stop being decoded during the walk, which would yield true.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/json/JsonExpressionUtils.java:
##########
@@ -117,4 +122,151 @@ public static UTF8String jsonTypeof(UTF8String json) {
return null;
}
}
+
+ // Shape codes for the SQL `IS [NOT] JSON` predicate, kept in sync with IsJsonShape on the
+ // Scala side. SHAPE_ANY covers both bare `IS JSON` and `IS JSON VALUE` (any well-formed value).
+ public static final int SHAPE_ANY = 0;
+ public static final int SHAPE_OBJECT = 1;
+ public static final int SHAPE_ARRAY = 2;
+ public static final int SHAPE_SCALAR = 3;
+
+ // A strict JSON factory for the ANSI SQL `IS [NOT] JSON` predicate. Unlike the shared factory
+ // used by the Hive-compatible JSON functions, it leaves ALLOW_SINGLE_QUOTES and
+ // ALLOW_UNESCAPED_CONTROL_CHARS disabled, so only well-formed ANSI JSON is accepted
+ // (e.g. `{'a':1}` with single quotes is rejected).
+ private static final JsonFactory STRICT_JSON_FACTORY = new JsonFactory();
+
+ /**
+ * Evaluates the SQL `IS JSON` predicate for a single, already-evaluated operand: returns whether
+ * {@code json} is a well-formed ANSI JSON value of the requested {@code shape}. The operand is
+ * read exactly once here, so a non-deterministic input is evaluated a single time (unlike a
+ * plan that references the child in more than one place). NULL handling (NULL input yields a
+ * NULL result) is left to the calling expression; this method is only invoked for non-null
+ * input. Malformed input yields false. Input that exceeds one of Jackson's default
+ * {@code StreamReadConstraints} (nesting depth, number length, string length, or field name
+ * length) cannot be classified and raises an IS_JSON_INPUT_LIMIT_EXCEEDED condition, at the
+ * same limits where {@code parse_json} fails -- but only if checking the input actually
+ * requires going past a limit: a shape mismatch decided by the first token (including an
+ * over-long root number under OBJECT or ARRAY, which cannot be an object or an array)
+ * returns false, like any other mismatch.
+ */
+ public static boolean isJson(UTF8String json, int shape) {
+ // The parser decodes the bytes through a UTF-8 reader that silently replaces malformed
+ // sequences with U+FFFD, so check the raw bytes first: input that is not valid UTF-8
+ // (e.g. a lone continuation byte, or an encoded surrogate) is not well-formed JSON text.
+ if (!json.isValid()) {
+ return false;
+ }
+ try (JsonParser jsonParser =
+ CreateJacksonParser.utf8String(STRICT_JSON_FACTORY, json)) {
+ JsonToken token;
+ try {
+ token = jsonParser.nextToken();
+ } catch (StreamConstraintsException e) {
+ // Jackson validates a number's length while producing its token, and no other limit can
+ // be reached while producing the first token (nesting needs a second token, and strings
Review Comment:
**Nit (P3):** The parenthetical is half right. Jackson defers decoding string values until `getText()`, but it decodes a field name when `nextToken()` produces the `FIELD_NAME` token, and the name is available from `currentName()` without `getText()`. The conclusion still holds because the first token can never be a field name, and the comment could give that reason instead: nesting needs a second token, string values are decoded lazily, and a first token is never a field name. As written, it misleads anyone who extends this handler or the trailing-content check about which limits can fire where.
**Verification:**
- **Inspection:** The first-token handler's comment no longer claims that field names are decoded only at getText, and it justifies the over-long-number premise from the kinds of token that can come first.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -2689,3 +2689,70 @@ object JsonObjectExpr {
case _ => false
}
}
+
+// Shape qualifier for IS [NOT] JSON: which top-level JSON type to require.
+sealed trait IsJsonShape
+object IsJsonShape {
+ // Per ANSI SQL:2016 T821, IS JSON VALUE is semantically equivalent to bare IS JSON:
+ // both accept any well-formed JSON value (object, array, or scalar). They are kept as
+ // separate shapes so the sql() output round-trips faithfully.
+ case object Any extends IsJsonShape // IS JSON (no qualifier)
+ case object Value extends IsJsonShape // IS JSON VALUE
+ case object Scalar extends IsJsonShape // IS JSON SCALAR -- primitives only (not object/array)
+ case object Array extends IsJsonShape // IS JSON ARRAY
+ case object Object extends IsJsonShape // IS JSON OBJECT
+}
+
+case class IsJson(child: Expression, shape: IsJsonShape = IsJsonShape.Any)
Review Comment:
**Nit (P3):** `IsJson` is built by the parser but is neither a `Predicate` nor listed in `ResolverGuard.isGenerallySupportedExpression`, so the single-pass analyzer's guard rejects every `IS [NOT] JSON` query, and `NOT (... IS JSON)` through its child. Its parser-built scalar siblings `JsonArray`, `JsonObjectExpr`, `JsonValue`, `JsonQuery` and `JsonExists` are all in the guard's JSON arm and in ResolverGuardSuite's "SQL JSON constructor and path functions" test. `JSON_OBJECT` added its entry in the same change that introduced it.
As a result, IS JSON gets no single-pass resolution and no dual-run comparison in tests, where `spark.sql.analyzer.singlePassResolver.dualRunWithLegacy` defaults to true. Results under the default configuration are unaffected, because the single-pass resolver is off. Adding `IsJson` to that JSON arm, with guard-suite cases, would close the gap.
**Verification:**
- **Behavior:** The resolver guard accepts SELECT '{}' IS JSON, SELECT '{}' IS NOT JSON OBJECT and SELECT NULL IS JSON.
- **Compatibility:** With dual-run analysis enabled, IS JSON queries over STRING, collated, CHAR/VARCHAR and untyped NULL operands produce identical plans from both analyzers.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/json/JsonExpressionUtils.java:
##########
@@ -117,4 +122,151 @@ public static UTF8String jsonTypeof(UTF8String json) {
return null;
}
}
+
+ // Shape codes for the SQL `IS [NOT] JSON` predicate, kept in sync with IsJsonShape on the
+ // Scala side. SHAPE_ANY covers both bare `IS JSON` and `IS JSON VALUE` (any well-formed value).
+ public static final int SHAPE_ANY = 0;
+ public static final int SHAPE_OBJECT = 1;
+ public static final int SHAPE_ARRAY = 2;
+ public static final int SHAPE_SCALAR = 3;
+
+ // A strict JSON factory for the ANSI SQL `IS [NOT] JSON` predicate. Unlike the shared factory
+ // used by the Hive-compatible JSON functions, it leaves ALLOW_SINGLE_QUOTES and
+ // ALLOW_UNESCAPED_CONTROL_CHARS disabled, so only well-formed ANSI JSON is accepted
+ // (e.g. `{'a':1}` with single quotes is rejected).
+ private static final JsonFactory STRICT_JSON_FACTORY = new JsonFactory();
+
+ /**
+ * Evaluates the SQL `IS JSON` predicate for a single, already-evaluated operand: returns whether
+ * {@code json} is a well-formed ANSI JSON value of the requested {@code shape}. The operand is
+ * read exactly once here, so a non-deterministic input is evaluated a single time (unlike a
+ * plan that references the child in more than one place). NULL handling (NULL input yields a
+ * NULL result) is left to the calling expression; this method is only invoked for non-null
+ * input. Malformed input yields false. Input that exceeds one of Jackson's default
+ * {@code StreamReadConstraints} (nesting depth, number length, string length, or field name
+ * length) cannot be classified and raises an IS_JSON_INPUT_LIMIT_EXCEEDED condition, at the
+ * same limits where {@code parse_json} fails -- but only if checking the input actually
+ * requires going past a limit: a shape mismatch decided by the first token (including an
+ * over-long root number under OBJECT or ARRAY, which cannot be an object or an array)
+ * returns false, like any other mismatch.
+ */
+ public static boolean isJson(UTF8String json, int shape) {
+ // The parser decodes the bytes through a UTF-8 reader that silently replaces malformed
+ // sequences with U+FFFD, so check the raw bytes first: input that is not valid UTF-8
+ // (e.g. a lone continuation byte, or an encoded surrogate) is not well-formed JSON text.
+ if (!json.isValid()) {
+ return false;
+ }
+ try (JsonParser jsonParser =
+ CreateJacksonParser.utf8String(STRICT_JSON_FACTORY, json)) {
+ JsonToken token;
+ try {
+ token = jsonParser.nextToken();
+ } catch (StreamConstraintsException e) {
+ // Jackson validates a number's length while producing its token, and no other limit can
+ // be reached while producing the first token (nesting needs a second token, and strings
+ // and field names are not decoded until getText), so the first value is an over-long
+ // number. It can never be an object or an array, so the shape is decided without
+ // classifying the number; the other shapes accept a number and still need it classified.
+ if (shape == SHAPE_OBJECT || shape == SHAPE_ARRAY) {
+ return false;
+ }
+ throw limitError(e, json);
+ }
+ if (token == null) {
+ // Empty input is not a JSON value.
+ return false;
+ }
+ boolean matchesShape = switch (shape) {
+ case SHAPE_OBJECT -> token == JsonToken.START_OBJECT;
+ case SHAPE_ARRAY -> token == JsonToken.START_ARRAY;
+ case SHAPE_SCALAR -> switch (token) {
+ case VALUE_STRING, VALUE_NUMBER_INT, VALUE_NUMBER_FLOAT,
+ VALUE_TRUE, VALUE_FALSE, VALUE_NULL -> true;
+ default -> false;
+ };
+ // SHAPE_ANY: any well-formed JSON value (object, array, or scalar).
+ default -> switch (token) {
+ case START_OBJECT, START_ARRAY, VALUE_STRING, VALUE_NUMBER_INT,
+ VALUE_NUMBER_FLOAT, VALUE_TRUE, VALUE_FALSE, VALUE_NULL -> true;
+ default -> false;
+ };
+ };
+ if (!matchesShape) {
+ return false;
+ }
+ // Walk the whole value token by token (rather than `skipChildren`, which does not look at
+ // string contents) so that malformed input (e.g. `{`, `[1,`) surfaces as a parse error
+ // and every string value and field name can be checked for unpaired surrogates.
+ int depth = 0;
+ do {
+ switch (token) {
+ case START_OBJECT, START_ARRAY -> depth++;
+ case END_OBJECT, END_ARRAY -> depth--;
+ case FIELD_NAME, VALUE_STRING -> {
+ if (hasUnpairedSurrogate(jsonParser.getText())) {
+ return false;
+ }
+ }
+ default -> { }
+ }
+ } while (depth > 0 && (token = jsonParser.nextToken()) != null);
+ if (depth > 0) {
+ return false;
+ }
+ // Reject trailing content after the first value, e.g. `1 2` or `{} []`, so only a single
+ // well-formed JSON value is accepted.
+ return jsonParser.nextToken() == null;
Review Comment:
**Nit (P3):** Any trailing token already decides the answer, but this call runs inside the outer `try` whose `StreamConstraintsException` handler raises `IS_JSON_INPUT_LIMIT_EXCEEDED`. Suppose a complete value is followed by a number longer than 1000 characters, such as `concat('{} ', repeat('9', 1001))` under `IS JSON OBJECT` or `concat('1 ', repeat('9', 1001))` under `IS JSON`. Jackson rejects that trailing number while producing its token, the same eager check the first-token handler relies on. The query then fails with `NUMBER_LENGTH` instead of returning false (true for `IS NOT JSON`).
That contradicts the Javadoc and `sql-ref-is-json.md`, which say trailing content returns false and the error applies only when checking the input requires going past a limit. It affects every shape whose first token matches, so one such row aborts a `WHERE payload IS JSON` filter. Giving this call its own handler that returns false would fix it. No test covers trailing over-limit content yet.
This extends an [existing discussion](https://github.com/apache/spark/pull/58024#issuecomment-6587687004).
**Verification:**
- **Regression:** For every shape whose first token matches (no qualifier or VALUE, OBJECT, ARRAY and SCALAR), a complete value followed by whitespace and an integer or floating-point number longer than the maximum number length returns false, and the IS NOT JSON form returns true, under interpreted and generated evaluation. Examples: concat('{} ', repeat('9', 1001)) IS JSON OBJECT, concat('[1] ', repeat('9', 1001)) IS JSON ARRAY, concat('1 ', repeat('9', 1001)) IS JSON and concat('"a" 0.', repeat('1', 1001)) IS JSON SCALAR.
- **Behavior:** Existing limit cases still raise NESTING_DEPTH, NUMBER_LENGTH and NAME_LENGTH with the same parameters, and trailing content such as '1 2' and '{} []' still returns false.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala:
##########
@@ -291,6 +291,45 @@ class ExpressionParserSuite extends AnalysisTest {
assertEqual("a is not distinct from b", $"a" <=> $"b")
}
+ test("SPARK-58793: is json expressions") {
+ assertEqual("a is json", IsJson($"a", IsJsonShape.Any))
+ assertEqual("a is not json", !IsJson($"a", IsJsonShape.Any))
+ assertEqual("a is json value", IsJson($"a", IsJsonShape.Value))
+ assertEqual("a is json object", IsJson($"a", IsJsonShape.Object))
+ assertEqual("a is not json array", !IsJson($"a", IsJsonShape.Array))
+ assertEqual("a is json scalar", IsJson($"a", IsJsonShape.Scalar))
+ // A non-type ARRAY/MAP/STRUCT keyword must not leave the lexer in complex-type mode, which
+ // would make a later `>>` or `>>>` lex as two `>` tokens and fail to parse.
+ assertEqual("(a is json array) = (b >> 1)",
+ IsJson($"a", IsJsonShape.Array) === ShiftRight($"b", 1))
+ assertEqual("(a is json array) = (b >>> 1)",
+ IsJson($"a", IsJsonShape.Array) === ShiftRightUnsigned($"b", 1))
+ assertEqual("array(a) = array(b >> 1)",
+ $"array".function($"a") === $"array".function(ShiftRight($"b", 1)))
+ assertEqual("map(a, b >> 1)", $"map".function($"a", ShiftRight($"b", 1)))
+ assertEqual("struct(a, b >>> 1)", CreateStruct(Seq($"a", ShiftRightUnsigned($"b", 1))))
+ // Angle-bracketed complex types still split consecutive closing brackets, including when
+ // whitespace or comments separate the keyword from '<'.
+ assertEqual("cast(a as array<array<int>>) = b >> 1",
+ Cast($"a", ArrayType(ArrayType(IntegerType))) === ShiftRight($"b", 1))
+ // Each comment form precedes every type keyword, so the closers only split if skipping that
Review Comment:
**Nit (P3):** The inputs below put each comment form after every type keyword, between the keyword and its `<` (`array $c<array $c<int>>`). That is the gap `isFollowedByLessThan` has to skip. A comment placed before a keyword would not exercise the lookahead at all, so this description is inverted. Someone who follows it could move the comments and silently lose the coverage the loop exists to provide. Saying "after every type keyword, before its `<`" would match the inputs.
**Verification:**
- **Inspection:** The comment above the comment-form loop describes each comment as following a type keyword and preceding its '<', matching the inputs.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala:
##########
@@ -291,6 +291,45 @@ class ExpressionParserSuite extends AnalysisTest {
assertEqual("a is not distinct from b", $"a" <=> $"b")
}
+ test("SPARK-58793: is json expressions") {
+ assertEqual("a is json", IsJson($"a", IsJsonShape.Any))
+ assertEqual("a is not json", !IsJson($"a", IsJsonShape.Any))
+ assertEqual("a is json value", IsJson($"a", IsJsonShape.Value))
+ assertEqual("a is json object", IsJson($"a", IsJsonShape.Object))
+ assertEqual("a is not json array", !IsJson($"a", IsJsonShape.Array))
+ assertEqual("a is json scalar", IsJson($"a", IsJsonShape.Scalar))
+ // A non-type ARRAY/MAP/STRUCT keyword must not leave the lexer in complex-type mode, which
+ // would make a later `>>` or `>>>` lex as two `>` tokens and fail to parse.
+ assertEqual("(a is json array) = (b >> 1)",
+ IsJson($"a", IsJsonShape.Array) === ShiftRight($"b", 1))
+ assertEqual("(a is json array) = (b >>> 1)",
+ IsJson($"a", IsJsonShape.Array) === ShiftRightUnsigned($"b", 1))
+ assertEqual("array(a) = array(b >> 1)",
+ $"array".function($"a") === $"array".function(ShiftRight($"b", 1)))
+ assertEqual("map(a, b >> 1)", $"map".function($"a", ShiftRight($"b", 1)))
+ assertEqual("struct(a, b >>> 1)", CreateStruct(Seq($"a", ShiftRightUnsigned($"b", 1))))
+ // Angle-bracketed complex types still split consecutive closing brackets, including when
+ // whitespace or comments separate the keyword from '<'.
+ assertEqual("cast(a as array<array<int>>) = b >> 1",
+ Cast($"a", ArrayType(ArrayType(IntegerType))) === ShiftRight($"b", 1))
+ // Each comment form precedes every type keyword, so the closers only split if skipping that
+ // comment form works: a single counted keyword would be enough to keep `>>` apart.
+ Seq("/* c */", "/* a /* b */ c */", "-- c\n", "-- c\r\n", "-- c\r", "-- c \\\n c\n")
Review Comment:
**Nit (P3):** These cases separate each keyword from `<` with an ASCII space and a comment. `isFollowedByLessThan` also has to skip the WS rule's no-break spaces (U+00A0, U+2007 and U+202F), and only its `Character.isSpaceChar` check does that. If that disjunct were dropped, or the check narrowed to `Character.isWhitespace`, a type such as `cast(a as array\u00A0<array\u00A0<int>>)` would lex its closers as a shift and fail to parse, although it parses on master, and every test would stay green. Adding the three no-break spaces, written as Unicode escapes, to these two- and three-level cases would pin the lexer comment's claim that the check covers every character of the WS rule.
**Verification:**
- **Regression:** The new cases fail if the separator check is narrowed to Character.isWhitespace, because the closers would then lex as a shift operator.
##########
sql/core/src/test/resources/sql-tests/inputs/json-functions.sql:
##########
@@ -335,3 +335,133 @@ select json_object('a' VALUE '{"b":1}');
select json_object('a' VALUE 1 RETURNING INT);
-- JSON_OBJECT error: malformed FORMAT JSON value
select json_object('a' VALUE '{bad' FORMAT JSON);
+
+-- IS [NOT] JSON
+-- basic well-formedness
+select '{"a":1}' IS JSON;
+select '[1,2]' IS JSON;
+select '42' IS JSON;
+select '"hello"' IS JSON;
+select 'not json' IS JSON;
+select '' IS JSON;
+-- IS NOT JSON
+select '{"a":1}' IS NOT JSON;
+select 'bad' IS NOT JSON;
+-- NULL propagates
+select cast(null as string) IS JSON;
+select cast(null as string) IS NOT JSON;
+-- shape qualifiers
+select '{"a":1}' IS JSON OBJECT;
+select '[1,2]' IS JSON OBJECT;
+select '[1,2]' IS JSON ARRAY;
+select '{"a":1}' IS JSON ARRAY;
+-- IS JSON VALUE: any valid JSON value (including objects and arrays)
+select '42' IS JSON VALUE;
+select '"hello"' IS JSON VALUE;
+select '{"a":1}' IS JSON VALUE;
+select '[1,2]' IS JSON VALUE;
+select 'bad' IS JSON VALUE;
+-- IS JSON SCALAR: only primitives (string, number, boolean, null -- not object/array)
+select '42' IS JSON SCALAR;
+select '"hello"' IS JSON SCALAR;
+select 'true' IS JSON SCALAR;
+select 'false' IS JSON SCALAR;
+select '[1]' IS JSON SCALAR;
+select '{"a":1}' IS JSON SCALAR;
+-- IS NOT JSON with shape qualifiers
+select '{"a":1}' IS NOT JSON OBJECT;
+select '[1,2]' IS NOT JSON ARRAY;
+select '{"a":1}' IS NOT JSON VALUE;
+select 'bad' IS NOT JSON VALUE;
+select '{"a":1}' IS NOT JSON SCALAR;
+select '42' IS NOT JSON SCALAR;
+-- JSON null literal is valid JSON and counts as a scalar
+select 'null' IS JSON;
+select 'null' IS JSON SCALAR;
+select 'null' IS JSON VALUE;
+select 'null' IS JSON OBJECT;
+-- strict ANSI parsing: Hive-style leniency (single quotes) is rejected
+select '{''a'':1}' IS JSON;
+select '[''a'', ''b'']' IS JSON;
+-- trailing content after a well-formed value is rejected
+select '1 2' IS JSON;
+select '{} []' IS JSON;
+-- other parser extensions are rejected: comments, unquoted names, NaN/Infinity, leading zeros
+select '/* c */ {}' IS JSON, '{a:1}' IS JSON, 'NaN' IS JSON, 'Infinity' IS JSON, '01' IS JSON;
+-- whitespace-only input is not a JSON value
+select ' ' IS JSON;
+-- duplicate field names are accepted
+select '{"a":1,"a":2}' IS JSON OBJECT;
+-- input beyond the parser limits raises an error, like parse_json
+select concat(repeat('[', 1001), repeat(']', 1001)) IS JSON;
+-- a shape mismatch is decided by the first token even for over-limit input: an over-long root
+-- number is neither an object nor an array, so those shapes return false instead of raising
+select repeat('9', 1001) IS JSON OBJECT, repeat('9', 1001) IS JSON ARRAY;
+-- structurally malformed containers are rejected: truncated input, trailing commas, missing
+-- values, and a mismatched closer
+select '{' IS JSON, '[1,' IS JSON, '{"a"' IS JSON;
+select '[1,]' IS JSON ARRAY, '{"a":1,}' IS JSON OBJECT, '[1,,2]' IS JSON ARRAY, '[1}' IS JSON ARRAY;
+-- input that is not valid UTF-8 is rejected (lone continuation byte, encoded surrogate)
+select cast(X'228022' AS STRING) IS JSON;
+select cast(X'22EDA08022' AS STRING) IS JSON;
+-- unpaired surrogate escapes are rejected in values and field names, also when nested
+select '"\\uD835"' IS JSON;
+select '"\\uDC00"' IS JSON SCALAR;
+select '{"\\uD835":1}' IS JSON OBJECT;
+select '[{"a":["\\uDC00"]}]' IS JSON ARRAY;
+select '"\\uD835\\uDC00"' IS JSON;
+-- shift operators after the ARRAY qualifier
+select '[1]' IS JSON ARRAY, 8 >> 1, 8 >>> 1;
+-- collated and trim-collated operands behave like STRING
+select '{"a":1}' COLLATE UTF8_LCASE IS JSON OBJECT, '[1] ' COLLATE UTF8_LCASE_RTRIM IS JSON ARRAY;
+select 'bad' COLLATE UNICODE IS JSON;
+-- by default a CHAR/VARCHAR cast target is rewritten to STRING at parse time, so this case
+-- exercises a STRING operand
+select cast('[1]' AS VARCHAR(10)) IS JSON ARRAY;
+-- with first-class CHAR/VARCHAR types (preserveCharVarcharTypeInfo or standard semantics), the
+-- cast keeps its type, so IS JSON gets a real VARCHAR or CHAR operand
Review Comment:
**Nit (P3):** This is true under `preserveCharVarcharTypeInfo` but not under standard semantics. There, type coercion promotes the operand to STRING, as the analyzer golden records: `is_json(cast(cast([1] as varchar(10)) as string), Array)`. The standard-semantics block also has only VARCHAR casts, so CHAR promotion is not exercised there. The comment could describe each config separately, and a CHAR case in the standard-semantics block, with the golden files regenerated, would close the gap.
**Verification:**
- **Behavior:** Under standard semantics, cast('[1]' AS CHAR(10)) IS JSON ARRAY returns true, and its analyzed plan shows the operand promoted to STRING.
- **Inspection:** The comment describes real VARCHAR and CHAR operands only under preserveCharVarcharTypeInfo, and STRING promotion under standard semantics.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #58024:
URL: https://github.com/apache/spark/pull/58024#issuecomment-5492952720
Also, ANSI-invalid JSON is accepted.
IsJson delegates to JsonTypeof, whose [shared parser](https://github.com/apache/spark/blob/db76f06661a30aa0f42e8767fe775638d4692213/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/json/JsonExpressionEvalUtils.scala#L109-L114) enables single quotes and unescaped control characters for Hive compatibility.
```
SELECT '{''a'':1}' IS JSON;
```
This returns true, although the [JIRA specification](https://issues.apache.org/jira/browse/SPARK-58793) requires well-formed ANSI JSON. The predicate needs strict parsing.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #58024:
URL: https://github.com/apache/spark/pull/58024#issuecomment-5492971361
I'm adding @cloud-fan who has more context here regarding jsonExpressions
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub)" <gi...@apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4392482699
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseLexer.g4:
##########
@@ -80,10 +80,52 @@ import java.util.Deque;
public int complex_type_level_counter = 0;
/**
- * Increase the counter by one when hits KEYWORD 'ARRAY', 'MAP', 'STRUCT'.
+ * Increase the counter by one when hits KEYWORD 'ARRAY', 'MAP', 'STRUCT' that starts an
+ * angle-bracketed complex type, such as ARRAY<INT>. Other uses of these keywords, such as the
+ * ARRAY(1, 2) function or the `IS JSON ARRAY` predicate, have no matching '>' and must leave
+ * the counter unchanged, otherwise a later '>>' could not be lexed as a shift operator.
*/
public void incComplexTypeLevelCounter() {
- complex_type_level_counter++;
+ if (isFollowedByLessThan()) complex_type_level_counter++;
+ }
+
+ /**
+ * Returns whether the next character after the current token, skipping whitespace and
+ * comments, is '<'. Called from a lexer action, where the input is positioned right after the
+ * matched keyword.
+ */
+ private boolean isFollowedByLessThan() {
+ int i = 1;
+ while (true) {
+ int c = _input.LA(i);
+ if (c != CharStream.EOF && (Character.isWhitespace(c) || Character.isSpaceChar(c))) {
+ // Covers every character of the WS rule, including no-break spaces.
+ i++;
+ } else if (c == '-' && _input.LA(i + 1) == '-') {
+ // Simple comment: skip to the end of the line, honoring backslash line continuations.
+ i += 2;
+ while (_input.LA(i) != '\n' && _input.LA(i) != CharStream.EOF) {
Review Comment:
fixed; the lookahead now stops at a bare CR the same way the lexer's comment rule does.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r3903434129
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -1631,3 +1631,66 @@ case class JsonTypeof(child: Expression)
override protected def withNewChildInternal(newChild: Expression): JsonTypeof =
copy(child = newChild)
}
+
+// Shape qualifier for IS [NOT] JSON: which top-level JSON type to require.
+sealed trait IsJsonShape
+object IsJsonShape {
+ // Per ANSI SQL:2016 T801, IS JSON VALUE is semantically equivalent to bare IS JSON:
+ // both accept any well-formed JSON value (object, array, or scalar). They are kept as
+ // separate shapes so the sql() output round-trips faithfully.
+ case object Any extends IsJsonShape // IS JSON (no qualifier)
+ case object Value extends IsJsonShape // IS JSON VALUE
+ case object Scalar extends IsJsonShape // IS JSON SCALAR -- primitives only (not object/array)
+ case object Array extends IsJsonShape // IS JSON ARRAY
+ case object Object extends IsJsonShape // IS JSON OBJECT
+}
+
+case class IsJson(child: Expression, shape: IsJsonShape = IsJsonShape.Any)
+ extends UnaryExpression
+ with RuntimeReplaceable
+ with ExpectsInputTypes {
+
+ override def inputTypes: Seq[AbstractDataType] =
+ Seq(StringTypeWithCollation(supportsTrimCollation = true))
+
+ override def dataType: DataType = BooleanType
+ // NULL input propagates as NULL (three-valued logic). Invalid JSON yields false, not NULL,
+ // because json_typeof returns NULL for both cases and the If(IsNull(child), ...) wrapper
+ // in replacement distinguishes them before the shape check runs.
+ override def nullable: Boolean = true
+
+ override def prettyName: String = "is_json"
+
+ override def sql: String = {
+ val shapeSql = shape match {
+ case IsJsonShape.Any => ""
+ case IsJsonShape.Value => " VALUE"
+ case IsJsonShape.Object => " OBJECT"
+ case IsJsonShape.Array => " ARRAY"
+ case IsJsonShape.Scalar => " SCALAR"
+ }
+ s"(${child.sql} IS JSON$shapeSql)"
+ }
+
+ // json_typeof returns NULL for both null input and invalid JSON. The If(IsNull) wrapper
+ // propagates NULL input as NULL; for invalid JSON json_typeof also returns NULL but the
+ // child is not NULL, so shapeCheck evaluates to false via IsNotNull.
+ override def replacement: Expression = {
Review Comment:
replacement evaluates child in IsNull and again in JsonTypeof; qualified predicates may parse it twice.
Nondeterministic operands can therefore return incorrect results.
```
SELECT IF(rand(0) < 0.5, CAST(NULL AS STRING), '{}') IS JSON
FROM range(10000);
```
Correct results can only be NULL or true; this implementation can produce false.
Bind/evaluate the operand once, preferably in a dedicated validation helper.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub)" <gi...@apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4065627064
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -1631,3 +1631,66 @@ case class JsonTypeof(child: Expression)
override protected def withNewChildInternal(newChild: Expression): JsonTypeof =
copy(child = newChild)
}
+
+// Shape qualifier for IS [NOT] JSON: which top-level JSON type to require.
+sealed trait IsJsonShape
+object IsJsonShape {
+ // Per ANSI SQL:2016 T801, IS JSON VALUE is semantically equivalent to bare IS JSON:
+ // both accept any well-formed JSON value (object, array, or scalar). They are kept as
+ // separate shapes so the sql() output round-trips faithfully.
+ case object Any extends IsJsonShape // IS JSON (no qualifier)
+ case object Value extends IsJsonShape // IS JSON VALUE
+ case object Scalar extends IsJsonShape // IS JSON SCALAR -- primitives only (not object/array)
+ case object Array extends IsJsonShape // IS JSON ARRAY
+ case object Object extends IsJsonShape // IS JSON OBJECT
+}
+
+case class IsJson(child: Expression, shape: IsJsonShape = IsJsonShape.Any)
+ extends UnaryExpression
+ with RuntimeReplaceable
+ with ExpectsInputTypes {
+
+ override def inputTypes: Seq[AbstractDataType] =
+ Seq(StringTypeWithCollation(supportsTrimCollation = true))
+
+ override def dataType: DataType = BooleanType
+ // NULL input propagates as NULL (three-valued logic). Invalid JSON yields false, not NULL,
+ // because json_typeof returns NULL for both cases and the If(IsNull(child), ...) wrapper
+ // in replacement distinguishes them before the shape check runs.
+ override def nullable: Boolean = true
+
+ override def prettyName: String = "is_json"
+
+ override def sql: String = {
+ val shapeSql = shape match {
+ case IsJsonShape.Any => ""
+ case IsJsonShape.Value => " VALUE"
+ case IsJsonShape.Object => " OBJECT"
+ case IsJsonShape.Array => " ARRAY"
+ case IsJsonShape.Scalar => " SCALAR"
+ }
+ s"(${child.sql} IS JSON$shapeSql)"
+ }
+
+ // json_typeof returns NULL for both null input and invalid JSON. The If(IsNull) wrapper
+ // propagates NULL input as NULL; for invalid JSON json_typeof also returns NULL but the
+ // child is not NULL, so shapeCheck evaluates to false via IsNotNull.
+ override def replacement: Expression = {
Review Comment:
Fixed.
replacement is now a single StaticInvoke into JsonExpressionUtils.isJson, so the operand is read once. NULL propagation is handled by StaticInvoke, so the result can only be NULL or true.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub) via reviews" <re...@spark.apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4474109409
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -2689,3 +2689,70 @@ object JsonObjectExpr {
case _ => false
}
}
+
+// Shape qualifier for IS [NOT] JSON: which top-level JSON type to require.
+sealed trait IsJsonShape
+object IsJsonShape {
+ // Per ANSI SQL:2016 T821, IS JSON VALUE is semantically equivalent to bare IS JSON:
+ // both accept any well-formed JSON value (object, array, or scalar). They are kept as
+ // separate shapes so the sql() output round-trips faithfully.
+ case object Any extends IsJsonShape // IS JSON (no qualifier)
+ case object Value extends IsJsonShape // IS JSON VALUE
+ case object Scalar extends IsJsonShape // IS JSON SCALAR -- primitives only (not object/array)
+ case object Array extends IsJsonShape // IS JSON ARRAY
+ case object Object extends IsJsonShape // IS JSON OBJECT
+}
+
+case class IsJson(child: Expression, shape: IsJsonShape = IsJsonShape.Any)
Review Comment:
Fixed.
IsJson is added to the JSON arm of ResolverGuard.isGenerallySupportedExpression, and ResolverGuardSuite gets cases for SELECT '{}' IS JSON, SELECT '{}' IS NOT JSON OBJECT and SELECT NULL IS JSON.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #58024:
URL: https://github.com/apache/spark/pull/58024#issuecomment-5306741863
LGTM, thank you @vinodkc!
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #58024:
URL: https://github.com/apache/spark/pull/58024#issuecomment-5492955234
Also, there are some conflicts now
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub) via reviews" <re...@spark.apache.org>.
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: 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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #58024:
URL: https://github.com/apache/spark/pull/58024#issuecomment-6434731013
Thanks for the updates. The UTF-8 validation, the unpaired-surrogate check, the lexer counter change, and the new IS JSON reference page are all in the current head. The PR description still says `OBJECT` and `SCALAR` are new keywords, though; only `SCALAR` is new, so that sentence still needs the edit.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"conversation:6352229512","thread_id":"conversation:6352229512","verdict_sha256":"38cf435f3ba8bf60d83b17dd355dd38452a4bcf26c8a70b16e14d3ea33ac3f9b"} -->
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub)" <gi...@apache.org>.
vinodkc commented on PR #58024:
URL: https://github.com/apache/spark/pull/58024#issuecomment-5766247967
Fixed.
The helper now uses a strict JsonFactory, so '{''a'':1}' IS JSON returns false. Added tests.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4068683249
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -1417,6 +1417,7 @@ predicate
| IS errorCapturingNot? kind=NULL
| IS errorCapturingNot? kind=(TRUE | FALSE | UNKNOWN)
| IS errorCapturingNot? kind=DISTINCT FROM right=valueExpression
+ | IS errorCapturingNot? kind=JSON shape=(VALUE | ARRAY | OBJECT | SCALAR)?
Review Comment:
**Non-blocking (P2):** This new non-type use of `ARRAY` leaves `complex_type_level_counter` elevated because the lexer action increments on every ARRAY token and only `GT` decrements it. A query such as `SELECT '[1]' IS JSON ARRAY, 8 >> 1` then cannot emit `SHIFT_RIGHT` and fails to parse. The lexer state decision needs to distinguish actual angle-bracketed complex types from non-type ARRAY uses.
**Recommended change:** Confine complex-type counter increments to ARRAY/MAP/STRUCT uses that actually begin angle-bracketed type syntax, preserving nested type closing behavior while leaving non-type keywords state-neutral; add composition coverage for JSON qualifiers followed by signed and unsigned right shifts.
**Why this works:** Move the complex-type-state decision to the lexer owner by distinguishing angle-bracketed type starts from ordinary keyword occurrences before incrementing. Do not patch the JSON parser branch with a context-specific reset that would leave other non-type uses broken or depend on token-stream timing.
**Scope:** Correct the shared lexer state boundary and verify both the new predicate composition and existing nested complex-type tokenization.
**Compatibility:** All existing complex-type, ARRAY expression, SQL/JSON wrapper, and shift-operator grammar remains accepted with its prior meaning.
**Risks:** Lookahead that identifies an angle-bracketed type start must handle permitted whitespace and comments without changing ordinary ARRAY/MAP/STRUCT token recognition.
**Constraints:** Nested complex types must continue splitting adjacent closing angle brackets rather than lexing them as shift operators.
**Success:** IS JSON ARRAY followed by >> or >>> in the same statement parses and evaluates normally. Non-type ARRAY keyword uses do not leave complex_type_level_counter elevated. Nested ARRAY/MAP/STRUCT type declarations continue to tokenize consecutive closing angle brackets correctly.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/json/JsonExpressionUtils.java:
##########
@@ -117,4 +118,62 @@ public static UTF8String jsonTypeof(UTF8String json) {
return null;
}
}
+
+ // Shape codes for the SQL `IS [NOT] JSON` predicate, kept in sync with IsJsonShape on the
+ // Scala side. SHAPE_ANY covers both bare `IS JSON` and `IS JSON VALUE` (any well-formed value).
+ public static final int SHAPE_ANY = 0;
+ public static final int SHAPE_OBJECT = 1;
+ public static final int SHAPE_ARRAY = 2;
+ public static final int SHAPE_SCALAR = 3;
+
+ // A strict JSON factory for the ANSI SQL `IS [NOT] JSON` predicate. Unlike the shared factory
+ // used by the Hive-compatible JSON functions, it leaves ALLOW_SINGLE_QUOTES and
+ // ALLOW_UNESCAPED_CONTROL_CHARS disabled, so only well-formed ANSI JSON is accepted
+ // (e.g. `{'a':1}` with single quotes is rejected).
+ private static final JsonFactory STRICT_JSON_FACTORY = new JsonFactory();
+
+ /**
+ * Evaluates the SQL `IS JSON` predicate for a single, already-evaluated operand: returns whether
+ * {@code json} is a well-formed ANSI JSON value of the requested {@code shape}. The operand is
+ * read exactly once here, so a non-deterministic input is evaluated a single time (unlike a
+ * plan that references the child in more than one place). NULL handling (NULL input yields a
+ * NULL result) is left to the calling expression; this method is only invoked for non-null
+ * input and always returns a definite true/false.
+ */
+ public static boolean isJson(UTF8String json, int shape) {
+ try (JsonParser jsonParser =
+ CreateJacksonParser.utf8String(STRICT_JSON_FACTORY, json)) {
Review Comment:
**Non-blocking (P2):** `CreateJacksonParser.utf8String` wraps these bytes in an `InputStreamReader`, whose UTF-8 decoder replaces malformed sequences before Jackson sees them. A reachable value such as `CAST(X'228022' AS STRING)` therefore becomes a quoted U+FFFD string and returns true even though the original text is not valid UTF-8 JSON. Please validate the original UTF8String bytes before replacement decoding and add a malformed-byte regression case.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/json/JsonExpressionUtils.java:
##########
@@ -117,4 +118,62 @@ public static UTF8String jsonTypeof(UTF8String json) {
return null;
}
}
+
+ // Shape codes for the SQL `IS [NOT] JSON` predicate, kept in sync with IsJsonShape on the
+ // Scala side. SHAPE_ANY covers both bare `IS JSON` and `IS JSON VALUE` (any well-formed value).
+ public static final int SHAPE_ANY = 0;
+ public static final int SHAPE_OBJECT = 1;
+ public static final int SHAPE_ARRAY = 2;
+ public static final int SHAPE_SCALAR = 3;
+
+ // A strict JSON factory for the ANSI SQL `IS [NOT] JSON` predicate. Unlike the shared factory
+ // used by the Hive-compatible JSON functions, it leaves ALLOW_SINGLE_QUOTES and
+ // ALLOW_UNESCAPED_CONTROL_CHARS disabled, so only well-formed ANSI JSON is accepted
+ // (e.g. `{'a':1}` with single quotes is rejected).
+ private static final JsonFactory STRICT_JSON_FACTORY = new JsonFactory();
+
+ /**
+ * Evaluates the SQL `IS JSON` predicate for a single, already-evaluated operand: returns whether
+ * {@code json} is a well-formed ANSI JSON value of the requested {@code shape}. The operand is
+ * read exactly once here, so a non-deterministic input is evaluated a single time (unlike a
+ * plan that references the child in more than one place). NULL handling (NULL input yields a
+ * NULL result) is left to the calling expression; this method is only invoked for non-null
+ * input and always returns a definite true/false.
+ */
+ public static boolean isJson(UTF8String json, int shape) {
+ try (JsonParser jsonParser =
+ CreateJacksonParser.utf8String(STRICT_JSON_FACTORY, json)) {
+ JsonToken token = jsonParser.nextToken();
Review Comment:
**Non-blocking (P2):** Jackson emits inputs such as `"\uD835"` as `VALUE_STRING`; `skipChildren()` does not validate its contents, so this helper returns true. Spark's strict `VariantBuilder` JSON path rejects lone high and low surrogates in values and object keys while accepting valid pairs. Please apply equivalent validation while traversing the complete token stream, including nested strings and field names.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4385471348
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseLexer.g4:
##########
@@ -80,10 +80,52 @@ import java.util.Deque;
public int complex_type_level_counter = 0;
/**
- * Increase the counter by one when hits KEYWORD 'ARRAY', 'MAP', 'STRUCT'.
+ * Increase the counter by one when hits KEYWORD 'ARRAY', 'MAP', 'STRUCT' that starts an
+ * angle-bracketed complex type, such as ARRAY<INT>. Other uses of these keywords, such as the
+ * ARRAY(1, 2) function or the `IS JSON ARRAY` predicate, have no matching '>' and must leave
+ * the counter unchanged, otherwise a later '>>' could not be lexed as a shift operator.
*/
public void incComplexTypeLevelCounter() {
- complex_type_level_counter++;
+ if (isFollowedByLessThan()) complex_type_level_counter++;
+ }
+
+ /**
+ * Returns whether the next character after the current token, skipping whitespace and
+ * comments, is '<'. Called from a lexer action, where the input is positioned right after the
+ * matched keyword.
+ */
+ private boolean isFollowedByLessThan() {
+ int i = 1;
+ while (true) {
+ int c = _input.LA(i);
+ if (c != CharStream.EOF && (Character.isWhitespace(c) || Character.isSpaceChar(c))) {
+ // Covers every character of the WS rule, including no-break spaces.
+ i++;
+ } else if (c == '-' && _input.LA(i + 1) == '-') {
+ // Simple comment: skip to the end of the line, honoring backslash line continuations.
+ i += 2;
+ while (_input.LA(i) != '\n' && _input.LA(i) != CharStream.EOF) {
Review Comment:
**Blocking (P1):** `isFollowedByLessThan` ends a `--` comment only at LF, but the lexer's `SIMPLE_COMMENT` rule (line 765) also ends one at a bare CR. When SQL text uses bare-CR line endings and a `--` comment sits between `ARRAY`/`MAP`/`STRUCT` and its `<`, this loop runs past the `<` to EOF (or the next LF), the lookahead returns false, and the keyword is no longer counted. Once enough of the open keywords go uncounted (both, for a two-level type), the adjacent closers lex as a shift operator and a type that parses on master becomes a syntax error. This applies to every CAST target, DDL column type, and schema string this lexer parses; CRLF and LF line endings are unaffected.
I tokenized `CAST(NULL AS ARRAY -- c<CR><ARRAY -- c<CR><INT>>)` (`<CR>` is a bare carriage return) with lexers generated by ANTLR 4.13.1 from both revisions: master gives `LT LT GT GT`, this PR gives `LT LT SHIFT_RIGHT`. The three-level `>>>` and the MAP forms fail the same way. Also stopping at `'\r'` matches `SIMPLE_COMMENT`, and with that change every case lexes as on master while `SELECT array(1), 8 >> 1` and `'[1]' IS JSON ARRAY, 8 >> 1` still get `SHIFT_RIGHT`:
```suggestion
while (_input.LA(i) != '\n' && _input.LA(i) != '\r' && _input.LA(i) != CharStream.EOF) {
```
See **Shared repair plan 1** in the review body.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/JsonExpressionsSuite.scala:
##########
@@ -1138,4 +1138,81 @@ 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)") {
Review Comment:
**Nit (P3):** The strictness tests cover single quotes and raw control characters, but several false cases that `sql-ref-is-json.md` promises have no assertion: comments, unquoted field names, `NaN`/`Infinity`, leading zeros, and whitespace-only input. The documented acceptance of duplicate keys and the operand contract are untested too: there is no `UTF8_LCASE` or trim-collated operand, no `VARCHAR`, no untyped `NULL IS JSON` (the PR description's example, which relies on the `ExpectsInputTypes` NullType coercion), and no case showing that `1 IS JSON` is rejected. Enabling `ALLOW_NON_NUMERIC_NUMBERS` or `ALLOW_JAVA_COMMENTS` on the strict factory, turning on duplicate detection, or switching to `ImplicitCastInputTypes` (which would make `123 IS JSON` true) would all leave every current test green. A few cases here or in `json-functions.sql`, with the golden files regenerated, would pin the documented contract.
**Verification:**
- **Behavior:** Each documented false case (comments, unquoted field names, NaN or Infinity, leading zeros, whitespace-only input) makes IS JSON return false, and an object with duplicate field names makes it return true.
- **Behavior:** IS JSON over a UTF8_LCASE operand, a trim-collated operand, and a VARCHAR operand returns the same results as over a plain STRING; untyped `NULL IS JSON` returns NULL; and `1 IS JSON` fails analysis with a data-type mismatch.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala:
##########
@@ -291,6 +291,31 @@ class ExpressionParserSuite extends AnalysisTest {
assertEqual("a is not distinct from b", $"a" <=> $"b")
}
+ test("SPARK-58793: is json expressions") {
+ assertEqual("a is json", IsJson($"a", IsJsonShape.Any))
+ assertEqual("a is not json", !IsJson($"a", IsJsonShape.Any))
+ assertEqual("a is json value", IsJson($"a", IsJsonShape.Value))
+ assertEqual("a is json object", IsJson($"a", IsJsonShape.Object))
+ assertEqual("a is not json array", !IsJson($"a", IsJsonShape.Array))
+ assertEqual("a is json scalar", IsJson($"a", IsJsonShape.Scalar))
+ // A non-type ARRAY/MAP/STRUCT keyword must not leave the lexer in complex-type mode, which
+ // would make a later `>>` or `>>>` lex as two `>` tokens and fail to parse.
+ assertEqual("(a is json array) = (b >> 1)",
+ IsJson($"a", IsJsonShape.Array) === ShiftRight($"b", 1))
+ assertEqual("(a is json array) = (b >>> 1)",
+ IsJson($"a", IsJsonShape.Array) === ShiftRightUnsigned($"b", 1))
+ assertEqual("array(a) = array(b >> 1)",
+ $"array".function($"a") === $"array".function(ShiftRight($"b", 1)))
+ assertEqual("map(a, b >> 1)", $"map".function($"a", ShiftRight($"b", 1)))
+ assertEqual("struct(a, b >>> 1)", CreateStruct(Seq($"a", ShiftRightUnsigned($"b", 1))))
+ // Angle-bracketed complex types still split consecutive closing brackets, including when
+ // whitespace or comments separate the keyword from '<'.
+ assertEqual("cast(a as array<array<int>>) = b >> 1",
+ Cast($"a", ArrayType(ArrayType(IntegerType))) === ShiftRight($"b", 1))
+ assertEqual("cast(a as map /* c */ <int, array -- c\n <int>>) = b >> 1",
Review Comment:
**Nit (P3):** These comment cases can't catch a regression in a single comment-skipping branch of `isFollowedByLessThan`. Here `map` has a bracketed comment and `array` a `--` comment, so if either branch alone breaks, the other keyword is still counted and `>>` still splits; the case fails only when both branches break. The new `bitwise.sql` case comments only the outer `array`, so it passes even with no comment skipping at all. I checked both with lexers generated from this grammar with one or both branches forced to return false. To isolate a branch, the same comment form has to precede every type keyword of the nested type, e.g. `array /* c */ <array /* c */ <int>>` for `>>` and a three-level type for `>>>`. A comment before only the inner keyword doesn't work either, because the outer keyword is still counted directly. Bare-CR, CRLF, nested bracketed, and backslash-continued `--` comments have no case yet; the bare-CR form is the one that currently breaks.
This extends an [existing discussion](https://github.com/apache/spark/pull/58024#discussion_r4068683249).
See **Shared repair plan 1** in the review body.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/json/JsonExpressionUtils.java:
##########
@@ -117,4 +118,62 @@ public static UTF8String jsonTypeof(UTF8String json) {
return null;
}
}
+
+ // Shape codes for the SQL `IS [NOT] JSON` predicate, kept in sync with IsJsonShape on the
+ // Scala side. SHAPE_ANY covers both bare `IS JSON` and `IS JSON VALUE` (any well-formed value).
+ public static final int SHAPE_ANY = 0;
+ public static final int SHAPE_OBJECT = 1;
+ public static final int SHAPE_ARRAY = 2;
+ public static final int SHAPE_SCALAR = 3;
+
+ // A strict JSON factory for the ANSI SQL `IS [NOT] JSON` predicate. Unlike the shared factory
+ // used by the Hive-compatible JSON functions, it leaves ALLOW_SINGLE_QUOTES and
+ // ALLOW_UNESCAPED_CONTROL_CHARS disabled, so only well-formed ANSI JSON is accepted
+ // (e.g. `{'a':1}` with single quotes is rejected).
+ private static final JsonFactory STRICT_JSON_FACTORY = new JsonFactory();
+
+ /**
+ * Evaluates the SQL `IS JSON` predicate for a single, already-evaluated operand: returns whether
+ * {@code json} is a well-formed ANSI JSON value of the requested {@code shape}. The operand is
+ * read exactly once here, so a non-deterministic input is evaluated a single time (unlike a
+ * plan that references the child in more than one place). NULL handling (NULL input yields a
+ * NULL result) is left to the calling expression; this method is only invoked for non-null
+ * input and always returns a definite true/false.
+ */
+ public static boolean isJson(UTF8String json, int shape) {
+ try (JsonParser jsonParser =
+ CreateJacksonParser.utf8String(STRICT_JSON_FACTORY, json)) {
Review Comment:
Thanks, verified. `isJson` now returns false when `UTF8String.isValid()` fails, before decoding, and the `X'228022'` and `X'22EDA08022'` cases cover it.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:4068683245","thread_id":"inline:4068683245","verdict_sha256":"38cf435f3ba8bf60d83b17dd355dd38452a4bcf26c8a70b16e14d3ea33ac3f9b"} -->
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -1417,6 +1417,7 @@ predicate
| IS errorCapturingNot? kind=NULL
| IS errorCapturingNot? kind=(TRUE | FALSE | UNKNOWN)
| IS errorCapturingNot? kind=DISTINCT FROM right=valueExpression
+ | IS errorCapturingNot? kind=JSON shape=(VALUE | ARRAY | OBJECT | SCALAR)?
Review Comment:
Thanks, the counter now moves only when the keyword is followed by `<`, and the `IS JSON ARRAY` and `array(...)` cases followed by `>>`/`>>>` parse. This review also raises a testing follow-up: the lookahead's comment cases don't fail if only one of its comment-skipping branches regresses.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:4068683249","thread_id":"inline:4068683249","verdict_sha256":"38cf435f3ba8bf60d83b17dd355dd38452a4bcf26c8a70b16e14d3ea33ac3f9b"} -->
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/json/JsonExpressionUtils.java:
##########
@@ -117,4 +118,62 @@ public static UTF8String jsonTypeof(UTF8String json) {
return null;
}
}
+
+ // Shape codes for the SQL `IS [NOT] JSON` predicate, kept in sync with IsJsonShape on the
+ // Scala side. SHAPE_ANY covers both bare `IS JSON` and `IS JSON VALUE` (any well-formed value).
+ public static final int SHAPE_ANY = 0;
+ public static final int SHAPE_OBJECT = 1;
+ public static final int SHAPE_ARRAY = 2;
+ public static final int SHAPE_SCALAR = 3;
+
+ // A strict JSON factory for the ANSI SQL `IS [NOT] JSON` predicate. Unlike the shared factory
+ // used by the Hive-compatible JSON functions, it leaves ALLOW_SINGLE_QUOTES and
+ // ALLOW_UNESCAPED_CONTROL_CHARS disabled, so only well-formed ANSI JSON is accepted
+ // (e.g. `{'a':1}` with single quotes is rejected).
+ private static final JsonFactory STRICT_JSON_FACTORY = new JsonFactory();
+
+ /**
+ * Evaluates the SQL `IS JSON` predicate for a single, already-evaluated operand: returns whether
+ * {@code json} is a well-formed ANSI JSON value of the requested {@code shape}. The operand is
+ * read exactly once here, so a non-deterministic input is evaluated a single time (unlike a
+ * plan that references the child in more than one place). NULL handling (NULL input yields a
+ * NULL result) is left to the calling expression; this method is only invoked for non-null
+ * input and always returns a definite true/false.
+ */
+ public static boolean isJson(UTF8String json, int shape) {
+ try (JsonParser jsonParser =
+ CreateJacksonParser.utf8String(STRICT_JSON_FACTORY, json)) {
+ JsonToken token = jsonParser.nextToken();
Review Comment:
Thanks, verified. The full token walk now rejects unpaired surrogate escapes in both values and field names at any depth, and correctly paired escapes still pass.
<!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:4068683254","thread_id":"inline:4068683254","verdict_sha256":"38cf435f3ba8bf60d83b17dd355dd38452a4bcf26c8a70b16e14d3ea33ac3f9b"} -->
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub) via reviews" <re...@spark.apache.org>.
vinodkc commented on PR #58024:
URL: https://github.com/apache/spark/pull/58024#issuecomment-6587687004
Thanks. All four findings are fixed and the question is answered below.
**The root-number question.** Confirmed by running the queries: `repeat('9', 1001) IS JSON` in all shapes raises with the number-length reason, while the over-long string and over-deep array return false under OBJECT. Chose the first option: a `StreamConstraintsException` from the first `nextToken()` is a shape mismatch for OBJECT/ARRAY . with default constraints only an over-long number can raise there, so the first value is a number, which can never be an object or an array. VALUE and SCALAR still raise. Documented in `sql-ref-is-json.md` and the Javadoc; pinned by suite and golden tests.
**`<reason>` parameter.** Reshaped into sub-conditions like `COLLECTION_SIZE_LIMIT_EXCEEDED`: `IS_JSON_INPUT_LIMIT_EXCEEDED.{NESTING_DEPTH, NUMBER_LENGTH, STRING_LENGTH, NAME_LENGTH}` each carry the numeric maximum as a parameter, plus `UNKNOWN` for an unrecognized Jackson message. The golden file now records `"errorClass": "IS_JSON_INPUT_LIMIT_EXCEEDED.NESTING_DEPTH"` with `"maxNestingDepth": "1000"`.
**VARCHAR operand.** The default-config case is labeled a STRING case, and `json-functions.sql` now runs CHAR and VARCHAR cases under both configs. The goldens pin both paths: real `varchar(10)`/`char(10)` operands under `preserveCharVarcharTypeInfo`, and the `cast(cast(... as varchar(10)) as string)` promotion under standard semantics.
**Javadoc.** Reworded to the reference-page rule: the error is raised only when checking the input requires going past a limit; a shape mismatch decided by the first token returns false. Consistent with the behavior chosen above.
**Malformed containers.** Added: truncated (`{`, `[1,`), trailing commas (`[1,]`, `{"a":1,}`), missing values (`[1,,2]`), missing colon (`{"a"}`), mismatched closers (`[1}`), in both `JsonExpressionsSuite` (codegen and interpreted, plus matching-shape variants) and the golden files
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub) via reviews" <re...@spark.apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4474115088
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/json/JsonExpressionUtils.java:
##########
@@ -117,4 +122,151 @@ public static UTF8String jsonTypeof(UTF8String json) {
return null;
}
}
+
+ // Shape codes for the SQL `IS [NOT] JSON` predicate, kept in sync with IsJsonShape on the
+ // Scala side. SHAPE_ANY covers both bare `IS JSON` and `IS JSON VALUE` (any well-formed value).
+ public static final int SHAPE_ANY = 0;
+ public static final int SHAPE_OBJECT = 1;
+ public static final int SHAPE_ARRAY = 2;
+ public static final int SHAPE_SCALAR = 3;
+
+ // A strict JSON factory for the ANSI SQL `IS [NOT] JSON` predicate. Unlike the shared factory
+ // used by the Hive-compatible JSON functions, it leaves ALLOW_SINGLE_QUOTES and
+ // ALLOW_UNESCAPED_CONTROL_CHARS disabled, so only well-formed ANSI JSON is accepted
+ // (e.g. `{'a':1}` with single quotes is rejected).
+ private static final JsonFactory STRICT_JSON_FACTORY = new JsonFactory();
+
+ /**
+ * Evaluates the SQL `IS JSON` predicate for a single, already-evaluated operand: returns whether
+ * {@code json} is a well-formed ANSI JSON value of the requested {@code shape}. The operand is
+ * read exactly once here, so a non-deterministic input is evaluated a single time (unlike a
+ * plan that references the child in more than one place). NULL handling (NULL input yields a
+ * NULL result) is left to the calling expression; this method is only invoked for non-null
+ * input. Malformed input yields false. Input that exceeds one of Jackson's default
+ * {@code StreamReadConstraints} (nesting depth, number length, string length, or field name
+ * length) cannot be classified and raises an IS_JSON_INPUT_LIMIT_EXCEEDED condition, at the
+ * same limits where {@code parse_json} fails -- but only if checking the input actually
+ * requires going past a limit: a shape mismatch decided by the first token (including an
+ * over-long root number under OBJECT or ARRAY, which cannot be an object or an array)
+ * returns false, like any other mismatch.
+ */
+ public static boolean isJson(UTF8String json, int shape) {
+ // The parser decodes the bytes through a UTF-8 reader that silently replaces malformed
+ // sequences with U+FFFD, so check the raw bytes first: input that is not valid UTF-8
+ // (e.g. a lone continuation byte, or an encoded surrogate) is not well-formed JSON text.
+ if (!json.isValid()) {
+ return false;
+ }
+ try (JsonParser jsonParser =
+ CreateJacksonParser.utf8String(STRICT_JSON_FACTORY, json)) {
+ JsonToken token;
+ try {
+ token = jsonParser.nextToken();
+ } catch (StreamConstraintsException e) {
+ // Jackson validates a number's length while producing its token, and no other limit can
+ // be reached while producing the first token (nesting needs a second token, and strings
Review Comment:
Fixed.
The comment now gives the correct reason: nesting needs a second token, string values are decoded lazily at getText(), and the first token is never a field name (a field name only appears inside an object, after START_OBJECT).
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub)" <gi...@apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4353943523
##########
sql/api/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBaseParser.g4:
##########
@@ -1417,6 +1417,7 @@ predicate
| IS errorCapturingNot? kind=NULL
| IS errorCapturingNot? kind=(TRUE | FALSE | UNKNOWN)
| IS errorCapturingNot? kind=DISTINCT FROM right=valueExpression
+ | IS errorCapturingNot? kind=JSON shape=(VALUE | ARRAY | OBJECT | SCALAR)?
Review Comment:
Fixed in the lexer.
ARRAY/MAP/STRUCT now increase the complex-type counter only when followed by <.
This also fixes SELECT array(1), 8 >> 1, which failed before.
Added tests.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub)" <gi...@apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4392534655
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/JsonExpressionsSuite.scala:
##########
@@ -1138,4 +1138,81 @@ 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)") {
Review Comment:
Fixed.
lists the strictness, duplicate-key and operand-type cases added
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub) via reviews" <re...@spark.apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4474103205
##########
sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/json/JsonExpressionUtils.java:
##########
@@ -117,4 +122,151 @@ public static UTF8String jsonTypeof(UTF8String json) {
return null;
}
}
+
+ // Shape codes for the SQL `IS [NOT] JSON` predicate, kept in sync with IsJsonShape on the
+ // Scala side. SHAPE_ANY covers both bare `IS JSON` and `IS JSON VALUE` (any well-formed value).
+ public static final int SHAPE_ANY = 0;
+ public static final int SHAPE_OBJECT = 1;
+ public static final int SHAPE_ARRAY = 2;
+ public static final int SHAPE_SCALAR = 3;
+
+ // A strict JSON factory for the ANSI SQL `IS [NOT] JSON` predicate. Unlike the shared factory
+ // used by the Hive-compatible JSON functions, it leaves ALLOW_SINGLE_QUOTES and
+ // ALLOW_UNESCAPED_CONTROL_CHARS disabled, so only well-formed ANSI JSON is accepted
+ // (e.g. `{'a':1}` with single quotes is rejected).
+ private static final JsonFactory STRICT_JSON_FACTORY = new JsonFactory();
+
+ /**
+ * Evaluates the SQL `IS JSON` predicate for a single, already-evaluated operand: returns whether
+ * {@code json} is a well-formed ANSI JSON value of the requested {@code shape}. The operand is
+ * read exactly once here, so a non-deterministic input is evaluated a single time (unlike a
+ * plan that references the child in more than one place). NULL handling (NULL input yields a
+ * NULL result) is left to the calling expression; this method is only invoked for non-null
+ * input. Malformed input yields false. Input that exceeds one of Jackson's default
+ * {@code StreamReadConstraints} (nesting depth, number length, string length, or field name
+ * length) cannot be classified and raises an IS_JSON_INPUT_LIMIT_EXCEEDED condition, at the
+ * same limits where {@code parse_json} fails -- but only if checking the input actually
+ * requires going past a limit: a shape mismatch decided by the first token (including an
+ * over-long root number under OBJECT or ARRAY, which cannot be an object or an array)
+ * returns false, like any other mismatch.
+ */
+ public static boolean isJson(UTF8String json, int shape) {
+ // The parser decodes the bytes through a UTF-8 reader that silently replaces malformed
+ // sequences with U+FFFD, so check the raw bytes first: input that is not valid UTF-8
+ // (e.g. a lone continuation byte, or an encoded surrogate) is not well-formed JSON text.
+ if (!json.isValid()) {
+ return false;
+ }
+ try (JsonParser jsonParser =
+ CreateJacksonParser.utf8String(STRICT_JSON_FACTORY, json)) {
+ JsonToken token;
+ try {
+ token = jsonParser.nextToken();
+ } catch (StreamConstraintsException e) {
+ // Jackson validates a number's length while producing its token, and no other limit can
+ // be reached while producing the first token (nesting needs a second token, and strings
+ // and field names are not decoded until getText), so the first value is an over-long
+ // number. It can never be an object or an array, so the shape is decided without
+ // classifying the number; the other shapes accept a number and still need it classified.
+ if (shape == SHAPE_OBJECT || shape == SHAPE_ARRAY) {
+ return false;
+ }
+ throw limitError(e, json);
+ }
+ if (token == null) {
+ // Empty input is not a JSON value.
+ return false;
+ }
+ boolean matchesShape = switch (shape) {
+ case SHAPE_OBJECT -> token == JsonToken.START_OBJECT;
+ case SHAPE_ARRAY -> token == JsonToken.START_ARRAY;
+ case SHAPE_SCALAR -> switch (token) {
+ case VALUE_STRING, VALUE_NUMBER_INT, VALUE_NUMBER_FLOAT,
+ VALUE_TRUE, VALUE_FALSE, VALUE_NULL -> true;
+ default -> false;
+ };
+ // SHAPE_ANY: any well-formed JSON value (object, array, or scalar).
+ default -> switch (token) {
+ case START_OBJECT, START_ARRAY, VALUE_STRING, VALUE_NUMBER_INT,
+ VALUE_NUMBER_FLOAT, VALUE_TRUE, VALUE_FALSE, VALUE_NULL -> true;
+ default -> false;
+ };
+ };
+ if (!matchesShape) {
+ return false;
+ }
+ // Walk the whole value token by token (rather than `skipChildren`, which does not look at
+ // string contents) so that malformed input (e.g. `{`, `[1,`) surfaces as a parse error
+ // and every string value and field name can be checked for unpaired surrogates.
+ int depth = 0;
+ do {
+ switch (token) {
+ case START_OBJECT, START_ARRAY -> depth++;
+ case END_OBJECT, END_ARRAY -> depth--;
+ case FIELD_NAME, VALUE_STRING -> {
+ if (hasUnpairedSurrogate(jsonParser.getText())) {
+ return false;
+ }
+ }
+ default -> { }
+ }
+ } while (depth > 0 && (token = jsonParser.nextToken()) != null);
+ if (depth > 0) {
+ return false;
+ }
+ // Reject trailing content after the first value, e.g. `1 2` or `{} []`, so only a single
+ // well-formed JSON value is accepted.
+ return jsonParser.nextToken() == null;
Review Comment:
Fixed.
The trailing nextToken() now has its own handler that returns false on a StreamConstraintsException, so trailing over-limit content returns false (true for IS NOT JSON) like any other trailing content. Javadoc and sql-ref-is-json.md now state the rule. Added suite cases for all four shapes plus golden cases.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub) via reviews" <re...@spark.apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4474130967
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala:
##########
@@ -291,6 +291,45 @@ class ExpressionParserSuite extends AnalysisTest {
assertEqual("a is not distinct from b", $"a" <=> $"b")
}
+ test("SPARK-58793: is json expressions") {
+ assertEqual("a is json", IsJson($"a", IsJsonShape.Any))
+ assertEqual("a is not json", !IsJson($"a", IsJsonShape.Any))
+ assertEqual("a is json value", IsJson($"a", IsJsonShape.Value))
+ assertEqual("a is json object", IsJson($"a", IsJsonShape.Object))
+ assertEqual("a is not json array", !IsJson($"a", IsJsonShape.Array))
+ assertEqual("a is json scalar", IsJson($"a", IsJsonShape.Scalar))
+ // A non-type ARRAY/MAP/STRUCT keyword must not leave the lexer in complex-type mode, which
+ // would make a later `>>` or `>>>` lex as two `>` tokens and fail to parse.
+ assertEqual("(a is json array) = (b >> 1)",
+ IsJson($"a", IsJsonShape.Array) === ShiftRight($"b", 1))
+ assertEqual("(a is json array) = (b >>> 1)",
+ IsJson($"a", IsJsonShape.Array) === ShiftRightUnsigned($"b", 1))
+ assertEqual("array(a) = array(b >> 1)",
+ $"array".function($"a") === $"array".function(ShiftRight($"b", 1)))
+ assertEqual("map(a, b >> 1)", $"map".function($"a", ShiftRight($"b", 1)))
+ assertEqual("struct(a, b >>> 1)", CreateStruct(Seq($"a", ShiftRightUnsigned($"b", 1))))
+ // Angle-bracketed complex types still split consecutive closing brackets, including when
+ // whitespace or comments separate the keyword from '<'.
+ assertEqual("cast(a as array<array<int>>) = b >> 1",
+ Cast($"a", ArrayType(ArrayType(IntegerType))) === ShiftRight($"b", 1))
+ // Each comment form precedes every type keyword, so the closers only split if skipping that
+ // comment form works: a single counted keyword would be enough to keep `>>` apart.
+ Seq("/* c */", "/* a /* b */ c */", "-- c\n", "-- c\r\n", "-- c\r", "-- c \\\n c\n")
Review Comment:
Fixed. The three no-break spaces are added to the two- and three-level cases, written as visible \u00A0 / \u2007 / \u202F escapes. They fail if the separator check is narrowed to Character.isWhitespace, because the closers would then lex as a shift.
--
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
Re: [PR] [SPARK-58793][SQL] Support ANSI SQL `IS [NOT] JSON` predicate [spark]
Posted by "vinodkc (via GitHub) via reviews" <re...@spark.apache.org>.
vinodkc commented on code in PR #58024:
URL: https://github.com/apache/spark/pull/58024#discussion_r4474137432
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala:
##########
@@ -291,6 +291,45 @@ class ExpressionParserSuite extends AnalysisTest {
assertEqual("a is not distinct from b", $"a" <=> $"b")
}
+ test("SPARK-58793: is json expressions") {
+ assertEqual("a is json", IsJson($"a", IsJsonShape.Any))
+ assertEqual("a is not json", !IsJson($"a", IsJsonShape.Any))
+ assertEqual("a is json value", IsJson($"a", IsJsonShape.Value))
+ assertEqual("a is json object", IsJson($"a", IsJsonShape.Object))
+ assertEqual("a is not json array", !IsJson($"a", IsJsonShape.Array))
+ assertEqual("a is json scalar", IsJson($"a", IsJsonShape.Scalar))
+ // A non-type ARRAY/MAP/STRUCT keyword must not leave the lexer in complex-type mode, which
+ // would make a later `>>` or `>>>` lex as two `>` tokens and fail to parse.
+ assertEqual("(a is json array) = (b >> 1)",
+ IsJson($"a", IsJsonShape.Array) === ShiftRight($"b", 1))
+ assertEqual("(a is json array) = (b >>> 1)",
+ IsJson($"a", IsJsonShape.Array) === ShiftRightUnsigned($"b", 1))
+ assertEqual("array(a) = array(b >> 1)",
+ $"array".function($"a") === $"array".function(ShiftRight($"b", 1)))
+ assertEqual("map(a, b >> 1)", $"map".function($"a", ShiftRight($"b", 1)))
+ assertEqual("struct(a, b >>> 1)", CreateStruct(Seq($"a", ShiftRightUnsigned($"b", 1))))
+ // Angle-bracketed complex types still split consecutive closing brackets, including when
+ // whitespace or comments separate the keyword from '<'.
+ assertEqual("cast(a as array<array<int>>) = b >> 1",
+ Cast($"a", ArrayType(ArrayType(IntegerType))) === ShiftRight($"b", 1))
+ // Each comment form precedes every type keyword, so the closers only split if skipping that
Review Comment:
Fixed.
The comment now says each comment form follows every type keyword and precedes its <. The PR description's test section had the same inversion and is fixed too.
--
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