u70b3 commented on code in PR #4971:
URL: https://github.com/apache/datafusion-comet/pull/4971#discussion_r4051935236
##########
native/spark-expr/src/string_funcs/get_json_object.rs:
##########
@@ -372,25 +435,65 @@ impl<'de> Visitor<'de> for SegmentVisitor<'_> {
}
fn visit_seq<A: SeqAccess<'de>>(self, mut seq: A) -> Result<Self::Value,
A::Error> {
- let PathSegment::Index(idx) = &self.segments[0] else {
- IgnoredAny.visit_seq(seq)?;
- return Ok(None);
- };
-
- for _ in 0..*idx {
- if seq.next_element::<IgnoredAny>()?.is_none() {
- return Ok(None);
+ match &self.segments[0] {
+ PathSegment::Index(idx) => {
+ for _ in 0..*idx {
+ if seq.next_element::<IgnoredAny>()?.is_none() {
+ return Ok(PathResult::default());
+ }
+ }
+ let found = seq
+ .next_element_seed(PathSeed {
+ segments: &self.segments[1..],
+ reject_direct_null: false,
+ flatten: self.flatten,
+ })?
+ .unwrap_or_default();
+ // The remaining elements are still visited, so that a
malformed element
+ // after the match yields no match, as a full parse would.
+ IgnoredAny.visit_seq(seq)?;
+ Ok(found)
+ }
+ PathSegment::Wildcard => {
+ let mut found = PathResult::default();
+ while let Some(mut result) = seq.next_element_seed(PathSeed {
+ segments: &self.segments[1..],
+ reject_direct_null: false,
+ flatten: self.flatten,
+ })? {
+ if result.matched {
+ found.matched = true;
+ found.values.append(&mut result.values);
Review Comment:
Fixed in 7099e9d2 — 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.
--
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]
