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

feat(trino): Support JSON_QUERY #4206

Merged
merged 2 commits into from
Oct 3, 2024
Merged

feat(trino): Support JSON_QUERY #4206

merged 2 commits into from
Oct 3, 2024

Conversation

VaggelisD
Copy link
Collaborator

Fixes #4200

This PR adds parsing support for Trino's JSON_QUERY(...) which is a more powerful version of JSON_EXTRACT(...).

For this matter, exp.JSONExtract node is reused, but to differentiate between the two a new arg is introduced; This is also required because Presto's jsonextract_sql has the variant_access logic which must be preserved for Trino.

Docs

JSON_QUERY | JSON_EXTRACT

Copy link
Collaborator

@georgesittas georgesittas left a comment

Choose a reason for hiding this comment

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

What are the advantages of treating this as JSONExtract? Other dialects also support JSON_QUERY, possibly with similar semantics. Check T-SQL for example.

@VaggelisD
Copy link
Collaborator Author

@georgesittas For Trino, JSON_EXTRACT and JSON_QUERY are semantically equivalent with the exception that JSON_QUERY allows for more options baked-in; Splitting these two to separate nodes would duplicate a lot of code for no particular reason as it stands. My guess is that the former is the Presto-compatible API while the latter is Trino specific as I couldn't find it in Presto's docs.

Afaict, for other dialects like T-SQL JSON_QUERY is just the equivalent function for extraction so I'm also not sure if there are any nuances that justify splitting these two for now. Wdyt?

@georgesittas
Copy link
Collaborator

georgesittas commented Oct 3, 2024

@georgesittas For Trino, JSON_EXTRACT and JSON_QUERY are semantically equivalent with the exception that JSON_QUERY allows for more options baked-in; Splitting these two to separate nodes would duplicate a lot of code for no particular reason as it stands. My guess is that the former is the Presto-compatible API while the latter is Trino specific as I couldn't find it in Presto's docs.

Not sure if this is true, I think there are some differences. For example, there's a path that looks like 'lax $.children[*]' in Trino's docs. I don't remember needing to specify lax or strict in JSON_EXTRACT. Also, at least in T-SQL (perhaps others too), JSON_QUERY only extracts nested values, and you need to use JSON_VALUE for literals. So I don't think this is accurate:

Afaict, for other dialects like T-SQL JSON_QUERY is just the equivalent function for extraction so I'm also not sure if there are any nuances that justify splitting these two for now. Wdyt?

@VaggelisD
Copy link
Collaborator Author

VaggelisD commented Oct 3, 2024

Not sure if this is true, I think there are some differences.

Right, I consider these extra options but at the end both JSON_EXTRACT and JSON_QUERY extract JSON values so I don't see the dire need to split them apart yet.

Also, at least in T-SQL (perhaps others too), JSON_QUERY only extracts nested values

For T-SQL you'd indeed need JSON_VALUE for scalar values but that'd be exp.JSONExtractScalar, right?

Also, in SQLGlot today T-SQL is the only other dialect which has JSON_QUERY

@georgesittas
Copy link
Collaborator

georgesittas commented Oct 3, 2024

For T-SQL you'd indeed need JSON_VALUE for scalar values but that'd be exp.JSONExtractScalar, right?

Not exactly. Take these T-SQL expressions, for example:

1> select json_query('{"x": 1}', '$.x')
2> GO -- yields NULL
3> select json_query('{"x": [1,2,3]}', '$.x')
4> GO -- yields [1,2,3]
5> select json_value('{"x": 1}', '$.x')
6> GO -- yields 1

@VaggelisD VaggelisD merged commit 25b18d2 into main Oct 3, 2024
6 checks passed
@VaggelisD VaggelisD deleted the vaggelisd/trino_jsonquery branch October 3, 2024 15:11
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.

can't parse JSON query when using with keyword
2 participants