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 support for collection of primitive types inside JSON columns #28688

Closed
Tracked by #30731
maumar opened this issue Aug 12, 2022 · 18 comments
Closed
Tracked by #30731

Json: add support for collection of primitive types inside JSON columns #28688

maumar opened this issue Aug 12, 2022 · 18 comments
Assignees
Labels
area-json area-primitive-collections 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 Aug 12, 2022

No description provided.

@ajcvickers
Copy link
Member

Note from triage: also consider other issues related to primitive collections--see label area-primitive-collections.

@saikotek

This comment was marked as resolved.

@roji

This comment was marked as resolved.

@saikotek

This comment was marked as resolved.

@roji

This comment was marked as resolved.

@saikotek

This comment was marked as resolved.

@roji

This comment was marked as resolved.

@roji
Copy link
Member

roji commented Dec 11, 2022

Note: similar to this, we could allow weakly-typed mapping via Dictionary as well (see #29825).

@roji roji changed the title Json: add support for collection of primitive types Json: add support for collection of primitive types inside JSON columns Dec 12, 2022
@Soundman32
Copy link

I have a working prototype for this from a few years back for .EF5 - should I see if I can resurrect it for EF7 ?

@roji
Copy link
Member

roji commented Jan 1, 2023

@Soundman32 this tracks support specifically inside the new EF 7 JSON columns feature, so I'm guessing any work on EF 5 would need serious adjustment in order to be relevant. But you can always take a look.

@Soundman32
Copy link

@roji My prototype was purely for the Cosmos driver, so it might not be quite as simple as I first thought, but I'll have a go :-)

@Eirenarch
Copy link

Is this why Dictionary<string, string> with items in it ends up in the database as { }?

@roji
Copy link
Member

roji commented Apr 16, 2023

@Elrenarch that sounds like a bug - can you please open a separate issue with a minimal, runnable code sample?

@Eirenarch
Copy link

@Elrenarch that sounds like a bug - can you please open a separate issue with a minimal, runnable code sample?

Done - #30707

@roji roji self-assigned this Apr 23, 2023
@roji roji modified the milestones: Backlog, 8.0.0 Apr 26, 2023
@roji
Copy link
Member

roji commented Apr 26, 2023

SEE COMMENT BELOW FIRST

@maumar this was also done as part of #30738... We have some basic testing there specifically for when the primitive collection is inside an owned JSON entity (calling Count, indexing), though at least in principle it shouldn't matter whether the collection is inside or not (see conversation #30738 (comment)). If we do decide to add more coverage, better to add a JSON owned entity to PrimitiveCollectionsTestBase with some tests on that, and remove Column_collection_inside_json_owned_entity from NonSharedPrimitiveCollectionsQueryRelationalTestBase.

Am leaving this issue open for a bit - let me know what you think, the basic testing we already have may be enough.

@roji
Copy link
Member

roji commented Apr 26, 2023

Ah no, there's a problem with the way things currently are: the primitive collection is integrated as a string inside the larger owned entity JSON document, instead of as an in-line JSON array. For example, looking at the [TestOwner] table created by the Column_collection_inside_json_owned_entity test, I see:

{"Strings":"[\u0022foo\u0022,\u0022bar\u0022]"}`

i.e. the document contains a string value, which itself encodes an array inside. Instead, it should be:

{"Strings":["foo","bar"]}

So we still have a bit of work here.

@roji
Copy link
Member

roji commented Apr 26, 2023

I think what needs to happen is something like the following:

  • The (owned) JSON serialization/deserialization code needs to recognize primitive properties as such (via metadata which we're currently lacking, or possibly by checking whether the type mapping has ElementTypeMapping)
  • When serialization encounters such a property, it needs to do more or less the same thing we'll do in the newly-introduced CollectionToJsonStringConverter: iterate over the elements, and call into the future type mapping API to serialize them (#30677)

This is all better handled after we do the move to the lower-level Utf8JsonReader/Writer APIs (#30604), and after the type mapping APIs are introduced (#30677)

@roji roji added the blocked label Apr 26, 2023
@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 15, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0, 8.0.0-rc1 Aug 19, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0-rc1, 8.0.0 Nov 14, 2023
@alrz
Copy link
Member

alrz commented Sep 14, 2024

There is numerous issues on json collection/dictionary support to a point that it's not clear what is exactly supported or not and at what verison.

Is there any docs / samples that detail this? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-json area-primitive-collections 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

No branches or pull requests

7 participants