Repository navigation
fix: choose get_json_object number-limit behavior from the runtime Jackson version - #6600
Conversation
…ckson version The Spark 3.4 shim always routed native get_json_object to the variant that skips Jackson's 1000-digit number limit, assuming Spark 3.4's bundled Jackson 2.14. Spark 3.4 can run with Jackson 2.15+, which enforces that limit, so Comet returned values where Spark returns null. Pick the variant from the Jackson version on the classpath instead.
sunchao
left a comment
sunchao
left a comment
There was a problem hiding this comment.
Summary
- Prior state and problem: Spark 3.4 always selected the native variant without Jackson limits, even when running Jackson 2.15+.
- Design approach: Select between the existing native variants using the runtime Jackson version.
- Correctness / compatibility analysis: Spark sources and focused runtime checks support the version boundary. No introduced P1/P2 issues found within this review.
- Key design decisions: Keep the decision in the Spark 3.4 shim and cache the version check lazily. This adds no per-row version lookup or new abstraction.
- Implementation sketch:
getJsonObjectNativeFunctionNameselectsget_json_objectfor Jackson 2.15+ andget_json_object_spark34otherwise. The SQL-test comment describes runtime selection. - Behavioral changes worth calling out: The change affects only the opt-in native path. Default Spark dispatch and Spark 3.5+ shims remain unchanged. Comparison with
branch-1.1confirmed unchanged default dispatch and distinguished intervening native-parser changes from this PR. - Suggested improvements: None meeting the P1/P2 reporting bar.
Reviewed full SHA 708ad58313395ce1293492603f2650cc8849d611 against base 3bc2faa934f04799686705b952d59ed32a707dda. Verified non-draft status and read the discussion snapshot plus live comments and reviews, all empty. Inspected the complete comparison: the three codegen differences originate from base-only commit #6552, rather than PR changes. The full PR diff contains two files.
Routed skills: review-comet-pr and review-comet-expression-pr. Memory and FFI sibling skills were also screened while establishing the codegen differences’ ancestry.
Exact-head CI: 19 successful, 6 running, 14 skipped, with no failures. Rust tests, four Spark 4.1 suites, and TPC-DS remain running. Spark SQL suites, including Spark 3.4, were skipped.
Validation: An isolated JDK 17 probe compiled the changed selector and passed 102 Spark 3.4.3 expression checks across Jackson 2.14.2, 2.15.0, and 2.17.0, covering numeric and nesting boundaries, nulls, invalid JSON, and literal/dynamic paths. Compared relevant Spark 3.4–4.2 sources. This was not an end-to-end Comet run: the full project, native execution, and Spark SQL suites were not built or run locally.
2 participants
Which issue does this PR close?
N/A. Follow-up to #4971.
Rationale for this change
#4971 added two native
get_json_objectvariants. One enforces Jackson's default 1000-digit number limit and nesting limit, which Jackson 2.15 added viaStreamReadConstraints. The other,get_json_object_spark34, skips those limits. The Spark 3.4 shim always picks the second one because Spark 3.4 bundles Jackson 2.14.The behavior depends on the Jackson version, not the Spark version. A Spark 3.4 deployment running with Jackson 2.15+ enforces the limits, so Spark returns null for documents with an over-long number while Comet's native path returns a value. That breaks the existing large-number queries in
get_json_object.sql.What changes are included in this PR?
CometExprShimreads the runtime Jackson version (com.fasterxml.jackson.core.json.PackageVersion.VERSION). It usesget_json_objectfor Jackson 2.15+ andget_json_object_spark34otherwise. The version is read once, lazily. Spark 3.5+ shims are unchanged.get_json_object.sqlto match.How are these changes tested?
Covered by the existing large-number queries in
expressions/string/get_json_object.sql:-Pspark-3.4(Jackson 2.14): passes, same as before.CometSqlFileTestSuitefails on those queries without this change and passes with it.