Skip to content

fix: composite PK detection + filter type-safety from schema source of truth#879

Merged
pyramation merged 1 commit intomainfrom
devin/1774087459-fix-filter-typesafety-composite-pk
Mar 21, 2026
Merged

fix: composite PK detection + filter type-safety from schema source of truth#879
pyramation merged 1 commit intomainfrom
devin/1774087459-fix-filter-typesafety-composite-pk

Conversation

@pyramation
Copy link
Contributor

@pyramation pyramation commented Mar 21, 2026

Summary

Two targeted fixes in the ORM codegen introspection pipeline:

1. Composite primary key detection (infer-tables.ts)
inferPrimaryKeyFromInputObject previously returned null when multiple candidate key fields existed (e.g., junction table PostTag with postId + tagId). Now returns all candidate fields, enabling composite PK detection. Prefers DeleteInput over UpdateInput as signal source since it has fewer non-PK fields to filter out.

2. Plugin filter type-safety (input-types-generator.ts)
buildTableFilterProperties previously only generated filter fields from table.fields (entity columns), missing plugin-injected fields like bm25Body, tsvTsv, trgmName, vectorEmbedding, geom, etc. Now uses the schema's {Entity}Filter input type from the TypeRegistry as sole source of truth when available — same pattern as buildOrderByValues(). Adds collectFilterExtraInputTypes() to discover custom filter types (e.g., Bm25BodyFilter) that need generation.

Both changes are backward compatible: composite PK detection is additive, and filter generation falls back to the existing table.fields-based approach when TypeRegistry is absent.

Review & Testing Checklist for Human

  • Verify typeRefToTs resolves plugin filter types correctly — the schema path uses typeRefToTs(field.type) for ALL filter fields (including and/or/not self-referential types and plugin types like Bm25BodyFilter). Confirm the generated TypeScript matches what the old fallback path would produce for standard fields, and that plugin types resolve to valid type names.
  • Confirm composite PK assumption is safe — the fix assumes that after filtering out clientMutationId and patch fields from DeleteInput, ALL remaining fields are PK fields. This is correct for PostGraphile's standard Delete inputs, but verify no plugins inject extra non-PK fields into Delete input types.
  • Check hasValidPrimaryKey() behavior — it returns false for composite keys (pk.fields.length === 1 check at utils.ts:456). This means single-row query hooks still won't generate for junction tables. Verify this is intentional and that the M:N add/remove path doesn't depend on hasValidPrimaryKey().
  • Run orm-test suite against a real schema with plugins to verify both fixes end-to-end: plugin filter fields appear typed in generated input-types.ts, and junction table composite PKs show up in constraints.primaryKey.

Notes

  • inferPrimaryKeyFromInputObject(deleteInput) is called twice in the ternary on lines 728-730. Minor inefficiency — could cache the result, but not a correctness issue.
  • No new tests are included in this PR. The fixes were validated via pnpm build (all packages compile). Integration testing against a live schema with plugins would provide stronger confidence.

Link to Devin session: https://app.devin.ai/sessions/745d2d10b699452091e24131ba5edef2
Requested by: @pyramation


Open with Devin

…f truth

Issue 1: buildTableFilterProperties now uses the schema's filter input type
as the sole source of truth (via TypeRegistry), capturing plugin-injected
filter fields (bm25Body, tsvTsv, trgmName, vectorEmbedding, geom, etc.)
that are not present on the entity type. Same pattern as buildOrderByValues.
Adds collectFilterExtraInputTypes to discover custom filter types referenced
by plugin fields.

Issue 2: inferPrimaryKeyFromInputObject now returns all candidate key fields
instead of null when multiple exist, enabling composite PK detection for
junction tables like PostTag(postId, tagId). Prefers DeleteInput over
UpdateInput as signal source (fewer non-PK fields to filter).
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 3 additional findings in Devin Review.

Open in Devin Review

Comment on lines +727 to +730
const keyFields =
inferPrimaryKeyFromInputObject(deleteInput).length > 0
? inferPrimaryKeyFromInputObject(deleteInput)
: inferPrimaryKeyFromInputObject(updateInput);
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 Redundant double invocation of inferPrimaryKeyFromInputObject(deleteInput)

The ternary expression calls inferPrimaryKeyFromInputObject(deleteInput) twice — once to check .length > 0 and again to obtain the result. The function iterates through inputFields with .find() and .filter() each time. While the function is pure and this doesn't cause incorrect behavior, it unnecessarily performs the work twice. The result of the first call should be stored in a variable.

Suggested change
const keyFields =
inferPrimaryKeyFromInputObject(deleteInput).length > 0
? inferPrimaryKeyFromInputObject(deleteInput)
: inferPrimaryKeyFromInputObject(updateInput);
const deleteKeyFields = inferPrimaryKeyFromInputObject(deleteInput);
const keyFields = deleteKeyFields.length > 0
? deleteKeyFields
: inferPrimaryKeyFromInputObject(updateInput);
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration
Copy link
Contributor

Superseded by unified PR #880 which combines this PR with #867 and #869.

@pyramation pyramation merged commit d497aa5 into main Mar 21, 2026
44 checks passed
@pyramation pyramation deleted the devin/1774087459-fix-filter-typesafety-composite-pk branch March 21, 2026 20:27
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