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

Bug: apply default on upsert only for new items #404

Open
zirkelc opened this issue Jul 16, 2024 · 2 comments
Open

Bug: apply default on upsert only for new items #404

zirkelc opened this issue Jul 16, 2024 · 2 comments

Comments

@zirkelc
Copy link
Contributor

zirkelc commented Jul 16, 2024

Describe the bug
A clear and concise description of what the bug is.

The default value or function will always be applied on upsert(), even if the item already exists. This will potentially override existing values.

ElectroDB Version
Specify the version of ElectroDB you are using
(e.g. 1.8.4)
latest

ElectroDB Playground Link
Playground

Entity/Service Definitions
See playground

Expected behavior
A clear and concise description of what you expected to happen.

If the item is new, the default values should be applied. This is like create.
If the item already exists, the default values should not be applied. This is like update.
Am I understanding upsert correctly?

There are two cases to consider:

  1. does the item exist?
    • yes: do not overwrite field with default value
    • no: create new item with default values
  2. does the field on the item exist?
    • yes: do not overwrite field with default value
    • no: do not overwrite field with default value

I think 2) could be fixed by using field = if_not_exists(field, field_default_value). This should update the field only if it doesn't exist. However, I'm not sure if/how 1) could be fixed. If it's not fixable, maybe it could be added to the docs that upsert differs from update in that it always applies default values.

@tywalch
Copy link
Owner

tywalch commented Jul 20, 2024

Hi @zirkelc 👋

This is an improvement that has been considered (I thought there might have been an existing issue for this?), and it is an ideal state. I remember looking into implementing this and finding that it would required a non-trivial refactor of some parts of the library to know if a value was provided vs set via "default". Here is how you can ensure the value is not overridden by a default, it's not as ideal as the library handling it under the hood but it does prevent the issue bring up.

I'd like to keep this ticket open so I can prioritize it in my backlog, which is deeper than I'd like right now. Thank you for reporting this!

@zirkelc
Copy link
Contributor Author

zirkelc commented Jul 21, 2024

Thanks for getting back on this! Sure, let's keep it open and let me know if I can help or test anything!

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

No branches or pull requests

2 participants