Skip to content

[Variant] variant_get should follow JSONPath semantics for Field path element#9676

Merged
scovich merged 3 commits intoapache:mainfrom
sdf-jkl:jsonpath
Apr 10, 2026
Merged

[Variant] variant_get should follow JSONPath semantics for Field path element#9676
scovich merged 3 commits intoapache:mainfrom
sdf-jkl:jsonpath

Conversation

@sdf-jkl
Copy link
Copy Markdown
Contributor

@sdf-jkl sdf-jkl commented Apr 8, 2026

Which issue does this PR close?

Currently this is the only place in main that handles Path in variant_get. Other variant_get related PRs already follow the JSONPath sementics. (#9598 and #8354)

Rationale for this change

Check issue

What changes are included in this PR?

  • Changed variant_get field path handling when can't cast to Struct
  • Updated the related unit test to check the new logic
  • Cleaned up some nearby tests

Are these changes tested?

Yes, unit tests

Are there any user-facing changes?

Yes, behavior change for variant_get kernel

@github-actions github-actions bot added the parquet-variant parquet-variant* crates label Apr 8, 2026
@sdf-jkl
Copy link
Copy Markdown
Contributor Author

sdf-jkl commented Apr 8, 2026

@scovich @klion26 @codephage2020 please take a look when available!

Copy link
Copy Markdown
Contributor

@scovich scovich left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Copy Markdown
Contributor

@codephage2020 codephage2020 left a comment

Choose a reason for hiding this comment

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

LGTM! One small nit:

Copy link
Copy Markdown
Member

@klion26 klion26 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution.

shredding_state: &BorrowedShreddingState<'a>,
path_element: &VariantPathElement<'_>,
cast_options: &CastOptions,
_cast_options: &CastOptions,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we still want to keep it if it doesn't needed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If this PR goes first, the next variant_get PR will return it.

@scovich scovich merged commit b36beac into apache:main Apr 10, 2026
17 checks passed
@alamb
Copy link
Copy Markdown
Contributor

alamb commented Apr 10, 2026

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parquet-variant parquet-variant* crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

variant_get should follow JSONpath semantics

5 participants