-
Notifications
You must be signed in to change notification settings - Fork 72
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
Sparse index is not dropped by updates that don't include index members #479
Comments
Bringing in some relevant discussion from #366 that I glossed over on first read. It looks like this limitation has already been discussed and understood, although I think that the caveat 1 mentioned here isn't well captured in the current documentation. Aside from fixing this at the docs, I think there is also a backwards-compatible way to fully resolve this by introduction of a new
Here's how I picture this working:
This is really only a small extension of the current behaviour and is done so in a backwards-compatible way: The check that was introduced to fix caveat 2 raised here would still apply in the default case of Here's an updated playground link showing how I picture a developer would use this. |
Firstly, thanks a bunch for this great library. I've worked with DynamoDB for a few years now, but it's only using ElectroDB that I feel like its full capabilities can be used without getting bogged down in boilerplate.
Describe the bug
The
condition
predicate for a sparse index is not evaluated for update calls (update()
,patch()
, orupsert()
) that don't include all members of the index.This means that if the
condition
is independent of the composite index members, then it is quite easy to write an update call that you would expect to result in the index being dropped, but does not. This is best described by the example in this playground link. In summary:byName
index is sparse on the conditionstatus !== "deleted"
.status: "deleted"
, this condition is not evaluated, and the index is not dropped.byName
index "into scope" by providing its composite fields to the.composite()
call (commented out in the playground) causes the condition to be evaluated and the index to be dropped as expected.Expected behavior
The index should be dropped when an update call sets
status: "deleted"
.I think that there is probably no practical way to change this behaviour, as it would require evaluating all
condition
predicates on every update. I expect that many/mostcondition
implementations assume that the index members values will be available, so evaluation of these conditions in a different context would break a lot of existing code.Short of fixing/changing the behaviour, is there a way to improve the documentation to help developers form a better mental model about how this works? Perhaps something along the lines of:
The
condition
callback is only evaluated for updates that either involve all index members, or where the index members are provided usingcomposite().
I also found this misleading in the documentation for
condition
, which implies that the condition is evaluated for every mutation:Additional context
Related: #366
The text was updated successfully, but these errors were encountered: