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

feat: support Document type on update/remove etc #332

Open
Cologler opened this issue Aug 8, 2020 · 8 comments
Open

feat: support Document type on update/remove etc #332

Cologler opened this issue Aug 8, 2020 · 8 comments

Comments

@Cologler
Copy link
Contributor

Cologler commented Aug 8, 2020

Currently:

table = ...
items = table.all()
... # work with items, than:
for item in items:
    table.update(item, doc_ids=[item.doc_id])

Expected:

table = ...
items = table.all()
... # work with items, than:
for item in items:
    table.update(item)
@msiemens
Copy link
Owner

That's an interesting idea! When implementing the update method I had SQL's UPDATE statement in mind where you either pass a query (WHERE) to update selected rows or skip the WHERE argument to update all rows. Although this is a nice idea I'm not sure whether it deviates too much from general expectations.

One could use a replace method instead to replace one or more documents but this might bloat the API too much…

@Cologler
Copy link
Contributor Author

Cologler commented Aug 13, 2020

SQLite has similar update stmt insert or replace values (...) to update single row without where query repr.

@montarion
Copy link

I too would like this

@stale
Copy link

stale bot commented Oct 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Feel free to reopen this if needed. Thank you for your contributions ❤️

@stale stale bot added the stale label Oct 4, 2020
@stale stale bot removed the stale label Oct 5, 2020
@benetherington
Copy link
Contributor

I don't think a separate doc_ids argument is too much of a stretch here, but I do really like how clean that example code is! @Cologler, would you be willing to use upsert(item) instead? I just put in PR #395 which allows this, and it would mimic the insert or replace statement you mentioned.

If upsert won't do, I'd be happy to work on a replace method, Marcus. It could just be an alias for update(document, doc_ids=[document.doc_id]). I don't think it's a good idea to roll this into update() itself as the conditional logic would really dive into the weeds.

@msiemens
Copy link
Owner

If upsert won't do, I'd be happy to work on a replace method, Marcus.

I think upsert is the best way to support this feature as it matches the desired semantics most closely 🙂

@Cologler
Copy link
Contributor Author

@benetherington @msiemens

update equals update or failed, upsert equals update or insert, which is different.

In most cases, I insert a dict into tinydb, then load as Document from tinydb, do some changes, finally update back to the database. If I use SQLite, I always use UPDATE instead of UPSERT to write back, this is strange that I should use upsert to update it on tinydb.

The doc_id is generated by the tinydb in most cases, and I never manually create a Document instance, they usually created by the tinydb when I query it.

@Cologler
Copy link
Contributor Author

This issue is about what we can do after we load an entity from a tinydb, and update and remove are two basic operations of CRUD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants