Skip to content

Fold json_get casts written as TRY_CAST, arrow_cast or arrow_try_cast - #127

Merged
adriangb merged 1 commit into
mainfrom
fix-try-cast-json-get
Sep 29, 2026
Merged

adriangb merged 1 commit into
mainfrom
fix-try-cast-json-get

Conversation

@adriangb

@adriangb adriangb commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator

JsonFunctionRewriter folds CAST(json_get(a, 'b') AS BIGINT) into json_get_int(a, 'b') so the JSON union never has to be materialized. Three other ways of spelling the same cast are missed, and each falls back to building the union and casting it away:

-- folded today
select (json_data->'foo')::bigint from test;
-- Projection: json_get_int(test.json_data, Utf8("foo"))

-- not folded
select try_cast((json_data->'foo') as bigint) from test;
-- Projection: TRY_CAST(json_get(test.json_data, Utf8("foo")) AS Int64)

select arrow_cast((json_data->'foo'), 'Int64') from test;
-- Projection: CAST(json_get(test.json_data, Utf8("foo")) AS Int64)

select arrow_try_cast((json_data->'foo'), 'Int64') from test;
-- Projection: TRY_CAST(json_get(test.json_data, Utf8("foo")) AS Int64)

All four return the correct value, so this is a missed optimization rather than a correctness bug. It matters most for a table provider that pushes the typed accessors down to a column or an index: it sees json_get and its union return type instead of json_get_int, so the pushdown and any statistics-based pruning keyed on the concrete type are lost as well as the union round-trip.

Two separate causes:

  • TRY_CAST builds an Expr::TryCast, which the rewriter's match never looked at.
  • arrow_cast and arrow_try_cast are still Expr::ScalarFunction calls when the rewriter runs. DataFusion lowers them to Expr::Cast / Expr::TryCast in SimplifyExpressions, an optimizer rule, whereas function rewrites are applied by ApplyFunctionRewrites at the start of the analyzer, which never runs again. So the Cast node does not exist yet at the only moment the rewriter can see it, and this holds for the non-try arrow_cast too.

Changes

All four spellings now share one type table.

TRY_CAST goes through the same path as CAST. The typed accessors already yield NULL for a value of another type rather than failing, and because #129 keeps the cast where the accessor returns a wider type, a value outside the target type's range still becomes NULL rather than being handed back as the accessor's wider type.

arrow_cast and arrow_try_cast are matched by function name, which is all that is visible at analyzer time, and only their first argument is replaced, so the named type is still what comes out. The type argument is parsed the way DataFusion parses it itself in ArrowCastFunc::return_field_from_args, and ArrowCastFunc::simplify then drops the cast by itself when the accessor already returns the named type.

Because #129 keeps the cast, this needs no restriction to the types an accessor returns exactly: arrow_cast(x, 'Int32') folds to CAST(json_get_int(x) AS Int32) and keeps its Int32, so it stops materializing the union for no reason.

Tests

tests/main.rs gains plan and value tests for each new spelling, including test_plan_arrow_cast_fn_narrowing_type_keeps_cast.

The property matrix in tests/rewrite_differential.rs widens from the two SQL cast spellings to all five (CAST, ::, TRY_CAST, arrow_cast, arrow_try_cast) × 8 target types, so prop_fold_is_invisible and every_cast_spelling_folds_as_documented cover every folding pair rather than only the SQL ones. narrowing_cast_preserves_type_and_narrows gains its TRY_CAST column: on {"a": 3000000000}, TRY_CAST(json_get(x,'a') AS INT) is NULL (Int32), as TRY_CAST promises.

Verification

  • cargo test green (192 tests), cargo fmt --check and cargo clippy --all-targets -- -D warnings clean.

🤖 Generated with Claude Code

@codecov-commenter

codecov-commenter commented Sep 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.46154% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.94%. Comparing base (f4f0a6e) to head (9044fe2).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/rewrite.rs 88.46% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #127      +/-   ##
==========================================
- Coverage   94.00%   93.94%   -0.06%     
==========================================
  Files          18       18              
  Lines        1902     1950      +48     
  Branches     1902     1950      +48     
==========================================
+ Hits         1788     1832      +44     
- Misses         64       66       +2     
- Partials       50       52       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cetra3 cetra3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adriangb
adriangb force-pushed the fix-try-cast-json-get branch from 5ff1fd6 to 86ee661 Compare September 10, 2026 19:07
@adriangb
adriangb changed the base branch from main to keep-cast-when-folding-json-get September 10, 2026 19:08
@adriangb
adriangb added this pull request to stack #131 September 10, 2026 19:09
Base automatically changed from keep-cast-when-folding-json-get to main September 11, 2026 15:23
@adriangb
adriangb force-pushed the fix-try-cast-json-get branch from 86ee661 to 536486c Compare September 11, 2026 15:23
@adriangb
adriangb force-pushed the fix-try-cast-json-get branch from 536486c to 02c967d Compare September 29, 2026 00:40
Only `Expr::Cast` was folded, so three other spellings of the same cast fell
back to materializing the JSON union and casting it away.

`TRY_CAST` builds an `Expr::TryCast`, which the rewriter's `match` never
looked at. `arrow_cast` and `arrow_try_cast` are still scalar function calls
at this point: DataFusion lowers them to `Cast` / `TryCast` in
`SimplifyExpressions`, an optimizer rule, whereas function rewrites are
applied by `ApplyFunctionRewrites` at the start of the analyzer, which never
runs again. So the `Cast` node does not exist yet at the only moment the
rewriter can see it, and this holds for the non-`try` `arrow_cast` too.

All four spellings now share one type table. Because the fold keeps the cast
whenever the accessor returns a wider type, `arrow_cast(x, 'Int32')` folds
to `CAST(json_get_int(x) AS Int32)` and still comes out as `Int32`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@adriangb
adriangb force-pushed the fix-try-cast-json-get branch from 02c967d to 9044fe2 Compare September 29, 2026 00:51
@adriangb
adriangb merged commit 3820540 into main Sep 29, 2026
7 checks passed
@adriangb
adriangb deleted the fix-try-cast-json-get branch September 29, 2026 04:05
@adriangb adriangb mentioned this pull request Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants