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

feature request: loadByCompositeKeyAsync #201

Closed
quinlanj opened this issue Nov 15, 2022 · 3 comments · Fixed by #272
Closed

feature request: loadByCompositeKeyAsync #201

quinlanj opened this issue Nov 15, 2022 · 3 comments · Fixed by #272
Assignees

Comments

@quinlanj
Copy link
Member

https://github.com/expo/universe/pull/10823/files#r1020707597

@wschurman
Copy link
Member

Previously discussed in #123 (comment)

@wschurman
Copy link
Member

In a subset of cases where we have unique composite keys (UNIQUE (a, b)), we could tell the Entity framework that the pair (a, b) is cacheable and have the cache invalidated when the same way single-field keys get their cache entries invalidated. I think this autoinvalidation could be built on top of the more flexible, generic secondary cacher in this PR.
(Autoinvalidation would not work for the case like "ORDER BY created_at DESC LIMIT 1" where there is no unique key.)

To accomplish this I think a generalized approach would be better than separate implementation. I could see it being done by factoring out what already exists in the loader into things like loadByFieldsCompositeKey(Tuple<FieldName, FieldValue>[]) and then make loadByFieldEqualing the n=1 case that calls into it (and doing similar conversions for other existing methods).

The tough thing will be figuring out what the set of keys to auto-invalidate are:

  • Columns: a, b, c
  • User specifies a, b as composite key
  • User updates column b to new value, we need to invalidate the cartesian product of old and new sub-column values (though I haven't verified this yet)

Thinking about this, let's say we have an entity definition where an entity is unique name per account:

TestEntity
  columns:
    id: cache - true
    other_unique_field: cache - true
    name
    account_id
  composite_keys:
    [account_id, name]

Before composite keys:

  • Create { id: 1, other_unique_field: 'blah', name: 'hello', account_id: 1 }
    Invalidations:
    • testentity:id:1 (dataloader, cache)
    • testentity:other_unique_field:blah (dataloader, cache)
    • testentity:name:hello (dataloader)
    • testentity:account_id:1 (dataloader)
  • Update { id: 1, other_unique_field: 'blah2', name: 'hello2', account_id: 1 }
    Invalidations:
    • testentity:id:1 (dataloader, cache)
    • testentity:other_unique_field:blah (dataloader, cache)
    • testentity:other_unique_field:blah2 (dataloader, cache)
    • testentity:name:hello (dataloader)
    • testentity:name:hello2 (dataloader)
    • testentity:account_id:1 (dataloader)
  • Delete
    Invalidations:
    • testentity:id:1 (dataloader, cache)
    • testentity:other_unique_field:blah2 (dataloader, cache)
    • testentity:name:hello2 (dataloader)
    • testentity:account_id:1 (dataloader)

After composite key on (account_id, name), additional invalidations needed for above scenario (need to come up with some mechanism for cache key creation that ensures no conflicts based on field values). There'd also be a composite dataloader for each composite key:

  • Create
    • composite:testentity:account_id,name:1,hello (dataloader, cache)
  • Update
    • composite:testentity:account_id,name:1,hello (dataloader, cache)
    • composite:testentity:account_id,name:1,hello2 (dataloader, cache)
  • Delete
    • composite:testentity:account_id,name:1,hello2 (dataloader, cache)

I think this might be fairly straightforward. But hard to know. It'd be good to spend some time coming up with an edge case.

@wschurman wschurman self-assigned this Mar 19, 2025
@wschurman
Copy link
Member

wschurman commented Mar 22, 2025

Okay, got this implemented on a branch: main...@wschurman/03-21-chore_refactor_common_adapter_loading_and_caching

Planning to reorder the PRs to ease review, but the general approach is to genericize the keys/values interface for the batched/cached pipeline (data manager -> cache adapter -> database adapter). To do this, we need to add a "holder object" concept that both single-field and composite-field loads/invalidations implement. And then to make the holders work in existing code, we need to make them "hashable" in the sense that we need to override equality for the relevant internal operations, which turn out to mostly be using these holders in Maps as keys.

So, concretely the plan is:

  1. Introduce some generic utils that will be used further up the stack (feat: add some utility classes and methods in preparation for composite keys #269)
  2. Convert the current single-value batched/cached pipeline to use holders (feat: convert batched/cached loader interface to holder pattern #271).
  3. Add a new composite value holder. Add new loader methods for loading by composite value (feat: add composite field loading and caching #272).

wschurman added a commit that referenced this issue Mar 28, 2025
…te keys (#269)

# Why

This adds the utility classes needed for composite field loading as described in #201.

# How

This adds a few concepts:
- `areSetsEqual` - simple utility for checking set equality
- `SerializableKeyMap` - a map where keys must be of a custom interface for checking equality.

# Test Plan

Add new tests, run them. Full coverage.
wschurman added a commit that referenced this issue Mar 28, 2025
# Why

Step 2 of #201.

This PR abstracts out (genericizes) the load key and load value interfaces for batched/cached loading in preparation for composite field loading.

# How

Accomplishing this requires a number of things:
1. A set of interfaces for the keys and load values to be used to load entity field objects.
2. A concrete implementation of that interface for the current use case (single fieldName, fieldValue(s)).
3. Data structure changes in the adapters and data manager to handle the new key type not being comparable via `===`. (see base PR for new map data structure for using these objects as keys).
4. Updates to the test matchers to know how to compare the serializable holder objects.

# Test Plan

Update a number of tests and add a few new tests. This has full coverage.
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

Successfully merging a pull request may close this issue.

2 participants