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

Add duckdb::JsonValue #11868

Closed
wants to merge 13 commits into from
Closed

Add duckdb::JsonValue #11868

wants to merge 13 commits into from

Conversation

Maxxen
Copy link
Member

@Maxxen Maxxen commented Apr 29, 2024

This PR adds a new JsonValue class to represent/parse/print json objects.
JSON is everywhere, and its pretty useful to be able to have a small json utility built into DuckDB when working with extensions or other tooling, even if its not used in the query engine itself, or as efficient as the implementation in the dedicated JSON extension.

Right now this only replaces the ToJSONMap() function and co used when stringifying exceptions in the ErrorData, but I have some plans and ideas to use it in:

  • Parsing GeoParquet metadata in the parquet extension
  • Moving the json_serialize_sql(), json_parse_serialized_sql() and JsonSerializer functions into core. This would allow use to tighten up the serializer infrastructure a bit if we can assume that extensions won't push their own serializers.
  • Converting some of the serialization generation scripts into proper c++ tools as IMO they are starting to outgrow python in complexity.
  • As an output format for a future auto-documentation-generation subsystem for sql functions
  • Maybe unifying the json output option in the shell?
  • Other configs, e.g. in the fuzzer, etc.

The parser does not handle unicode escape characters, but should otherwise be JSON compliant. We can make it more sophisticated in the future.

@duckdb-draftbot duckdb-draftbot marked this pull request as draft April 29, 2024 15:46
@Maxxen Maxxen marked this pull request as ready for review April 29, 2024 15:46
@duckdb-draftbot duckdb-draftbot marked this pull request as draft April 29, 2024 15:48
@Maxxen Maxxen marked this pull request as ready for review April 29, 2024 15:51
@Tishj
Copy link
Contributor

Tishj commented Apr 29, 2024

Converting some of the serialization generation scripts into proper c++ tools as IMO they are starting to outgrow python in complexity.

I'm not sure I agree with this statement, they are starting to outgrow the current architecture of the script however.
A refactor is needed and I've been thinking of some ways to do it, but I need to become more familiar with all of its current quirks

@duckdb-draftbot duckdb-draftbot marked this pull request as draft April 29, 2024 20:42
@Maxxen Maxxen marked this pull request as ready for review April 29, 2024 20:42
@Maxxen Maxxen requested a review from Mytherin April 30, 2024 15:10
Copy link
Collaborator

@Mytherin Mytherin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks great - some minor comments:

src/common/json/json_value.cpp Show resolved Hide resolved
test/common/test_json.cpp Show resolved Hide resolved
@duckdb-draftbot duckdb-draftbot marked this pull request as draft May 5, 2024 13:25
@Maxxen Maxxen marked this pull request as ready for review May 5, 2024 13:25
@duckdb-draftbot duckdb-draftbot marked this pull request as draft May 5, 2024 15:44
@Maxxen Maxxen mentioned this pull request May 10, 2024
@Maxxen Maxxen closed this May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants