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 LazyJSONDocument, which wraps a JSON string and only deserializes it if needed. #2470

Merged
merged 9 commits into from
Apr 30, 2024

Conversation

nicktobey
Copy link
Contributor

This is the GMS side of dolthub/dolt#7749

This is a new JSONWrapper implementation. It isn't used by the GMS in-memory storage, but it will be used in Dolt to speed up SELECT queries that don't care about the structure of the JSON.

A big difference between this and JSONDocument is that even after it de-serializes the JSON into a go value, it continues to keep the string in memory. This is good in cases where we would want to re-serialize the JSON later without changing it. (So statements like SELECT json FROM table WHERE json->>"$.key" = "foo"; will still be faster.) But with the downside of using more memory than JSONDocument)

nicktobey and others added 4 commits April 24, 2024 18:18
…atches MySQL's JSON output.

I added docstrings to StringifyJSON and MarshallJSON to make this distinction clear.
…s compatible with MySQL's JSON output, including spaces.
@nicktobey
Copy link
Contributor Author

One detail to note is that we currently desalinize JSON prior to writing it on the wire, so that we can re-serialize it in a way that matches MySQL's output (different whitespacing.) This is expensive and I don't think is worth it.

I discussed with Max whether or not we consider it a requirement to match MySQL's wire responses exactly for JSON, and agreed that we could probably relax that requirement. With this change, casting a document to a text type will still result in the same output as MySQL, but simply displaying it in a query result won't.

Copy link
Contributor

@fulghum fulghum left a comment

Choose a reason for hiding this comment

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

Looks good. It seems reasonable to tradeoff fully matching the whitespace formatting in MySQL's JSON output, with getting faster performance.

@nicktobey nicktobey merged commit 3d60d20 into main Apr 30, 2024
7 checks passed
@nicktobey nicktobey deleted the nicktobey/json branch April 30, 2024 01:56
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.

2 participants