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

Json: add ability to specify type of the json column #28452

Closed
Tracked by #22953
maumar opened this issue Jul 14, 2022 · 7 comments · Fixed by #34401
Closed
Tracked by #22953

Json: add ability to specify type of the json column #28452

maumar opened this issue Jul 14, 2022 · 7 comments · Fixed by #34401
Labels
area-json closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@maumar
Copy link
Contributor

maumar commented Jul 14, 2022

E.g. in postgres there are 2 types for json column: json and jsonb. We should allow specifying the type explicitly for each aggregate, once we land on final API

@shvmgpt116
Copy link

@ajcvickers @maumar
When can we expect this feature? Please could someone let me know.

@ajcvickers
Copy link
Member

We hope to include it in EF8, but that will depend on prioritization against other features. Make sure to vote (👍) for this issue.

@rofenix2
Copy link

rofenix2 commented Feb 1, 2023

Sorry for reopen but does this mean that i cant change nvarchar(max) for json columns?. Cause that is exactly what i am trying to do according to Microsoft documentation to use nvarchar(4000) for performance reasons.

@roji
Copy link
Member

roji commented Feb 1, 2023

@cdrfenix can you share some more context? Are you trying to store JSON documents in an nvarchar(4000) column (instead of max)? If so, why? Is it so that you can index the entire JSON document, and if so, how would you be using that index?

@shvmgpt116 some more information on why you need this would be appreciated.

@rofenix2
Copy link

rofenix2 commented Feb 1, 2023

No i wont index it, i am just trying to follow ms recommendation according to the documentation in here: https://learn.microsoft.com/en-us/sql/relational-databases/json/store-json-documents-in-sql-tables?view=sql-server-ver16

Even tho they tell you twice to use nvarchar(4000) "for performance reasons" it doesnt explain in detail why. So if the reason its only for using a index then i guess i dont have a problem and nvarchar(max) will just do it. Also this article could be relevant in relation to this issue (but not my use case):

https://bornsql.ca/blog/think-twice-about-storing-json-in-your-sql-server-database/

@roji
Copy link
Member

roji commented Feb 1, 2023

Thanks for the links. I'm not enough of a SQL Server expert to know exactly how nvarchar(4000) vs. nvarchar(max) affect performance, but this comment does seem to point to that being important:

The nvarchar(max) data type lets you store JSON documents that are up to 2 GB in size. If you're sure that your JSON documents aren't greater than 8 KB, however, we recommend that you use NVARCHAR(4000) instead of NVARCHAR(max) for performance reasons.

Note that compression (and possibly even columnstore) is a bit orthogonal to this; but also important.

@roji
Copy link
Member

roji commented Apr 30, 2024

Another need here is using varchar instead of nvarchar on SQL Server, especially for UTF8 (raised in #33643).

@ajcvickers ajcvickers assigned ajcvickers and unassigned maumar Aug 11, 2024
@ajcvickers ajcvickers modified the milestones: Backlog, 9.0.0 Aug 11, 2024
ajcvickers added a commit that referenced this issue Aug 11, 2024
Fixes #28452
Fixes #32150

Remaining work:

- Test reverse engineering from an existing database
- Output a warning when the native JSON type is used
- Replace the ToJson overload with HasColumnType()
- Move the type mapping visitation to another visitor

Known issues:

- Various issues communicated with the SQL team--see TODO:SQLJSON
- Testing is disabled until we have an appropriate server and driver to test against
ajcvickers added a commit that referenced this issue Aug 11, 2024
Fixes #28452
Fixes #32150

Remaining work:

- Test reverse engineering from an existing database
- Output a warning when the native JSON type is used
- Replace the ToJson overload with HasColumnType()
- Move the type mapping visitation to another visitor

Known issues:

- Various issues communicated with the SQL team--see TODO:SQLJSON
- Testing is disabled until we have an appropriate server and driver to test against
ajcvickers added a commit that referenced this issue Aug 12, 2024
Fixes #28452
Fixes #32150

Remaining work:

- Test reverse engineering from an existing database
- Output a warning when the native JSON type is used
- Replace the ToJson overload with HasColumnType()
- Move the type mapping visitation to another visitor

Known issues:

- Various issues communicated with the SQL team--see TODO:SQLJSON
- Testing is disabled until we have an appropriate server and driver to test against
ajcvickers added a commit that referenced this issue Aug 12, 2024
Fixes #28452
Fixes #32150

Remaining work:

- Test reverse engineering from an existing database
- Output a warning when the native JSON type is used
- Replace the ToJson overload with HasColumnType()
- Move the type mapping visitation to another visitor

Known issues:

- Various issues communicated with the SQL team--see TODO:SQLJSON
- Testing is disabled until we have an appropriate server and driver to test against
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 12, 2024
ajcvickers added a commit that referenced this issue Aug 13, 2024
Fixes #28452
Fixes #32150

Remaining work:

- Test reverse engineering from an existing database
- Output a warning when the native JSON type is used
- Replace the ToJson overload with HasColumnType()
- Move the type mapping visitation to another visitor

Known issues:

- Various issues communicated with the SQL team--see TODO:SQLJSON
- Testing is disabled until we have an appropriate server and driver to test against
ajcvickers added a commit that referenced this issue Aug 14, 2024
Fixes #28452
Fixes #32150

Remaining work:

- Test reverse engineering from an existing database
- Output a warning when the native JSON type is used
- Replace the ToJson overload with HasColumnType()
- Move the type mapping visitation to another visitor

Known issues:

- Various issues communicated with the SQL team--see TODO:SQLJSON
- Testing is disabled until we have an appropriate server and driver to test against
@ajcvickers ajcvickers modified the milestones: 9.0.0, 9.0.0-rc1 Aug 21, 2024
@ajcvickers ajcvickers removed their assignment Aug 31, 2024
@roji roji modified the milestones: 9.0.0-rc1, 9.0.0 Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-json closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants