Conversation
6f73bf5 to
361a314
Compare
7037198 to
c36c06c
Compare
c5b82da to
d38a346
Compare
Matching Spark's first-occurrence semantics for duplicate keys is the right fix, and keeping the full traversal so that malformed content after the match still rejects the row is a detail that would have been easy to get wrong. I think there is a case where the code does not do what the description says, though. A first match that resolves to nothing falls through to the second occurrence The description says:
But the code is: while let Some(matched) = map.next_key_seed(KeySeed(name))? {
if matched && !found.matched {
let candidate = map.next_value_seed(PathSeed { segments: &self.segments[1..], reject_direct_null: true })?;
if candidate.matched {
found = candidate;
}
} else {
map.next_value::<IgnoredAny>()?;
}
Two concrete cases I would expect to differ from Spark: SELECT get_json_object('{"a":{"b":1},"a":{"c":2}}', '$.a.b')Spark stops at the first SELECT get_json_object('{"a":null,"a":2}', '$.a')
Could you check that second case against Spark? If it does differ, the guard needs to be on whether the key was seen rather than on The change is bigger than the description says The description talks about A performance note on The |
|
Thanks for the review. I checked each point against Spark's 1. Lock semantics — the code matches Spark; it was the description that was stale Spark's object loop (lines 478-488) only skips later fields once
So guarding on "key was seen" (as the old description described) would actually diverge from Spark; locking on the first successful match is the correct semantics. A test for 2. Scope of the change Fair point — the description now covers the 3. The trailing |
a8b0928 to
1a7d25d
Compare
1a7d25d to
8db201e
Compare
| while let Some(matched) = map.next_key_seed(KeySeed(name))? { | ||
| if matched { | ||
| found = map.next_value_seed(PathSeed { | ||
| if matched && !found.matched { |
There was a problem hiding this comment.
This returns 1 for {"a":[[{"b":1}]],"a":null} with $.a[*][*].b, while Spark 4.1.3 and the pre-change native UDF return SQL NULL. Could you preserve Spark’s match decision here and add a regression test?
There was a problem hiding this comment.
Good catch — confirmed against Spark 4.1.3, and the root cause was deeper than the duplicate-key lock: Spark never treats [*][*] as two wildcards. Its parser emits Subscript :: Wildcard :: Subscript :: Wildcard, and evaluatePath consumes both at once (the "non-structure preserving double wildcard" case in JsonExpressionEvalUtils), applying the remaining path to the outer array's elements themselves in flatten style. So for {"a":[[{"b":1}]],"a":null} with $.a[*][*].b, the first a's outer element [{"b":1}] is an array and cannot match .b — nothing is written, dirty stays false — and the second a is null, hence SQL NULL.
Fixed in the latest commit: [*][*] now parses to a dedicated DoubleWildcard segment. The remaining path is applied to the outer elements with Spark's flatten style (array leaves are spliced recursively; an array that flattens to nothing writes no leaf nodes, so it is not a match), and the collected matches are always wrapped in a single array, even when there is only one — matching Spark's generator, which unconditionally wraps this case.
Regression coverage:
test_duplicate_key_double_wildcard_match_decisioncovers this exact input (-> NULL), the same shape without the duplicate key, and the fall-through to a later occurrence that does match ({"a":[[{"b":1}]],"a":[{"b":2}]}->[2]).- Five more unit tests pin the flatten semantics (one-level flatten mirroring Spark's
$.store.basket[*][*]suite case, single match staying wrapped, empty-flatten no-match, objects under[*][*].b, recursive flatten). - Added
[*][*]queries — including this exact query — tospark/src/test/resources/sql-tests/expressions/string/get_json_object.sql, so CI validates against Spark itself.CometSqlFileTestSuitepasses locally on Spark 4.1.3.
andygrove
left a comment
There was a problem hiding this comment.
You were right on the lock semantics and I was wrong. evaluatePath sets dirty only when something is written, and a named field whose value is VALUE_NULL returns false, so continuing on to a later duplicate is Spark's behaviour rather than a divergence from it. The updated description matches the code now.
I built the branch and checked the [*][*] fix against the expectations in Spark's own JsonExpressionsSuite rather than re-deriving them. $.store.basket[*][*], [*][0], [0][*], [*].category, [*].isbn, [*].reader and the non-existent-key cases all match. The parse-time merge is faithful too. Spark's parser emits Subscript :: Wildcard for each [*] and a bare Wildcard for .* and ['*'], so only the subscript form pairs, and evaluatePath pairs them greedily left to right in the same way a left-to-right merge does.
Two cases still differ, and I confirmed they differ identically on main at 58ab5f6, so neither comes from this PR:
$.store.basket[0][*].b Spark ["y"] Comet "y"
$.a[*].b[*] Spark [[1,2],[3]] Comet [1,2,3]
The first is a case in Spark's own suite. The cause is that Spark switches RawStyle to QuotedStyle on entering an array wildcard, and the wildcard arm under QuotedStyle wraps unconditionally, so only the outermost wildcard ever gets the single-match unwrap. evaluate_path collects into a flat Vec and decides the wrapping once at the top, which cannot represent that nesting. It is a redesign rather than a patch, so I would rather it did not ride along here. Could you file an issue for it and add those two queries to get_json_object.sql behind ignore(<issue link>), so the gap is recorded where the next person will look?
Approving. A pre-existing gap should not hold up a change that makes duplicate keys and [*][*] correct.
|
The previous CI failure in I pushed an empty commit to retrigger CI, but the new run is awaiting maintainer approval. Could a committer please approve the workflow run (or re-run the failed job)? Thanks! |
sunchao
left a comment
There was a problem hiding this comment.
Reviewed 7f54d2ca7277d10023e3c913c4a2729b9b6673b0 against base bb9e74020adc228e486f6f4d0fa68292b30bff31. Found two new correctness regressions in the opt-in native get_json_object implementation, detailed inline: a nested wildcard loses an array dimension, and wildcard extraction accepts an oversized skipped numeric token that Spark rejects. The default JVM-dispatched path is unaffected.
Validation: all 41 focused native tests passed with cargo test -p datafusion-comet-spark-expr get_json_object --locked --offline. Both findings were independently reproduced using the compiled head UDF in scalar/scalar, column/scalar, and column/column modes, compared with Spark 4.1.3's actual GetJsonObjectEvaluator and the exact source-extracted base evaluator. The full Comet JVM integration suite was not run locally, so these checks do not establish end-to-end Spark/Comet SQL execution.
Current-head Comet CI and CodeQL await workflow approval. The failed Spark 4.1 job in the earlier CI run stopped during dependency resolution with Connection reset downloading org.ow2.asm:asm:9.9; its merge tree differs from the reviewed head. Holding approval while the two reproduced regressions remain.
| if result.matched { | ||
| found.matched = true; | ||
| found.values.append(&mut result.values); |
There was a problem hiding this comment.
[P2] Preserve the quoted wrapper around nested wildcard results
For input [[[[[[[1]]]]]]] and path $[0][*][0][*][*], Spark 4.1.3 and the PR base return [[1]], but this head returns [1]. I also reproduced [1] through the compiled native scalar and both column entry points.
The first [0] followed by [*] makes Spark enter QuotedStyle, so that surrounding wildcard must retain its array wrapper even when only one child matches. The new double wildcard flattens the selected subtree, but this append and the final singleton serialization do not preserve the surrounding quoted wrapper. Although other nested-wildcard formatting gaps predate this PR, this particular input is correct on the base and regresses here.
Could you preserve Spark's per-level raw/quoted/flatten output styles, including the index-before-wildcard transition, and add this regression case to the SQL tests?
There was a problem hiding this comment.
Fixed in 7099e9d — thank you for the precise diagnosis; the QuotedStyle transition on [0] followed by [*] was the key.
Rather than patching the single case, I ported Spark's write styles end to end: Style {Raw, Quoted, Flatten} now propagates through the evaluation the way evaluatePath threads its style parameter, and each wildcard arm makes its own wrapper decision — Quoted style always keeps the wrapper, Raw/Flatten buffer the element writes and strip the outer brackets only for a lone writer. Results are modeled on Spark's generator protocol (a list of fragment writes plus the dirty flag), which turned out to matter beyond this case: the Quoted and double-wildcard arms write their brackets even when nothing inside matched, and Spark's generator keeps those bytes, so {"a":[[{}]],"a":[[{"b":1}]]} with $.a[0][*].b really produces [] [1] on 4.1.3 (root-level writes separated by a space). That is reproduced as well.
[[[[[[[1]]]]]]] / $[0][*][0][*][*] now returns [[1]], and the case is in get_json_object.sql together with the simpler $[0][*] shapes.
This also closes the two gaps recorded earlier in the thread: $.store.basket[0][*].b → ["y"] and $.a[*].b[*] → [[1,2],[3]], both now asserted positively.
Validation: a 600-case differential (20 documents × 30 paths) against Spark 4.1.3's GetJsonObjectEvaluator driven directly from the 4.1.3 catalyst jar; 596/600 match. The 4 mismatches are all $-on-duplicate-keys documents — serde_json's Value materialization collapses duplicate keys where Spark's token copy preserves them. That gap predates this PR (the pre-change UDF materializes Value the same way) and is unchanged by it.
| fn evaluate_path(json_str: &str, path: &ParsedPath) -> Option<String> { | ||
| if !path.has_wildcard { | ||
| return value_into_string(extract_no_wildcard(json_str, &path.segments)?); | ||
| let mut result = extract_path(json_str, &path.segments)?; |
There was a problem hiding this comment.
[P2] Preserve validation of oversized numeric tokens in skipped wildcard fields
For a document of the form [{"a":1,"b":<1001 consecutive nines>}] and path $[*].a, Spark 4.1.3 and the PR base return SQL NULL, while this head returns 1. Reversing the field order gives the same result. I reproduced this through the compiled native scalar and both column entry points.
Routing wildcard paths through extract_path makes the unselected numeric value go through IgnoredAny, which does not enforce Spark's numeric-token length constraint. Spark accepts the skipped 1000-digit token but rejects the 1001-digit token. The old wildcard parser also rejected the latter value, so this is a new wildcard regression even though an analogous non-wildcard compatibility gap already existed.
Could you preserve validation of skipped numeric tokens and add coverage on both sides of Spark's 1000-digit boundary?
There was a problem hiding this comment.
Fixed in 7099e9d. Since serde's visitor sees an already-parsed number (the token length is gone by visit_f64), the limit is enforced with a pre-parse byte scan mirroring Jackson's StreamReadConstraints.maxNumberLength = 1000: any number token longer than 1000 characters anywhere in the document returns null, while 1000-digit tokens are accepted — including digits inside string literals being ignored, which Jackson does not constrain.
Both sides of the boundary are covered in get_json_object.sql (via repeat('9', 1000/1001)) and in unit tests, for wildcard and non-wildcard paths.
One adjacent pre-existing gap, unchanged by this PR and worth recording: a selected number with 309–1000 digits parses fine in Jackson (copied verbatim) but overflows serde_json's f64 during Value materialization, so Comet returns null there. Fixing that needs raw-token capture in the materialized value; happy to file a follow-up issue if useful.
|
x |
sunchao
left a comment
There was a problem hiding this comment.
Re-reviewed 7099e9d2d657aa24267f5d34111107be66bc6dac against base bb9e74020adc228e486f6f4d0fa68292b30bff31 and the previously reviewed 7f54d2ca7277d10023e3c913c4a2729b9b6673b0. Both earlier concrete reproductions now match Spark: the nested wildcard retains [[1]], and the skipped positive 1,001-digit number returns SQL NULL. Two new issues in the numeric pre-scan remain, detailed inline: valid signed/fractional numbers are rejected at the length boundary, and scanning quoted strings byte by byte causes a substantial evaluator regression. These affect the opt-in native implementation (spark.comet.expression.GetJsonObject.allowIncompatible=true); the default JVM-dispatched implementation is unchanged.
Validation: all 47 focused native tests passed with cargo test -p datafusion-comet-spark-expr get_json_object --locked --offline. The new numeric counterexamples were independently checked against Spark 4.0.4 and 4.1.3, the exact base/prior/head evaluator sources, and the freshly compiled head UDF in scalar/scalar, column/scalar, and column/column modes. Optimized component benchmarks used cached parsed paths, black-box inputs/results, and alternating revision order; a separate harness reproduced the slowdown and isolated the pre-scan. These are evaluator timings, not whole-query timings. The full Comet JVM integration suite was not run locally, so the direct Spark evaluator/native UDF checks do not establish end-to-end Spark/Comet SQL execution.
Current-head Comet CI, CodeQL, and PR title check still await workflow approval; labeling passed. Holding approval pending the two findings below.
| if i - start > MAX_NUMBER_LEN { | ||
| return true; |
There was a problem hiding this comment.
[P2] Count numeric length the way Jackson does
The new check counts all token bytes, but Jackson excludes the leading minus sign from integer length and uses the integer/fraction/exponent digit counts for floating-point length. For {"a":1,"b":-<1000 consecutive nines>} with path $.a, Spark 4.0.4, Spark 4.1.3, the PR base, and the prior head return 1; this head returns SQL NULL because it counts 1,001 bytes. A finite decimal also regresses: [{"a":1,"b":0.<999 consecutive ones>}] with $[*].a returns 1 on those versions but SQL NULL here because the decimal point is counted. Both new-head outputs also reproduce through the compiled scalar and both column entry points.
Could you mirror Jackson's numeric length counters instead of the raw token-byte length, and add signed, fractional, and exponent boundary cases? Otherwise an unrelated, valid numeric field can null out an otherwise successful extraction.
There was a problem hiding this comment.
Fixed in 38429ffb — the scan now counts digits exactly the way jackson-core 2.21.2 does, per ParserBase.resetInt/resetFloat: the sign and decimal point do not count; integers are limited by their digit count; floats by the sum of the integer-part, fraction and exponent digit counts.
Probing the pinned jackson-core directly (Spark 4.1.3's fasterxml.jackson.version is 2.21.2) pinned down one more corner: a lone leading-zero integer part counts as zero digits, except when both a fraction and an exponent are present, where it counts as one — 0.5e followed by 999 exponent digits is rejected with "Number value length (1001) exceeds the maximum allowed (1000)", while 0. + 1000 fraction digits and 0e + 1000 exponent digits are accepted. The scan mirrors all of this and it is covered in get_json_object.sql (signed, fractional and exponent boundary columns) and in unit tests.
Validation: a 69-case battery of these boundary shapes (plus string-content and escaped-quote controls) run through Spark 4.1.3's evaluator matches on every case that is not already a pre-existing $-materialization divergence.
| if has_oversized_number(json_str) { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
[P2] Avoid scanning every quoted-string byte before extraction
This unconditional pre-scan walks the entire input byte by byte, including quoted strings, before the existing parser consumes it again. For the valid document {"a":1,"unused":"<64 KiB of x>"} and path $.a, an optimized benchmark using the exact evaluator sources measured median times of 10.066 us on the base, 10.152 us on the prior head, and 117.018 us here, about 11.6x slower than the base. The path was parsed once, inputs/results were black-boxed, and revisions alternated across three rounds of 4,000 calls. A separate harness reproduced the slowdown; removing only this scan in a diagnostic copy restored the 64 KiB case to baseline. These are evaluator-component timings, not whole-query timings.
Could you retain numeric validation while skipping quoted strings efficiently, or integrate it into parsing, and add a representative benchmark? Ordinary documents with large unselected string fields now pay this cost on every extraction even when there is no numeric-length violation.
There was a problem hiding this comment.
Fixed in 38429ffb. String bodies are no longer walked byte by byte: short bodies are scanned inline and longer ones use memchr2 — the same approach as serde_json's own ignore_str, which is the floor a separate validation pass can reach. Integrating the check into the parse itself is not possible through serde's API: Read is a sealed trait and visitors see numbers only after the token has been parsed (the length is gone by visit_f64), which is why the skipped-value path imposes no limit in the first place.
Measured with the added large_skipped_string criterion benchmark (64 KiB unselected string, $.a, the same shape as your harness): 9.08 us/doc without the scan versus 9.6 us/doc with it on this machine, down from the ~117 us the byte-wise state machine cost. On the existing realistic-document benches the overhead is about a quarter (339 -> 427 ns/doc for $.name); a document only pays for the bytes the scan has to touch, and the scan exits at the first violation.
Your three-round alternating methodology is much better than a single before/after run — glad to adopt it for the numbers above.
…h Spark Spark's GetJsonObjectEvaluator stops at the first matching field, but Comet's SegmentVisitor.visit_map kept overwriting the result, resolving a duplicated key to its last occurrence. Lock in the first match (even when the subpath misses, per Spark semantics) and skip later occurrences while still consuming all entries to validate the document. Closes apache#4947
Spark serializes a null reached through array traversal as the JSON text null (copyCurrentStructure), unlike a null directly under a named field, which is not a match. Add non-duplicate-key coverage for the $.a[0] and $.a[*] cases.
Spark's evaluatePath consumes two consecutive subscript wildcards as a
single non-structure-preserving step: the remaining path applies to the
outer array's elements in flatten style, and the collected matches are
always wrapped in one array, even a single one. Treating `[*][*]` as two
independent wildcards descended into the inner arrays instead, so
`{"a":[[{"b":1}]],"a":null}` with `$.a[*][*].b` returned 1 where Spark
and the pre-change native UDF return NULL: the first `a` misses (its
outer element is an array with no field `b`) and the second is null.
Parse `[*][*]` into a dedicated DoubleWildcard segment, propagate
Spark's flatten style to leaf values (splicing array leaves
recursively), and add Rust unit tests plus SQL-file cases.
…cards Port Spark's WriteStyle machinery (Raw/Quoted/Flatten) from JsonExpressionEvalUtils so wildcard wrappers are decided per wildcard level rather than once at the top: - an index immediately followed by `[*]` switches to Quoted style, whose wildcard keeps its array wrapper even for a single match (review regression: `$[0][*][0][*][*]` lost an array dimension) - wildcards nested below another wildcard stay wrapped (closes the `$.store.basket[0][*].b` and `$.a[*].b[*]` gaps; both now asserted in the SQL tests) - results are modeled on Spark's generator protocol (fragment writes + dirty flag), which also reproduces the unmatched-duplicate-key wrapper output such as `[] [1]` - `.*`/`['*']` wildcards never match, matching Spark (no reachable arm) - number tokens over 1000 characters anywhere in the document return null, mirroring Jackson's StreamReadConstraints (review regression: previously accepted in skipped fields) Validated with a 600-case differential against Spark 4.1.3's GetJsonObjectEvaluator; the only remaining differences are the pre-existing `$`-on-duplicate-keys materialization gap.
…chr speed The pre-parse scan for Jackson's 1000-digit numeric constraint now counts the way jackson-core 2.21.2 does: the sign and decimal point do not count, integers are limited by their digit count, and floats by the sum of integer-part (a lone leading zero counts as zero digits, except when both a fraction and an exponent are present, where it counts as one), fraction and exponent digit counts. Previously the scan counted raw token bytes, so a skipped -<1000 digits> or 0.<999 digits> value nulled out an otherwise successful extraction. String bodies are no longer walked byte by byte: short bodies are scanned inline and long bodies use memchr2, the same approach as serde_json's own ignore_str. On a 64 KiB unselected string field the evaluator goes from ~11.6x slower than base to within a few percent; a new large_skipped_string criterion benchmark guards the case. Validated with a 69-case boundary battery (signed, fractional, exponent and leading-zero shapes) against Spark 4.1.3, plus the existing 600-case differential, which shows no new divergences.
38429ff to
a1fd4c9
Compare
| loop { | ||
| match memchr::memchr2(b'"', b'\\', &bytes[i..]) { | ||
| Some(off) if bytes[i + off] == b'"' => return Some(i + off + 1), | ||
| Some(off) => i += off + 2, // escaped byte |
There was a problem hiding this comment.
A truncated JSON string can now abort the native query instead of returning NULL. For {"a":1,"unused":" followed by 40 x characters and a final backslash, i advances past the buffer and panics. I reproduced this with constant and column paths. Please bounds-check the escape skip and add a regression.
| } | ||
|
|
||
| fn has_oversized_number(json: &str) -> bool { | ||
| const MAX_NUMBER_DIGITS: usize = 1000; |
There was a problem hiding this comment.
For {"a":1,"b":<1001 nines>} with $.a, Spark 3.4.3 and the base return 1, while this head returns NULL. I verified both evaluators. Spark 3.4.3 uses Jackson 2.14.2, which has no such limit. Please preserve the version-specific behavior and add a Spark 3.4 regression while retaining the 4.1 validation.
Which issue does this PR close?
Closes #4947.
Rationale for this change
For a JSON object with duplicate keys, Spark's
GetJsonObjectEvaluatorkeeps the first occurrence that produces a value, but Comet's native implementation resolved the key to its last occurrence (matchingserde_json's overwrite semantics rather than Spark's):What changes are included in this PR?
SegmentVisitor::visit_mapinnative/spark-expr/src/string_funcs/get_json_object.rsnow locks in the first successful match for a key and consumes later occurrences viaIgnoredAnyinstead of re-parsing them withPathSeed:GetJsonObjectEvaluator.evaluatePath, where a named field whose value is JSON null — or whose subtree does not resolve the rest of the path — does not setdirty, and evaluation continues to later duplicate keys. For example,{"a":null,"a":2}/$.aand{"a":{"x":1},"a":{"b":2}}/$.a.bboth return2in Spark.The PR also includes two supporting changes in the same file:
Option<Value>is replaced by aPathResultcarrying the matched values plus a separate matched flag, andPathSeedgains areject_direct_nullflag capturing Spark's rule that a JSON null directly below a named field is not a match, while a null reached through array traversal is a match and serializes as the textnull(e.g.{"a":[null]}/$.a[0]now returns the stringnullinstead of SQL NULL, matching Spark'scopyCurrentStructure).Wildcard paths are now evaluated with the same streaming seed parser instead of materializing the whole document with
serde_json::from_str. A single wildcard match on a null also serializes asnulltext, matching Spark.Two consecutive subscript wildcards
[*][*]parse to a dedicatedDoubleWildcardsegment, matching Spark's "non-structure preserving double wildcard" case inevaluatePath: the remaining path applies to the outer array's elements themselves (in flatten style, splicing array leaves recursively), and the collected matches are always wrapped in a single array, even a single one. Previously[*][*]was treated as two independent wildcards, which descended into the inner arrays instead — so{"a":[[{"b":1}]],"a":null}/$.a[*][*].breturned1where Spark returns NULL (the firsta's outer element is an array and cannot match.b, and the secondais null).PathSegment::Wildcardis split by form:[*]becomesSubscriptWildcardand.*/['*']becomeChildWildcard. Spark's evaluator has no reachable arm for the child-wildcard form (its parser emits a bare wildcard instruction that no dispatch case consumes), so those paths now return null for every document instead of iterating an array.Spark's
WriteStylemachinery is ported end to end. AStyle {Raw, Quoted, Flatten}value propagates through the evaluation the wayevaluatePaththreads itsstyleparameter, and each wildcard arm makes its own wrapper decision: an index immediately followed by[*]switches to Quoted style (whose wildcard keeps its array wrapper even for a single match), and wildcards nested below another wildcard run in Quoted style and stay wrapped. Previously the wrapper decision was made once at the top from the flat list of matches, which could not represent per-level nesting:[[[[[[[1]]]]]]]/$[0][*][0][*][*]returned[1]where Spark returns[[1]], and$.store.basket[0][*].b/$.a[*].b[*]returned"y"/[1,2,3]where Spark returns["y"]/[[1,2],[3]](the latter two were pre-existing onmain).PathResultis now modeled on Spark's generator protocol: a list of rendered fragment writes plus the dirty flag. This reproduces behaviors a value-tree cannot express — the Quoted and double-wildcard arms write their brackets even when nothing inside matched, and Spark's generator keeps those bytes, so an unmatched duplicate-key occurrence followed by a matching one yields output like[] [1](root-level writes separated by a space, as Jackson does). A Raw-style string leaf is written unquoted, which replaces the oldhas_wildcardheuristic at the top.A pre-parse scan rejects documents containing numbers whose digit count exceeds 1000, mirroring the counters of jackson-core 2.21.2's
StreamReadConstraints(the version Spark 4.1.3 pins): the sign and decimal point do not count, integers are limited by their digit count, and floats by the sum of their integer-part (a lone leading zero counts as zero digits, except when both a fraction and an exponent are present), fraction and exponent digit counts. Such numbers return null anywhere in the document — including values the path never selects — which matches Spark; serde_json'sIgnoredAnyskip imposes no such limit, and serde's sealedReadtrait gives no way to hook the token stream, so a scan is the only enforcement point. String bodies are skipped inline or with memchr2 (the same approach as serde_json's ownignore_str), so large unselected strings cost about one extra skip-pass; alarge_skipped_stringcriterion benchmark guards the case.How are these changes tested?
test_duplicate_key_last_winsunit test totest_duplicate_key_first_wins.test_duplicate_key_first_wins_nested({"a":{"b":1},"a":{"b":2}}/$.a.b->1).test_duplicate_key_first_successful_match_winscovering Spark's continue-on-null / continue-on-missing-subpath semantics ({"a":{"x":1},"a":{"b":2}}/$.a.b->2,{"a":null,"a":{"b":2}}/$.a.b->2,{"a":null,"a":2}/$.a->2,{"a":{"b":null,"b":2}}/$.a.b->2).test_duplicate_key_first_successful_match_wins_with_wildcardandtest_duplicate_key_null_reached_through_array_locks_match.test_null_reached_through_array_serializes_as_null_textfor the non-duplicate-key case ({"a":[null]}/$.a[0]and$.a[*]->nulltext,{"a":[null,1]}/$.a[*]->[null,1]).test_duplicate_key_double_wildcard_match_decisionplus five more unit tests pinning the[*][*]flatten semantics (one-level flatten mirroring Spark's own$.store.basket[*][*]case, single match staying wrapped, empty-flatten no-match, recursive flatten).[*][*]queries — including the reviewer-reported{"a":[[{"b":1}]],"a":null}/$.a[*][*].bcase — tospark/src/test/resources/sql-tests/expressions/string/get_json_object.sql, so they are validated against Spark itself.$[0][*](wrapper kept after an index),$[0][*][0][*][*](review regression),$.store.basket[0][*].band$.a[*].b[*](pre-existing gaps, now asserted positively),$[*][*][*](flatten flowing into a remaining wildcard), the never-matching.*/['*']forms, and the unmatched-duplicate-key wrapper output[] [1].repeat(...): 1000/1001-digit integers, signed 1000/1001, fraction-only and exponent-only boundaries on both sides, and the leading-zero shapes (0.<1000 digits>,0e<1000 digits>).test_index_then_wildcard_keeps_wrapper,test_wildcard_below_wildcard_keeps_inner_wrapper,test_oversized_number_in_skipped_field,test_child_wildcard_never_matches,test_wildcard_unmatched_writes_are_kept, andtest_triple_wildcard_flatten.GetJsonObjectEvaluator(driven directly from the 4.1.3 catalyst jar) and compared against this implementation. The only differences are the pre-existing value-materialization cases below. The 4 mismatches are all$-on-duplicate-keys documents, where serde_json'sValuematerialization collapses the duplicate keys that Spark's token copy preserves — a pre-existing limitation of value materialization, unchanged by this PR (the same holds for selected numbers of 309–1000 digits, which overflow f64 during materialization but are copied verbatim by Spark).datafusion-comet-spark-exprsuite passes, plusclippyandfmtchecks.spark/src/test/resources/sql-tests/andCometJsonJvmSuitefor duplicate-key cases: the only repeated keys there are across different objects inside arrays, which are unaffected by this change.