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 json data type #1051

Merged
merged 40 commits into from
Feb 10, 2023
Merged

Add json data type #1051

merged 40 commits into from
Feb 10, 2023

Conversation

guzman-raphael
Copy link
Collaborator

@guzman-raphael guzman-raphael commented Sep 10, 2022

  • Add support for json type. Useful doc on querying with JSON_VALUE.
    • insert
    • fetch
    • projection
    • querying
    • indexing
    • describe
    • documentation
  • Proposing here that we should only support json type in MySQL >=8.0 since JSON_VALUE approach is quite useful in returning with specific types, however, it was introduced in 8.x.
  • Implement using MySQL's approach since it should be compatible with Percona as well. Though there is similar functionality in MariaDB, they've chosen to use json as an alias for longtext + check constraint using JSON_VALID. Due to how different the implementations are, MariaDB won't be supported in our implementation of json type.
  • Full fetch is supported and partial fetches can be performed by projecting first (since a proper attribute name should be set).
  • From MySQL docs, looks like the size upper limit is the same as longblob (i.e. 4GB) per value.
  • Known gaps:
    • support for creating indexes. It is possible but there is more needed work to make it compatible with describe().

Depends on #1052, #1055, #1073

@guzman-raphael guzman-raphael linked an issue Sep 10, 2022 that may be closed by this pull request
@guzman-raphael guzman-raphael marked this pull request as draft September 12, 2022 19:31
@guzman-raphael guzman-raphael marked this pull request as ready for review September 15, 2022 08:33
@guzman-raphael guzman-raphael marked this pull request as draft September 21, 2022 05:52
@guzman-raphael guzman-raphael marked this pull request as ready for review February 9, 2023 13:00
@dimitri-yatsenko
Copy link
Member

@guzman-raphael Would you remove the obsolete functionality as a separate PR, so that it's not tied to the introduction of JSON fields?

@guzman-raphael guzman-raphael marked this pull request as draft February 9, 2023 16:51
@guzman-raphael
Copy link
Collaborator Author

guzman-raphael commented Feb 9, 2023

@guzman-raphael Would you remove the obsolete functionality as a separate PR, so that it's not tied to the introduction of JSON fields?

@dimitri-yatsenko Makes sense. I've created a separate PR with the non-JSON updates. Once it is merged, I'll update this PR so that the diff is cleaner.

@guzman-raphael guzman-raphael marked this pull request as ready for review February 9, 2023 23:16
Comment on lines 156 to 158
return ("NOT (%s)" if negate else "%s") % (
f"({f') {operator} ('.join(restrictions)})"
)
Copy link
Member

@dimitri-yatsenko dimitri-yatsenko Feb 9, 2023

Choose a reason for hiding this comment

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

Suggested change
return ("NOT (%s)" if negate else "%s") % (
f"({f') {operator} ('.join(restrictions)})"
)
return f"{'NOT ' if negate else ''} (%s)" % f"){operator}(".join(restrictions)"

@dimitri-yatsenko dimitri-yatsenko merged commit baf445a into datajoint:master Feb 10, 2023
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.

json datatype
2 participants