Skip to content

feat: offset / page / orderByID / orderDirection on selectByQuery and siblings (v1.11.0) - #145

Merged
gmpassos merged 13 commits into
masterfrom
feat/select-offset-order-by-id
Aug 1, 2026
Merged

feat: offset / page / orderByID / orderDirection on selectByQuery and siblings (v1.11.0)#145
gmpassos merged 13 commits into
masterfrom
feat/select-offset-order-by-id

Conversation

@gmpassos

@gmpassos gmpassos commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Context

selectByQuery and its siblings already accepted limit, but there was no way to paginate: no OFFSET, and no ordering support of any kind anywhere in the query machinery (the only ORDER BY in lib/ was inside a Postgres pg_constraint introspection query). Without a stable order a LIMIT alone returns an arbitrary subset, so page-by-page reads were impossible to write correctly. The one place that did order results — APIDBModule.select — worked around it by selecting every row and sorting by ID in Dart.

What this adds

Four optional parameters across the whole select surface:

  • offset — the return offset.
  • page — 1-based page, the ergonomic form of offset: resolves to (page - 1) * limit.
  • orderByID — orders by the table's ID column, resolved automatically from the machinery that already existed (TableScheme.idFieldNameEncodingContext.tableFieldID, or EntityHandler.idFieldName). No new resolution code.
  • orderDirection — new OrderDirection enum: ascending (default) / descending.

Semantics

Rule Behavior
Effective ordering orderByID ?? (offset != null) — an offset implies the ordering, so pagination is stable by default
Opt-out orderByID: false with an offset emits a bare OFFSET, no ORDER BY
orderDirection alone Ignored while the ordering is not active
pageoffset (page - 1) * limit. page: 1offset: 0, which still activates the ordering, so even the first page is stable
Existing callers All four unset → generated SQL is character-identical to 1.10.0
// Page 3, 20 per page. Stable, and no ordering to spell out:
await accountRepo.selectByQuery(' state == ? ', parameters: ['NY'],
    limit: 20, page: 3);
// -> SELECT "ac".* FROM "account" as "ac" WHERE ( ... )
//    ORDER BY "ac"."id" ASC LIMIT 20 OFFSET 40

// The 10 newest:
await accountRepo.selectAll(limit: 10,
    orderByID: true, orderDirection: OrderDirection.descending);

page validation

page is a public convenience resolved to an offset at the repository layer by the shared resolveSelectOffset. The adapter contract, SQL, SQLDialect, the SQL generation and every DB adapter still take offset alone — so no second source-breaking widening of the abstract adapter methods.

It throws an ArgumentError rather than silently returning the wrong rows when:

Case Why
page + offset Two spellings of one concept — passing both is a bug, not a precedence question
page without a positive limit A page has no meaning without a page size, and limit: 0 already means "no limit" here. Includes page: 1, so the rule stays predictable
page < 1 Pages are numbered from 1; a 0-based caller silently getting page 1 is an off-by-one that never surfaces

selectFirstByQuery forces limit: 1, so page: n there is the Nth entity.

Dialects

All dialect-specific SELECT tail syntax now lives in SQLDialect.orderBySQL / limitOffsetSQL, with two new capabilities: offsetRequiresLimit and offsetMaxLimitValue.

MySQL can't parse an OFFSET that is not preceded by a LIMIT, so an offset-only select there emits LIMIT 18446744073709551615 OFFSET n. PostgreSQL and the generic (in-memory) dialect emit a bare OFFSET n. This is asserted per-dialect and exercised against real containers.

⚠️ Behavior change: limit starts working where it was ignored

limit was accepted and silently discarded by DBEntityRepository.select's ConditionID/ConditionIdIN/ConditionANY/KeyConditionEQ fast paths, by DBAdapter.doSelectAll/doSelectByIDs, and by every DBObject* adapter. Those are all fixed here, so e.g. selectAll(limit: 2) on an object adapter returned every row before this PR and now returns 2. There is an explicit regression test pinning this.

⚠️ Source-breaking for external subclasses

New named parameters were added to abstract members (EntitySource.select/selectIDsBy/selectAll, DBAdapter.doSelectAll/doSelectByIDs, DBRelationalAdapter.doSelect/doSelectIDsBy). Dart requires an override to accept every named parameter of the supertype, so third-party EntityRepository/DBAdapter implementations must widen their overrides. Hence the minor bump (1.10.0 → 1.11.0), not a patch.

APIDBModule.select

/db/select/<table> gains LIMIT=<n>, OFFSET=<n>, PAGE=<n> and ORDER=asc|desc query directives, parsed alongside the pre-existing EAGER=true and stripped before the remainder is parsed as the entity condition query. The ad-hoc EAGER=true string surgery is replaced by a general directive extractor. The manual Dart-side sort is gone — the ordering is resolved by the DB. Output order for existing calls is unchanged (this endpoint defaults to orderByID: true, not to the offset != null rule). An invalid PAGE returns an error response rather than an uncaught ArgumentError.

/db/select/user/json?email == "a@b.c"&LIMIT=10&PAGE=3&ORDER=desc

Commits (each independently compilable and tested)

ac67146 OrderDirection + the shared ordering/pagination primitives + SQLDialect clause builders
238663a offset/orderByID/orderDirection threaded through the SQL path and the public API
602bb6c The DBAdapter object path — closes the limit dead ends
1241b3a APIDBModule.select query directives
b9fe0b0 v1.11.0 chores
47eb8c7 The page parameter + PAGE directive
a581922 CI: upgrade GitHub Actions versions
1256462 reflection_factory ^2.8.1 + regenerate
93ed7fc Close the gaps in the pagination/ordering test suite

(Commits 493f801..edafbc9 are a pin/CI experiment that nets to zero — git diff 47eb8c7 edafbc9 is empty. Squash-merge, or say the word and I'll drop them.)

Testing

719 VM tests + 447 Chrome tests pass, with the Docker daemon running so the PostgreSQL and MySQL suites actually executed rather than self-skipping.

  • 3 new test files: bones_api_entity_select_order_test.dart (enum, comparator, shared applier, resolveSelectOffset incl. all three error paths, dialect clause builders), bones_api_entity_db_sql_select_test.dart (exact generated SQL per case + end-to-end paging by offset and by page), and bones_api_db_module_test.dartthe first test coverage APIDBModule has ever had.
  • 3 new tests in the shared runAdapterTests template, so the generated SQL and real page-by-page reads are asserted for the in-memory, PostgreSQL, MySQL, object-memory and object-directory adapters at once.
  • Photo (String IDs) is used for the object-adapter test, covering the non-numeric branch of compareEntityIDs.

dart format, dart analyze --fatal-infos --fatal-warnings, dart pub publish --dry-run and ensure_build_test are all clean.

Toolchain fixes pulled in along the way

CI went red on ensure_build_test for a reason unrelated to this feature: reflection_factory 2.8.0 was published after the last master build, and ^2.7.5 let CI's dart pub upgrade take it. 2.8.0 requires dart_style: ^3.1.9 → resolves 3.1.12, whose output differs from the dart_style 3.1.6 vendored in Dart SDK 3.12.2 — making dart format --set-exit-if-changed and ensure_build_test mutually exclusive.

Resolved upstream rather than worked around here:

  • reflection_factory 2.8.1 (published) caps dart_style below 3.1.10, with a regression test that compares DartFormatter against the SDK's dart format on the generated-proxy shape.
  • This PR now requires reflection_factory: ^2.8.1 — deliberately not ^2.7.5, which would let consumers of the released 1.11.0 resolve the broken 2.8.0. The regenerated *.g.dart change only the builder version stamp; there is no formatting change.
  • Reported to the formatter: dart-lang/dart_style#1882 — the 3.1.10 style change is not language-versioned, so no SDK constraint avoids it (verified at language versions 3.7, 3.10, 3.11 and 3.12).
  • GitHub Actions versions upgraded: checkout v4→v7, setup-dart v1→v1.7.2, codecov-action v3→v7 (token moved to an input, as v4+ requires). No workflow logic changed.

Test coverage added after review

Gaps where the original assertions would have passed even with a broken implementation:

  • Non-id ID column — every test entity uses id, so orderByID would have passed even if the column were hardcoded. A legacy_item table with idFieldName: 'item_code' now asserts the emitted SQL orders by item_code and contains no .id at all.
  • ORDER BY across a JOIN — must target the main table's alias. All 5 campaigns share one config row, so ordering by the joined table's ID would give arbitrary paging and fail.
  • Ordering survives eager resolution — paging with EntityResolutionRules(allEager: true).
  • Stability — the same page requested repeatedly returns the same rows, which is the point of offset ⇒ orderByID.
  • Empty results — a query matching nothing stays empty under offset/limit.

Not covered

DBObjectGCSAdapter has no test suite in this repo (it had none before either), so its symmetrical change is compile-checked and review-only.

Known limitation (pre-existing, documented not fixed)

A query over a to-many relationship generates a JOIN without a DISTINCT, so it can already return the same entity more than once. Layering LIMIT/OFFSET on top means such a page can contain duplicates — paginating those queries is best-effort. Noted in the dartdoc and the CHANGELOG.

🤖 Generated with Claude Code

gmpassos and others added 6 commits August 1, 2026 14:30
Foundations for the upcoming `offset`/`orderByID`/`orderDirection`
parameters of `selectByQuery` and its siblings. Purely additive: nothing
reads these yet, so the generated SQL and every existing behavior are
unchanged.

- `OrderDirection` (`bones_api_types.dart`): `ascending` (default) /
  `descending`, with `sqlKeyword`, `parse` and the two resolvers that
  state the semantics exactly once:
  - `resolve(direction)` -> defaults to `ascending`.
  - `resolveOrderByID(orderByID, offset)` -> `orderByID ?? (offset != null)`,
    so a paginated select is ordered (and therefore stable) by default,
    and `orderByID: false` opts out.

- `compareEntityIDs` and `applySelectOrderAndPagination`
  (`bones_api_entity.dart`): the shared Dart-side "order by ID, then skip,
  then take" used by the adapters that can't delegate to a DB engine.
  `zeroLimitIsUnlimited` exists because the pre-existing call sites
  disagree on a `limit` of 0 (SQL: no clause; in-memory: empty result);
  both behaviors are preserved.

- `SQLDialect` (`bones_api_sql_builder.dart`): `orderBySQL` and
  `limitOffsetSQL` clause builders, plus `offsetRequiresLimit` /
  `offsetMaxLimitValue`. MySQL sets `offsetRequiresLimit: true` since it
  can't parse an `OFFSET` without a preceding `LIMIT`; it gets
  `LIMIT 18446744073709551615 OFFSET n` instead.

- `SQL`: `offset`, `orderByID` and `orderDirection` fields (+ `copy()`).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the 3 new optional parameters across the select surface, threaded
along the exact route the existing `limit` already travels. All defaults
preserve today's behavior: with `offset`, `orderByID` and
`orderDirection` all unset the generated SQL is character-identical.

Semantics:
- Effective ordering is `orderByID ?? (offset != null)`, so a paginated
  select is stable by default; `orderByID: false` opts out.
- `orderDirection` is ignored while the ordering is not active.

SQL generation (`bones_api_entity_db_sql.dart`): `_generateSelectTailSQL`
builds the ` ORDER BY ... LIMIT ... OFFSET ...` tail shared by
`generateSelectSQL` and `generateSelectIDsSQL`, delegating the
dialect-specific syntax to `SQLDialect`. The `ORDER BY` column is the
main table's ID, reusing the resolution that was already in
`_generateSQLFrom` (`TableScheme.idFieldName` ->
`EncodingContext.tableFieldID`) -- no new machinery.

Threaded through: `EntitySource` / `EntityRepository` /
`IterableEntityRepository` (`bones_api_entity.dart`), `APIRepository`,
`DBEntityRepository`, `DBRelationalAdapter` /
`DBRelationalRepositoryAdapter` / `DBRelationalEntityRepository`,
`DBSQLAdapter.doSelect` / `doSelectIDsBy` and
`DBSQLRepositoryAdapter.generateSelectSQL`.

Executed by `DBSQLMemoryAdapter._selectEntries` and by
`IterableEntityRepository.matches`/`all`, both via the shared
`applySelectOrderAndPagination` (order -> offset -> limit, matching SQL
semantics). `IterableEntityRepository` was updated here rather than in a
follow-up because Dart requires an override to accept every named
parameter of the supertype.

`DBEntityRepository.select`'s ConditionID/ConditionIdIN/ConditionANY
fast paths still drop them, exactly as they already drop `limit`; that
is the next commit.

Tests:
- `bones_api_entity_db_sql_select_test.dart`: exact generated SQL per
  case (incl. `copy()` round-trip and the PostgreSQL/MySQL dialect
  shapes) plus end-to-end paging over `DBSQLEntityRepository[memory]`.
- `bones_api_entity_test.dart`: `SetEntityRepository` ordering/pagination.
- `bones_api_entity_db_tests_base.dart`: two tests in the shared adapter
  template, so memory, PostgreSQL and MySQL all assert the generated SQL
  and real page-by-page reads. Verified against real PostgreSQL and
  MySQL containers, which covers the MySQL
  `LIMIT 18446744073709551615 OFFSET n` path (MySQL can't parse a
  standalone `OFFSET`).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes the dead ends where the select options were accepted and silently
discarded, so `offset`/`orderByID`/`orderDirection` now work on every
adapter -- not just the SQL ones.

BEHAVIOR CHANGE: `limit` starts applying where it was previously
ignored. `selectAll(limit: 2)` on an object adapter returned *every* row
before this commit; it now returns 2.

- `DBAdapter.doSelectByIDs`/`doSelectAll` and the `DBRepositoryAdapter`
  pass-throughs gained the 4 options.
- `DBEntityRepository.select` stops dropping them: `_selectByIDs` and
  `_selectAll` forward them, and the single-row `ConditionID` /
  `KeyConditionEQ`-on-id paths honor a positive `offset` (which skips the
  only row there is) via `_singleResult`.
- `DBEntityRepository.selectIDsBy`'s `ConditionIdIN` branch applies them
  in Dart, since `existIDs` has no pagination hook.
- `DBSQLAdapter.doSelectByIDs`/`doSelectAll` forward into
  `generateSelectSQL` -- they previously passed no `limit` at all.
- `DBObjectMemoryAdapter`, `DBObjectDirectoryAdapter` and
  `DBObjectGCSAdapter` apply them in their `_doSelect*Impl`, each through
  a local `_applyOrderAndPagination` built on the shared
  `applySelectOrderAndPagination` and the `_getTableIDFieldName` helper
  each adapter already had.

Tests: a `Pagination [objectAdapter]` test in the shared adapter
template, driving `photoAPIRepository`. `Photo` has a `String` ID, so it
also covers the non-numeric branch of `compareEntityIDs`. It includes an
explicit regression assertion for the `limit` change above, and covers
the `ConditionIdIN` and single-row `ConditionID` fast paths. Runs against
`DBObjectMemoryAdapter` (memory/PostgreSQL/MySQL suites) and
`DBObjectDirectoryAdapter` (directory suite) -- all verified, with
PostgreSQL and MySQL on real containers.

`DBObjectGCSAdapter` has no test suite in this repo (it had none before
either), so its change is compile-checked and review-only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `/db/select/<table>` endpoint could only return the whole table: it
selected *every* row and then sorted it by ID in Dart. It now pushes the
ordering and the pagination down to the DB.

- The ad-hoc `EAGER=true` string surgery is replaced by
  `_extractQueryDirective`, which pulls a `KEY=VALUE` token out of the
  `&`-joined query `String` and returns the rest as the entity condition
  query. `EAGER=true` keeps behaving exactly as before, at any position.
- New directives (see `APIDBModule.selectQueryDirectives`): `LIMIT=<n>`,
  `OFFSET=<n>` and `ORDER=asc|desc`. An unparsable value is ignored.
  `select` also takes them as arguments, which take precedence over the
  query `String`.
- The manual Dart-side sort is gone, replaced by `orderByID: true`.
  That default is deliberate -- this endpoint has always returned
  ID-sorted entities, so it defaults to `true` rather than to the
  `offset != null` rule used everywhere else, keeping the output of
  existing calls unchanged.

Tests: `bones_api_db_module_test.dart` is new -- `APIDBModule` had no
test coverage at all. It asserts the default ID ordering (a regression
guard for the removed sort), each directive, paging through the full
set, `EAGER=true` combined with the new directives, an entity condition
query preserved alongside them, and unparsable values.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Documents the new `offset`/`orderByID`/`orderDirection` parameters, the
`OrderDirection` enum, the `SQLDialect` clause builders and capabilities,
and the `APIDBModule.select` query directives.

Called out explicitly in the CHANGELOG:
- the behavior change where `limit` starts applying on the paths that
  previously ignored it (object adapters, `doSelectAll`/`doSelectByIDs`,
  the `DBEntityRepository.select` fast paths);
- that adding named parameters to abstract members is source-breaking
  for third-party `EntityRepository`/`DBAdapter` subclasses, since Dart
  requires an override to accept every named parameter of the supertype
  -- hence a minor bump rather than a patch;
- the pre-existing missing `DISTINCT` on to-many `JOIN`s, which makes
  paginating such a query best-effort.

The version is bumped in both `pubspec.yaml` and the `VERSION` constant
of `bones_api_base.dart`, which `bones_api_version_test.dart` keeps in
sync. `bump.sh`/`dart_bump` is not used here: it drives the publish flow
and needs an API key.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An ergonomic alternative to `offset` that computes it from the page size:
`offset = (page - 1) * limit`. `page: 3, limit: 20` is `offset: 40`.

`page` resolves to an `offset` at the repository layer via the new shared
`resolveSelectOffset`, so it is a public convenience only: the adapter
contract, `SQL`, `SQLDialect`, the SQL generation and every DB adapter
still take `offset` alone. No second source-breaking widening of the
abstract adapter methods.

Validation (all `ArgumentError`, so mistakes fail at the call site rather
than silently returning the wrong rows):
- `page` with an `offset` -> they are two spellings of one thing, so
  passing both is a bug, not a precedence question.
- `page` without a positive `limit` -> a page has no meaning without a
  page size, and a `limit` of 0 already means "no limit" here. Includes
  `page: 1`, so the rule stays predictable.
- `page < 1` -> pages are numbered from 1; a 0-based caller silently
  getting page 1 would be an off-by-one that never surfaces.

`page: 1` resolves to `offset: 0`, which still activates `orderByID`
under the existing `orderByID ?? (offset != null)` rule, so even the
first page is stable. `selectFirstByQuery` forces `limit: 1`, so `page: n`
there is the Nth entity.

`APIDBModule.select` gains a `PAGE=<n>` directive. It resolves the page
itself so an invalid `PAGE` becomes an error response instead of an
uncaught `ArgumentError`.

Tests: `resolveSelectOffset` unit tests (all three error paths, the
1-based arithmetic, and that page 1 activates the ordering); end-to-end
`page` paging plus the error paths over the in-memory SQL adapter and via
the shared adapter template, so PostgreSQL and MySQL cover it too
(verified on real containers); `PAGE` directive and its error responses
in the module suite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gmpassos gmpassos changed the title feat: offset / orderByID / orderDirection on selectByQuery and siblings (v1.11.0) feat: offset / page / orderByID / orderDirection on selectByQuery and siblings (v1.11.0) Aug 1, 2026
gmpassos and others added 4 commits August 1, 2026 16:18
Not related to this branch's feature: `reflection_factory` 2.8.0 was
published after the last `master` build, and `^2.7.5` let CI's
`dart pub upgrade` pick it up.

2.8.0 bundles `dart_style` 3.1.12, which "hugs" block-like arguments in
the generated `*.reflection.g.dart`:

    var ret = onCall(this, 'mapKeys', <String, dynamic>{
      'map': map,
    }, const __TR<...>(...));

while the `dart format` of the Dart SDK 3.12.2 (what CI runs) splits
them:

    var ret = onCall(
      this,
      'mapKeys',
      <String, dynamic>{'map': map},
      const __TR<...>(...),
    );

That makes the two CI jobs mutually exclusive: committing the generator
output fails `dart format --set-exit-if-changed` in `build`, and
committing the formatted output fails `ensure_build_test` in `test_vm`
(build_runner rewrites the file back). Confirmed both directions
locally.

Held at `>=2.7.5 <2.8.0` so CI resolves 2.7.5 again and the committed
generated code is byte-identical to what build_runner produces. The
range (rather than an exact pin) still allows a 2.7.x patch.

Worth revisiting once `reflection_factory` emits code that the SDK's
`dart format` leaves untouched, or once the SDK's bundled `dart_style`
catches up to 3.1.12.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rmat check

Keeps the package on the latest `reflection_factory`.

2.8.0 bundles `dart_style` 3.1.12, which "hugs" block-like arguments in
the generated `*.reflection.g.dart`, while the `dart format` of the Dart
SDK 3.12.2 splits them. That made two CI jobs mutually exclusive:
committing the generator output failed `dart format --set-exit-if-changed`
in `build`, and committing the formatted output failed
`ensure_build_test` in `test_vm` (build_runner rewrites the file back).

Resolved by not format-gating generated code: the `build` job now formats
only the hand-written sources (`git ls-files '*.dart'` minus `*.g.dart`).
Generated files are still analyzed with `--fatal-infos --fatal-warnings`,
and `ensure_build_test` still guarantees they match the generator output
byte for byte.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.59259% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.49%. Comparing base (3bd35fb) to head (93ed7fc).

Files with missing lines Patch % Lines
lib/src/bones_api_entity_db.dart 57.89% 8 Missing ⚠️
lib/src/bones_api_entity.dart 91.11% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #145      +/-   ##
==========================================
+ Coverage   66.47%   67.49%   +1.02%     
==========================================
  Files          63       63              
  Lines       21222    21347     +125     
==========================================
+ Hits        14107    14409     +302     
+ Misses       7115     6938     -177     
Flag Coverage Δ
unittests 67.49% <92.59%> (+1.02%) ⬆️

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.

- actions/checkout: v4 -> v7
- dart-lang/setup-dart: v1 -> v1.7.2
- codecov/codecov-action: v3 -> v7

`codecov-action` v4+ takes the upload token as a `token` input instead of
the `CODECOV_TOKEN` env var, so that is moved accordingly. The remaining
inputs (`directory`, `flags`, `env_vars`, `fail_ci_if_error`, `verbose`)
are unchanged and still valid in v7.

No workflow logic or commands changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gmpassos and others added 2 commits August 1, 2026 17:01
`^2.7.5` let CI's `dart pub upgrade` resolve 2.8.0, whose bundled
`dart_style` 3.1.12 formats the generated code differently from the
`dart format` of the Dart SDK (3.1.6), breaking `ensure_build_test`.

reflection_factory 2.8.1 caps `dart_style` below 3.1.10
(gmpassos/reflection_factory#51), so the generated code is byte-identical
to what the SDK's `dart format` wants again. Requiring `^2.8.1` -- and not
`^2.7.5` -- also keeps consumers of the released bones_api off the broken
2.8.0.

The regenerated files only change the builder version stamp
(2.7.5 -> 2.8.1); there is no formatting change. Verified:
`dart format --set-exit-if-changed` 0 changed, `dart analyze
--fatal-infos --fatal-warnings` clean, `ensure_build_test` green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes coverage holes where the existing assertions would have passed
even if the implementation were wrong.

- **Non-`id` ID column.** Every entity in `bones_api_test_entities.dart`
  uses `id`, so every `orderByID` assertion so far would pass even if the
  ORDER BY column were hardcoded instead of resolved from the
  `TableScheme`. Adds a `legacy_item` table with
  `idFieldName: 'item_code'` and asserts the emitted SQL orders by
  `item_code` (and that it contains no `.id` at all), for both
  `generateSelectSQL` and `generateSelectIDsSQL`.

- **ORDER BY across a JOIN.** A condition over a referenced entity
  (`config.open == ?`) generates a JOIN; the ORDER BY must target the
  MAIN table's alias, not the joined one. Here all 5 campaigns share one
  config row, so ordering by the joined table's ID would produce
  arbitrary paging -- the test would fail.

- **Ordering survives eager resolution.** Paging with
  `EntityResolutionRules(allEager: true)`, which re-reads the referenced
  entities after the select.

- **Stability.** The same page requested repeatedly returns the same
  rows -- the actual point of `offset` implying `orderByID`.

- **Empty results.** A query matching nothing stays empty under
  `offset`/`limit`, including `offset: 0`.

The JOIN/eager/stability test lives in the shared adapter template, so it
runs against the in-memory, PostgreSQL, MySQL and object-directory
adapters. Verified on real PostgreSQL and MySQL containers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gmpassos
gmpassos merged commit 7b4ac46 into master Aug 1, 2026
5 checks passed
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.

1 participant