Attributes provided in `composite()` take precedence when evaluating sparse index `condition` callback

[dabrowne]

Describe the bug Where a composite() call is used to provide additional context to an update operation, the attributes provided to composite() can override those provided to set() on the value that is passed to the condition callback of a sparse index.

The result is that the condition may be evaluated with a "current" value rather than the "updated" value, and the index may be unexpectedly created or dropped.

This overriding behaviour only occurs for fields that appear as members of an index. If the field is not referenced by an index, the value passed to set() takes precedence as expected.

ElectroDB Version 3.4.1

ElectroDB Playground Link Playground link

Entity/Service Definitions See playground link

Expected behavior Values provided to update calls should always take precedence over conflicting values that are provided to composite() when evaluating a condition callback.

[tywalch]

Hi @dabrowne 👋

Thanks for the ticket! At first glance it looks like you're use of composite is to add conditional expressions to your patch? Is that correct? If not, could you add some more detail about your needs/motivations for the using the composite api vs where?

[dabrowne]

Thanks for the quick response @tywalch.

My heuristic for use of composite is that it is used to provide additional context necessary for certain update operations where that context isn't provided by the update itself. For example, filling out missing composite index members for updates that include some (but not all) members of a particular composite index.

In the case of this issue, I am using composite as a way of ensuring that certain fields are available when evaluating the condition callback on a sparse index. In doing this, I expect that values supplied to set take precedence over any conflicting values that are also supplied to composite. This is true when ElectroDB resolves new composite index values (if a field appears in both set and composite, then the value from set is used to construct the new index value).

However in the playground example, you can see that when I specify a duplicate field (status) in both set and composite, the value provided to the condition callback of my byReference index includes the status value provided to composite (i.e. composite takes precedence over set, which is the opposite of expected behaviour). Worse still, this behaviour is flips depending on whether the conflicting field is part of an index.

Have a look at the comment in my playground link: check the console output and see whether that behaviour makes sense. Comment out the (seemingly unrelated) byStatus index and see how the output changes.

---

To further explain how I got here and why I think this issue is important:

One challenge with composite is that it requires the developer writing the query to have a good understanding of the index definitions on that entity and the constraints that ElectroDB enforces to ensure that those indexes can be kept up to date (this somewhat breaks down the nice abstraction that ElectroDB sets up). A developer who understands these constraints will include the necessary extra fields using a composite call to ensure that all composite indexes affected by the update can be fully resolved and kept in sync like in your example. However, manually choosing the fields to include in a composite call can be brittle, because changes to the entity's indexes might break existing updates written in this way. It also doesn't lend itself well to a "generic" update where the developer doesn't know up front exactly which fields will be included in the update, and therefore which composite values need to be included.

As in my playground link, I've adopted a pattern of supplying the whole entity data back to composite, which saves the developer having to think about which composite indexes are affected by the update and also makes the query future-proof against any new composite indexes that are added at a later date. It's also solves for generic updates, because I can be certain that all fields on the item will be available to resolve updated composite index values:

const { data: myEntity } = await MyEntity.get({ .. }).go();

// ...

await MyEntity
  .patch(myEntity)
  .set(updates)
  .composite(myEntity)
  .go()

I find that this pattern works really well with the exception of the fact that it can cause the values passed to condition to differ from the values written to the database.

[tywalch]

@dabrowne this response is AWESOME! This all makes perfect sense, let me see what I can do and I'll report back here (possibly with some follow up questions)