u70b3 commented on code in PR #4971:
URL: https://github.com/apache/datafusion-comet/pull/4971#discussion_r4055797041
##########
native/spark-expr/src/string_funcs/get_json_object.rs:
##########
@@ -246,68 +284,177 @@ fn parse_json_path(path: &str) -> Option<ParsedPath> {
}
}
- Some(ParsedPath {
- segments,
- has_wildcard,
- })
+ Some(ParsedPath { segments })
+}
+
+/// Jackson (and therefore Spark) rejects number tokens longer than 1000
+/// characters wherever they appear in the document — including values this
+/// evaluation skips — so `get_json_object` returns null. serde_json enforces
no
+/// such limit when skipping, so mirror it with a byte scan before parsing.
+fn has_oversized_number(json: &str) -> bool {
+ const MAX_NUMBER_LEN: usize = 1000;
+ let bytes = json.as_bytes();
+ let mut in_string = false;
+ let mut escaped = false;
+ let mut i = 0;
+ while i < bytes.len() {
+ let b = bytes[i];
+ if in_string {
+ if escaped {
+ escaped = false;
+ } else {
+ match b {
+ b'\\' => escaped = true,
+ b'"' => in_string = false,
+ _ => {}
+ }
+ }
+ i += 1;
+ continue;
+ }
+ if b == b'"' {
+ in_string = true;
+ } else if b.is_ascii_digit() || b == b'-' {
+ // A number token: digits, sign, decimal point and exponent marker
+ // all count towards Jackson's limit.
+ let start = i;
+ i += 1;
+ while i < bytes.len()
+ && matches!(bytes[i], b'0'..=b'9' | b'.' | b'e' | b'E' | b'+'
| b'-')
+ {
+ i += 1;
+ }
+ if i - start > MAX_NUMBER_LEN {
+ return true;
+ }
+ continue;
+ }
+ i += 1;
+ }
+ false
}
/// 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)?);
+ if has_oversized_number(json_str) {
+ return None;
}
Review 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.
--
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]
