Skip to content

feat: add ID-based and field-based knex database loaders for use with locking modifiers - #706

Merged
wschurman merged 1 commit into
mainfrom
09-25-feat_add_id_and_field_knex_database_loaders
Sep 28, 2026
Merged

wschurman merged 1 commit into
mainfrom
09-25-feat_add_id_and_field_knex_database_loaders

Conversation

@wschurman

@wschurman wschurman commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Why

For #704 and #705, these only exposed the modifiers on loadManyByEqualityConjunction/loadManyBySQL, but a decent proportion of the callsites in the Expo server would prefer loadById since the existence semantics are stronger (throws an EntityNotFoundError when not found).

The alternative is to require these callsites use loadManyByEqualityConjunction and then manually throw a EntityNotFoundError when null in the application, but this violates the abstraction responsibility as only the library should be throwing its errors if we can help it.

How

Expose similar-error-semantic loader methods on the knex loader, but with distinct names, that require a modifier (otherwise the normal loader should be used).

Agent use

Fully-agent-generated. Prompt was:

  A lot of callers that want to use skipLocked/forShare/forUpdate were loading by ID and expecting the row
  to exist (i.e. throw a EntityNotFoundError like loadByIDAsync does on the normal loader). Should we add
  analogous loadByIdAsync/loadByFieldEqualingAsync/etc on the knex loader? Maybe with names that are
  distinct to indicate better that they don't dataloader/cache and go directly to DB? what could we call
  them?
  is there a way to make it so that the required EntityLoaderRowLockingModifiers param requires at least
  one of forUpdate/forShared and optionally skipLocked ? also what's the behavior of these methods that
  return non-nullable when using skipLocked and the object was locked?

Test Plan

Run updated tests.

Copy link
Copy Markdown
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (68e222f) to head (39d0252).

Additional details and impacted files
@@            Coverage Diff             @@
##              main      #706    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          109       109            
  Lines        18169     18485   +316     
  Branches      1564      1596    +32     
==========================================
+ Hits         18169     18485   +316     
Flag Coverage Δ
integration 30.06% <99.68%> (+1.21%) ⬆️
unittest 94.80% <100.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@wschurman
wschurman force-pushed the 09-25-feat_add_id_and_field_knex_database_loaders branch from 8f6f53e to 9d9d299 Compare September 25, 2026 22:34
@wschurman
wschurman force-pushed the 09-25-feat_add_id_and_field_knex_database_loaders branch from 9d9d299 to 39d0252 Compare September 28, 2026 18:42
@wschurman
wschurman marked this pull request as ready for review September 28, 2026 19:19
@wschurman wschurman changed the title feat: add ID and field knex database loaders feat: add ID-based and field-based knex database loaders for use with locking modifiers Sep 28, 2026
Comment on lines +72 to +88
/**
* Row locking modifier that locks the selected rows for update using `SELECT ... FOR UPDATE`.
* The lock is held until the end of the transaction, so the query context must be transactional.
*/
export interface EntityLoaderForUpdateRowLockingModifier {
forUpdate: true;
forShare?: never;
}

/**
* Row locking modifier that locks the selected rows in share mode using `SELECT ... FOR SHARE`.
* The lock is held until the end of the transaction, so the query context must be transactional.
*/
export interface EntityLoaderForShareRowLockingModifier {
forShare: true;
forUpdate?: never;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitting out like this is nice.

@wschurman
wschurman merged commit e87e2a3 into main Sep 28, 2026
9 checks passed
@wschurman
wschurman deleted the 09-25-feat_add_id_and_field_knex_database_loaders branch September 28, 2026 23:29
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 this pull request may close these issues.

3 participants