rich7420 commented on code in PR #4971:
URL: https://github.com/apache/datafusion-comet/pull/4971#discussion_r4059775926
##########
native/spark-expr/src/string_funcs/get_json_object.rs:
##########
@@ -246,68 +284,236 @@ fn parse_json_path(path: &str) -> Option<ParsedPath> {
}
}
- Some(ParsedPath {
- segments,
- has_wildcard,
- })
+ Some(ParsedPath { segments })
+}
+
+/// Jackson (and therefore Spark) rejects numbers whose digit count exceeds
+/// 1000 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 (see its `ignore_integer`/`ignore_decimal`), so mirror
+/// Jackson's `StreamReadConstraints` counters with a byte scan before parsing:
+/// the sign and the 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), fraction and exponent digit counts.
+/// Find the end of a string body starting at `i` (just past the opening
+/// quote). Short bodies are scanned inline; long bodies use memchr2, the same
+/// approach as serde_json's `ignore_str`. Returns the index just past the
+/// closing quote, or None for an unterminated string (the parser rejects the
+/// document anyway).
+#[inline]
+fn skip_string_body(bytes: &[u8], mut i: usize) -> Option<usize> {
+ const SHORT_STRING: usize = 32;
+ if bytes.len() - i <= SHORT_STRING {
+ while i < bytes.len() {
+ match bytes[i] {
+ b'"' => return Some(i + 1),
+ b'\\' => i += 2,
+ _ => i += 1,
+ }
+ }
+ return None;
+ }
+ 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
Review 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.
##########
native/spark-expr/src/string_funcs/get_json_object.rs:
##########
@@ -246,68 +284,236 @@ fn parse_json_path(path: &str) -> Option<ParsedPath> {
}
}
- Some(ParsedPath {
- segments,
- has_wildcard,
- })
+ Some(ParsedPath { segments })
+}
+
+/// Jackson (and therefore Spark) rejects numbers whose digit count exceeds
+/// 1000 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 (see its `ignore_integer`/`ignore_decimal`), so mirror
+/// Jackson's `StreamReadConstraints` counters with a byte scan before parsing:
+/// the sign and the 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), fraction and exponent digit counts.
+/// Find the end of a string body starting at `i` (just past the opening
+/// quote). Short bodies are scanned inline; long bodies use memchr2, the same
+/// approach as serde_json's `ignore_str`. Returns the index just past the
+/// closing quote, or None for an unterminated string (the parser rejects the
+/// document anyway).
+#[inline]
+fn skip_string_body(bytes: &[u8], mut i: usize) -> Option<usize> {
+ const SHORT_STRING: usize = 32;
+ if bytes.len() - i <= SHORT_STRING {
+ while i < bytes.len() {
+ match bytes[i] {
+ b'"' => return Some(i + 1),
+ b'\\' => i += 2,
+ _ => i += 1,
+ }
+ }
+ return None;
+ }
+ 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
+ None => return None,
+ }
+ }
+}
+
+fn has_oversized_number(json: &str) -> bool {
+ const MAX_NUMBER_DIGITS: usize = 1000;
Review 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](https://github.com/apache/spark/blob/v3.4.3/pom.xml),
which has no such limit. Please preserve the version-specific behavior and add
a Spark 3.4 regression while retaining the 4.1 validation.
--
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]
