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

ReferenceMany: insert an empty array #1624

Closed
wants to merge 1 commit into from
Closed

Conversation

ajax13
Copy link

@ajax13 ajax13 commented Jul 28, 2017

Related to #1195

@malarzm
Copy link
Member

malarzm commented Jul 28, 2017

@ajax13 thanks for the PR! We require though that each change has a corresponding test so there will be no regressions in the future. Also I'm afraid that simply putting empty arrays after years of not doing anything will be a BC break for people looking for not existing field through query builder... thoughts on this @alcaeus @jmikola?

@jmikola
Copy link
Member

jmikola commented Jul 31, 2017

Also I'm afraid that simply putting empty arrays after years of not doing anything will be a BC break for people looking for not existing field through query builder...

I think a BC-friendly approach would be having an option on the EmbedMany or ReferenceMany option that allows an empty array to be created if the collection is empty (instead of leaving the field omitted).

If I had to guess, the current behavior of omitting empty collections is related to @Field and nullable (by default, fields with null values are unset in the document).

Otherwise, this seems like something you might simply change in 2.0 and document in the release notes.

@alcaeus
Copy link
Member

alcaeus commented Aug 1, 2017

I think a BC-friendly approach would be having an option on the EmbedMany or ReferenceMany option that allows an empty array to be created if the collection is empty

I agree on this and I also agree on using nullable, with the only difference that we wouldn't store null but rather [] (or {} if using set or atomicSet strategies).

@malarzm malarzm added this to the 2.0.0 milestone Aug 1, 2017
@malarzm
Copy link
Member

malarzm commented Aug 1, 2017

Putting into 2.0 bucket

@alcaeus alcaeus modified the milestones: 2.0.0, 2.x Apr 12, 2018
Base automatically changed from master to 2.3.x February 2, 2021 21:41
@alcaeus alcaeus removed this from the 2.x milestone Aug 5, 2021
@spiider8
Copy link

Hi, will be this funkcionality merged sometime pls?

@malarzm
Copy link
Member

malarzm commented Jul 31, 2022

We can't merge the PR as-is, it requires more work. Somebody interested needs to pick it up

@khaperets
Copy link
Contributor

khaperets commented Sep 6, 2023

Hi, will be this funkcionality merged sometime pls?

I hope someone fixes it

@malarzm
Copy link
Member

malarzm commented Oct 26, 2023

Superseded by #2547

@malarzm malarzm closed this Oct 26, 2023
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.

7 participants