sunchao commented on code in PR #4971:
URL: https://github.com/apache/datafusion-comet/pull/4971#discussion_r4043529697
##########
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:
[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?
##########
native/spark-expr/src/string_funcs/get_json_object.rs:
##########
@@ -255,59 +276,97 @@ fn parse_json_path(path: &str) -> Option<ParsedPath> {
/// Evaluate a parsed JSONPath against a JSON string.
/// Returns the result as a string, or None if no match.
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)?;
Review 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?
--
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]
