Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Wildcard support in json extract using simdjson #12281

Closed

Conversation

bikramSingh91
Copy link
Contributor

Summary:
This change addresses 2 bugs in the wildcard support of json_extract:

Bug 1: Failure to iterate on values inside an object, resulting in
empty arrays.
For eg: calling JSON_EXTRACT(
JSON '{"foo": {"a" : {"c": 1 , "d": 3} }, "b": 2}', '$.foo.a.[*]')
will return an empty array whereas we expect [1, 3]

Bug 2: Inconsistent handling of indefinite results, always returning a
JSON array even when the token is not evaluated.
or eg: JSON_EXTRACT('({"a": {"b": [123, 456]}})', '$.a.c[*]') should
return NULL instead of [] as the key 'c' does not exist.

Differential Revision: D69281233

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 7, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D69281233

Copy link

netlify bot commented Feb 7, 2025

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 2db2e90
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/67a6660625a4f7000738d9c5

Copy link
Contributor

@kevinwilfong kevinwilfong left a comment

Choose a reason for hiding this comment

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

LGTM, a couple small suggestions

@kevinwilfong
Copy link
Contributor

Could you fix the title before landing, I think the w in "wildcard" needs to be upper case

…or#12281)

Summary:

This change addresses 2 bugs in the wildcard support of json_extract:

Bug 1: Failure to iterate on values inside an object, resulting in
empty arrays.
For eg: calling JSON_EXTRACT(
   JSON '{"foo": {"a" : {"c": 1 , "d": 3} }, "b": 2}', '$.foo.a.[*]')
will return an empty array whereas we expect [1, 3]

Bug 2: Inconsistent handling of indefinite results, always returning a
JSON array even when the token is not evaluated.
or eg: JSON_EXTRACT('({"a": {"b": [123, 456]}})', '$.a.c[*]') should
return NULL instead of [] as the key 'c' does not exist.

Reviewed By: kevinwilfong

Differential Revision: D69281233
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D69281233

@bikramSingh91 bikramSingh91 changed the title fix: wildcard support in json extract using simdjson fix: Wildcard support in json extract using simdjson Feb 7, 2025
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in e383017.

kevinwilfong pushed a commit to kevinwilfong/velox that referenced this pull request Feb 11, 2025
Summary:
When json_parse throws a user error and it's not wrapped in a try, if that exception is caught
outside expression evaluation and the ExprSet attempts to be reused, the json_parse function
may be in an invalid state and will throw a runtime exception.

This was exposed in ExpressionFuzzer after 
facebookincubator#12281 was landed.

This fix is to clear the state whenever we see a UserException after calling parse whether or not 
the function is wrapped in a try.

Differential Revision: D69473523
facebook-github-bot pushed a commit that referenced this pull request Feb 11, 2025
Summary:
Pull Request resolved: #12308

When json_parse throws a user error and it's not wrapped in a try, if that exception is caught
outside expression evaluation and the ExprSet attempts to be reused, the json_parse function
may be in an invalid state and will throw a runtime exception.

This was exposed in ExpressionFuzzer after
#12281 was landed.

This fix is to clear the state whenever we see a UserException after calling parse whether or not
the function is wrapped in a try.

Reviewed By: kgpai

Differential Revision: D69473523

fbshipit-source-id: d4ae34d65136ce80138f9ed2e7f6f86bf566011e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants