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

Allow delete() on index #68

Closed
inexorabletash opened this issue Feb 24, 2016 · 9 comments
Closed

Allow delete() on index #68

inexorabletash opened this issue Feb 24, 2016 · 9 comments

Comments

@inexorabletash
Copy link
Member

Here's a use case for being able to call delete() on an index:

http://stackoverflow.com/questions/35511033/how-to-apply-expiry-times-to-keys-in-html5-indexeddb/35518068#35518068

Scenario: delete all records > 24 hours old

If you create an index on a timestamp field you can open an index cursor over the range and delete every value. But if you could call delete(range) on the index directly you could avoid iterating through script.

@sicking
Copy link
Contributor

sicking commented Feb 24, 2016

Makes sense to me.

@inexorabletash
Copy link
Member Author

@beidson @aliams - opinions?

@aliams
Copy link

aliams commented Jun 9, 2016

What would happen if multiEntry is set on IDBIndex? I suspect that may be one of the reasons why IDBIndex only exposes read operations (e.g. get and count) and doesn't expose manipulative operations (such as putting, adding, or deleting). I believe the philosophy for indexes was to be a "window" into an object store without providing a direct way to manipulate the underlying object store.

@sicking
Copy link
Contributor

sicking commented Jun 9, 2016

I agree that this makes delete operations on indexes somewhat different than delete operations on objectStores. However I don't think it's awkward or surprising enough behavior that it'll result in website bugs.

So I think the fact that there is author demand for this feature, as well as a relatively low cost of implementation, makes it worth adding this feature.

Originally we did indeed put this limitation there since the index is a "window" into the objectStore. However I think that we were overly conservative, and updating and deleting objectStores "through" indexes makes authors lives easier enough that we should allow it.

@beidson
Copy link

beidson commented Jun 9, 2016

While I think delete on key cursors is a no brainer, I'm not quite sure on this one.

While difficultly of implementation may be a vote against a particular feature, ease of implementation should not necessarily be a vote for the feature.

Author demand is definitely a vote in favor of the feature.

Bizarre behavior of the feature might be a vote against it.

For example, I am kind of caught up on the multiEntry thing. I think it's potentially bizarre and unexpected that deleting a single "record" might cause the index's count to drop by more than 1.

I don't yet have a conclusion on this one.

@sicking
Copy link
Contributor

sicking commented Jun 9, 2016

I do agree that ease of implementation is not an argument for. It was more meant as a "implementation burden is not an argument against in this case".

I'm not sure if this is an argument for or against, but even without multiEntry the count can drop by more than one if you do myindex.delete("a"). Indexes aren't always unique, so multiple entries in the objectStore can have entries in myindex with the key "a".

@inexorabletash
Copy link
Member Author

For example, I am kind of caught up on the multiEntry thing. I think it's potentially bizarre and unexpected that deleting a single "record" might cause the index's count to drop by more than 1.

Note that this happens today when calling delete() on a cursor over an index. (You can also update() on a cursor for an index, which could increase the count. I'd forgotten about this and worried we had a bug when I noticed it was permitted!)

@inexorabletash
Copy link
Member Author

TPAC 2019 Web Apps Indexed DB triage notes:

Due to the complication and that it's not a frequent request, we're closing this out. We're happy to hear more use cases for this feature to cause us to revisit.

@nolanlawson
Copy link
Member

I'd like to +1 this feature. My use case is that I had a little helper function like this:

function deleteAll (store, storeOrIndex, keyRange) {
  storeOrIndex.getAllKeys(keyRange).onsuccess = e => {
    for (const result of e.target.result) {
      store.delete(result)
    }
  }
}

Then I realized that IDBObjectStore.delete() accepts a key range. Great – I can remove this code!

Except then I noticed the storeOrIndex variable, and that there's no way to delete multiple records using a key range on an index.

My precise use case is that I'm deleting data from an object store based on a foreign key. (E.g. delete a post, and delete all replies to that post.) This, along with the "delete everything with item.timestamp less than some value," both seem like common use cases to me.

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

5 participants