Skip to content

fix(orm): resolve delegate-inherited fields in cursor pagination#2591

Merged
ymc9 merged 2 commits intozenstackhq:devfrom
genu:genu/issue-2588-check
Apr 18, 2026
Merged

fix(orm): resolve delegate-inherited fields in cursor pagination#2591
ymc9 merged 2 commits intozenstackhq:devfrom
genu:genu/issue-2588-check

Conversation

@genu
Copy link
Copy Markdown
Contributor

@genu genu commented Apr 18, 2026

Summary

Closes #2588.

buildCursorFilter hardcoded the child model alias for both the outer reference and the sub-select projection, so findMany({ cursor, orderBy: { <delegate-parent-field>: ... } }) errored with column "Child.field" does not exist. The fix mirrors the originModel-aware field resolution already used by buildFilter and buildOrderBy — when a field has originModel, reference the parent table alias (which buildSelectModel already joins using the parent's model name).

Test plan

  • Added tests/regression/test/issue-2588.test.ts covering delegate-parent cursor, cursor+skip, mixed child/delegate orderBy, and child-only baseline.
  • Existing cursor tests in tests/e2e/orm/client-api/find.test.ts pass (18/18).
  • Existing delegate tests pass (23/23).

Summary by CodeRabbit

  • Bug Fixes

    • Fixed cursor-based pagination to correctly reference fields inherited from parent/delegated models, ensuring accurate ordering and stable paginated query results.
  • Tests

    • Added a regression test suite that verifies cursor pagination and sorting when ordering by inherited/delegated fields across related models.

…stackhq#2588)

buildCursorFilter hardcoded the child model alias for both the outer
reference and the sub-select projection, so `cursor` combined with
`orderBy` on a @@DeleGate parent field produced a "column does not
exist" error. Mirror the originModel-aware resolution already used by
buildFilter and buildOrderBy.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 83ce6e04-5e18-4510-bd8a-b1cba6aac960

📥 Commits

Reviewing files that changed from the base of the PR and between a5510c9 and e9c84b0.

📒 Files selected for processing (1)
  • tests/regression/test/issue-2588.test.ts

📝 Walkthrough

Walkthrough

Cursor pagination now resolves fields inherited via @@DeleGate by using the origin model's aliases for outer comparisons and inner subqueries; a regression test validates cursor + orderBy scenarios involving delegated parent fields.

Changes

Cohort / File(s) Summary
Cursor Delegation Fix
packages/orm/src/client/crud/dialects/base-dialect.ts
Refactored cursor filter construction to compute outerAlias and subSelectAlias from fieldDef.originModel (fallback to existing aliases). Updated outer comparison expression and inner select projection to use these aliases so inherited/delegated fields reference the correct table aliases.
Regression Test
tests/regression/test/issue-2588.test.ts
Added Vitest regression tests reproducing issue #2588 with an inline schema (Asset delegated, Notification extends Asset). Seeds rows and asserts several findMany cursor+orderBy permutations to ensure correct ordering and no column errors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through aliases, found origins true,
I swapped the outer and inner to the proper view,
No missing columns, no puzzling night,
Cursor and order now dance just right,
A tiny rabbit cheers this tidy glue 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(orm): resolve delegate-inherited fields in cursor pagination' directly matches the main change: fixing buildCursorFilter to handle delegate-inherited fields.
Linked Issues check ✅ Passed The pull request fully addresses issue #2588: it updates buildCursorFilter to resolve delegate fields using originModel (as used by buildOrderBy/buildFilter), fixing cursor+orderBy queries on inherited fields.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing cursor pagination with delegate-inherited fields: updates to buildCursorFilter logic and comprehensive regression tests directly address issue #2588.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/regression/test/issue-2588.test.ts`:
- Around line 21-26: The test currently uses very short 2ms sleeps between
db.notification.create calls which can cause flaky createdAt ordering; update
the test in issue-2588.test.ts to either set explicit createdAt timestamps on
the created notifications (pass createdAt in the data for the calls to
db.notification.create for a deterministic order) or increase the inter-create
delay to a safer value (e.g., 10–20ms) before creating b and c to ensure stable
ordering in CI.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 84f28bdf-906e-410f-b5e0-45fb6189d77d

📥 Commits

Reviewing files that changed from the base of the PR and between 09b96e4 and a5510c9.

📒 Files selected for processing (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/regression/test/issue-2588.test.ts

Comment thread tests/regression/test/issue-2588.test.ts Outdated
Sleep-based ordering is flaky on SQLite (1ms now() resolution) under
CI scheduling jitter. Use deterministic createdAt values instead.
Copy link
Copy Markdown
Member

@ymc9 ymc9 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @genu it's an important fix.

@ymc9 ymc9 merged commit 4e35bc1 into zenstackhq:dev Apr 18, 2026
9 checks passed
@genu genu deleted the genu/issue-2588-check branch April 18, 2026 23:49
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.

buildCursorFilter doesn't resolve @@delegate fields, causes "column does not exist" on cursor + orderBy(parent-field)

2 participants