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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
