u70b3 commented on code in PR #4971:
URL: https://github.com/apache/datafusion-comet/pull/4971#discussion_r3974967321
##########
native/spark-expr/src/string_funcs/get_json_object.rs:
##########
@@ -320,50 +339,52 @@ struct SegmentVisitor<'a> {
}
impl<'de> Visitor<'de> for SegmentVisitor<'_> {
- type Value = Option<Value>;
+ type Value = PathResult;
fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str("a JSON value")
}
fn visit_bool<E>(self, _: bool) -> Result<Self::Value, E> {
- Ok(None)
+ Ok(PathResult::default())
}
fn visit_i64<E>(self, _: i64) -> Result<Self::Value, E> {
- Ok(None)
+ Ok(PathResult::default())
}
fn visit_u64<E>(self, _: u64) -> Result<Self::Value, E> {
- Ok(None)
+ Ok(PathResult::default())
}
fn visit_f64<E>(self, _: f64) -> Result<Self::Value, E> {
- Ok(None)
+ Ok(PathResult::default())
}
fn visit_str<E>(self, _: &str) -> Result<Self::Value, E> {
- Ok(None)
+ Ok(PathResult::default())
}
fn visit_unit<E>(self) -> Result<Self::Value, E> {
- Ok(None)
+ Ok(PathResult::default())
}
fn visit_map<A: MapAccess<'de>>(self, mut map: A) -> Result<Self::Value,
A::Error> {
let PathSegment::Field(name) = &self.segments[0] else {
IgnoredAny.visit_map(map)?;
- return Ok(None);
+ return Ok(PathResult::default());
};
- let mut found = None;
- // Every entry is visited so that a duplicated key resolves to its last
- // occurrence, as it would in a parsed object.
+ let mut found = PathResult::default();
while let Some(matched) = map.next_key_seed(KeySeed(name))? {
- if matched {
- found = map.next_value_seed(PathSeed {
+ if matched && !found.matched {
Review 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_decision` covers 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 — to
`spark/src/test/resources/sql-tests/expressions/string/get_json_object.sql`, so
CI validates against Spark itself. `CometSqlFileTestSuite` passes locally on
Spark 4.1.3.
--
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]
