Skip to content

feat: add count equivalent for knex loader methods#565

Merged
wschurman merged 1 commit intomainfrom
wschurman/04-06-feat_add_count_equivalent_for_knex_loaer_methods
Apr 11, 2026
Merged

feat: add count equivalent for knex loader methods#565
wschurman merged 1 commit intomainfrom
wschurman/04-06-feat_add_count_equivalent_for_knex_loaer_methods

Conversation

@wschurman
Copy link
Copy Markdown
Member

@wschurman wschurman commented Apr 6, 2026

Why

Looking at more ways the Expo server application needs escape hatches from entity, particularly in postgres/knex, it's:

  • count
  • upsert
  • batch create/update/delete (and then batch count/upsert/etc)

In other PRs I'm looking into batch stuff, but it's quite complex. In the meantime, some easy wins are count and upsert.

How

Adds count equivalents for knex loads. Note that these purposefully don't do authorization since the point of these is to not need to load the full row set, and authorization is row-based.

Test Plan

Run tests.

Copy link
Copy Markdown
Member Author

wschurman commented Apr 6, 2026

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

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (2240e94) to head (7e414b6).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##              main      #565    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          110       110            
  Lines        17424     17694   +270     
  Branches      1505      1519    +14     
==========================================
+ Hits         17424     17694   +270     
Flag Coverage Δ
integration 28.52% <74.83%> (+0.74%) ⬆️
unittest 94.59% <53.97%> (-0.62%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 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 wschurman/04-06-feat_add_count_equivalent_for_knex_loaer_methods branch from ea3c0cd to 4a4593d Compare April 6, 2026 23:25
@wschurman wschurman changed the title feat: add count equivalent for knex loaer methods feat: add count equivalent for knex loader methods Apr 6, 2026
@wschurman wschurman force-pushed the wschurman/04-06-feat_add_count_equivalent_for_knex_loaer_methods branch 2 times, most recently from 521941d to 52b9604 Compare April 6, 2026 23:42
@wschurman wschurman marked this pull request as ready for review April 6, 2026 23:57
@wschurman wschurman requested review from ide and quinlanj April 6, 2026 23:57
@ide
Copy link
Copy Markdown
Member

ide commented Apr 7, 2026

For upsert, there is now MERGE in Postgres. Awhile ago Sonnet 4.6 claimed it was not as efficient as ON CONFLICT but I think it would be best to profile both in a production scenario and see if we can tell a difference in the Grafana query stats.

Comment on lines +258 to +262
const query = this.applyFieldEqualityConjunctionWhereClause(
queryInterface.count('* as count').from(tableName),
tableFieldSingleValueEqualityOperands,
tableFieldMultiValueEqualityOperands,
);
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.

This is subtle but worth adding a test for IMO: if we have a WHERE clause that tries to match on count = ?, this count refers to a column named count in the table - not the `count(*). (If no such column exists, PG fails)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nice catch! Renamed the count to __entity_count__ and marked it as reserved.

@wschurman wschurman mentioned this pull request Apr 7, 2026
@ide
Copy link
Copy Markdown
Member

ide commented Apr 8, 2026

Another concern I have is performance, specifically big scans taking a long time. Docblocks that emphasize this seem important.

@wschurman wschurman force-pushed the wschurman/04-06-feat_add_count_equivalent_for_knex_loaer_methods branch from 52b9604 to 79fb324 Compare April 11, 2026 00:24
Copy link
Copy Markdown
Member Author

Added a docblock to emphasize that indexes are just as critical for these count methods as they are for their non-count counterparts.

@wschurman wschurman force-pushed the wschurman/04-06-feat_add_count_equivalent_for_knex_loaer_methods branch from 79fb324 to eab360e Compare April 11, 2026 00:40
@wschurman wschurman force-pushed the wschurman/04-06-feat_add_count_equivalent_for_knex_loaer_methods branch from eab360e to 7e414b6 Compare April 11, 2026 00:43
@wschurman wschurman merged commit 9a4f08e into main Apr 11, 2026
6 checks passed
@wschurman wschurman deleted the wschurman/04-06-feat_add_count_equivalent_for_knex_loaer_methods branch April 11, 2026 00:46
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