fix: allow selecting @RelationId() fields in find options#11967
Open
yen0304 wants to merge 2 commits intotypeorm:masterfrom
Open
fix: allow selecting @RelationId() fields in find options#11967yen0304 wants to merge 2 commits intotypeorm:masterfrom
yen0304 wants to merge 2 commits intotypeorm:masterfrom
Conversation
User descriptionDescription of change
Root cause: Fix: Add a check for How to reproduce (before fix): @Entity()
class Post {
@PrimaryGeneratedColumn()
id: number
@ManyToOne(() => Category)
@JoinColumn()
category: Category
@RelationId((post: Post) => post.category)
categoryId: number
}
// This throws EntityPropertyNotFoundError:
await postRepository.findAndCount({
select: { id: true, name: true, categoryId: true },
take: 500,
});Verified: Tests pass against PostgreSQL 17 (Docker). Pull-Request Checklist
PR TypeBug fix Description
Diagram Walkthroughflowchart LR
A["find/findAndCount with select"] --> B["buildSelect() validation"]
B --> C["Check relationIds metadata"]
C --> D["Select join columns"]
D --> E["RelationIdLoader populates value"]
|
| Relevant files | |||||||
|---|---|---|---|---|---|---|---|
| Bug fix |
| ||||||
| Tests |
|
PR Code Suggestions ✨Latest suggestions up to 895991e
Previous suggestionsSuggestions
|
|||||||||||||||||||||||
The buildSelect() method in SelectQueryBuilder only checked for columns, embeds, and relations when validating the select option. Properties decorated with @RelationId() were not recognized, causing EntityPropertyNotFoundError when using find() or findAndCount() with select that includes @RelationId() fields. Add a check for metadata.relationIds in buildSelect(). When a @RelationId() property is found, select the underlying join columns of the associated relation so RelationIdLoader can populate the value. Closes typeorm#11955
bb07f38 to
f3278d7
Compare
PR Code Suggestions ✨
|
|||||||||||
Code Review by Qodo
1. Composite join columns lost
|
Comment on lines
+4083
to
+4089
| if ( | ||
| relationId.relation.isManyToOne || | ||
| relationId.relation.isOneToOneOwner | ||
| ) { | ||
| for (const joinColumn of relationId.relation.joinColumns) { | ||
| this.selects.push(alias + "." + joinColumn.propertyPath) | ||
| } |
There was a problem hiding this comment.
1. Composite join columns lost 🐞 Bug ✓ Correctness
• For composite FK/PK relations, relation.joinColumns can contain multiple columns, but the PR selects them via joinColumn.propertyPath, which can be identical for multiple join columns. • buildEscapedEntityColumnSelects resolves selected columns via a Map keyed by alias.propertyPath, so duplicate propertyPath values collapse to one selected column. • RelationIdLoader then expects *all* join column values by joinColumn.databaseName in raw results; missing columns can yield incorrect/undefined @RelationId() hydration.
Agent Prompt
## Issue description
Selecting `@RelationId()` for ManyToOne/OneToOne-owner currently adds required join columns via `alias + '.' + joinColumn.propertyPath`. For composite-key relations, multiple join columns can share the same `propertyPath` (because virtual join columns are created with `propertyName = relation.propertyName`). This causes collisions when mapping selections to columns, so not all join columns are actually selected. `RelationIdLoader` expects every join column value to be present in raw results, so `@RelationId()` hydration can be wrong/undefined.
## Issue Context
- Join columns for ManyToOne/OneToOne-owner can be created as virtual columns with `propertyName = relation.propertyName`.
- Column resolution for partial selects uses a map keyed by `${alias}.${col.propertyPath}`.
- RelationIdLoader reads join column values from raw results using `joinColumn.databaseName`.
## Fix Focus Areas
- src/query-builder/SelectQueryBuilder.ts[4056-4090]
- src/query-builder/SelectQueryBuilder.ts[2974-2984]
- src/metadata-builder/RelationJoinColumnBuilder.ts[121-142]
- src/metadata-builder/RelationJoinColumnBuilder.ts[193-204]
- src/query-builder/relation-id/RelationIdLoader.ts[28-71]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
4078
to
4090
| } else if (relationId) { | ||
| // When selecting a @RelationId() property, we need to select | ||
| // the underlying join columns of the associated relation so | ||
| // the value is available in the raw query results for | ||
| // RelationIdLoader to populate. | ||
| if ( | ||
| relationId.relation.isManyToOne || | ||
| relationId.relation.isOneToOneOwner | ||
| ) { | ||
| for (const joinColumn of relationId.relation.joinColumns) { | ||
| this.selects.push(alias + "." + joinColumn.propertyPath) | ||
| } | ||
| } |
There was a problem hiding this comment.
2. Non-owner relationid ignored 🐞 Bug ✓ Correctness
• The new relationId handling only adds selects for ManyToOne/OneToOne-owner relations; for other @RelationId() relation types it adds nothing. • applyFindOptions only calls select(...) when this.selects.length > 0, so a select projection consisting only of such @RelationId() fields can be silently ignored. • This can unexpectedly return full entities (extra columns) and defeat the purpose of projection (potential perf/data-exposure issue).
Agent Prompt
## Issue description
For `@RelationId()` selections where the relation is not ManyToOne/OneToOne-owner, `buildSelect` currently adds no selects. Because `applyFindOptions` only calls `select()` when `this.selects.length > 0`, the projection can be ignored (especially when selecting only such relationId fields), resulting in full-entity selection.
## Issue Context
- The PR now recognizes `@RelationId()` properties as selectable.
- The current implementation only adds underlying join columns for ManyToOne/OneToOne-owner.
- Projections are only applied when `this.selects` is non-empty.
## Fix Focus Areas
- src/query-builder/SelectQueryBuilder.ts[4056-4090]
- src/query-builder/SelectQueryBuilder.ts[3253-3271]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Code Review by Qodo
1. RelationId select falls back
|
Comment on lines
+4078
to
+4086
| } else if (relationId) { | ||
| if ( | ||
| relationId.relation.isManyToOne || | ||
| relationId.relation.isOneToOneOwner | ||
| ) { | ||
| for (const joinColumn of relationId.relation.joinColumns) { | ||
| this.selects.push(alias + "." + joinColumn.propertyPath) | ||
| } | ||
| } |
There was a problem hiding this comment.
1. Relationid select falls back 🐞 Bug ✓ Correctness
• buildSelect now treats @RelationId properties as valid, but only adds actual column selections for ManyToOne/OneToOneOwner; for other relation types it adds nothing. • If the user’s select contains only such a relation-id property, applyFindOptions won’t call qb.select() (because this.selects is empty), and query generation falls back to SELECT *. • This violates FindOptions.select partial-selection semantics and can unexpectedly fetch all columns (performance + correctness surprise).
Agent Prompt
### Issue description
`buildSelect` now recognizes `@RelationId` properties, but for relation types other than `ManyToOne` / `OneToOneOwner` it does not add any actual column selections. When a user selects only such a relation-id field, `applyFindOptions` skips calling `select()`, and SQL generation falls back to `SELECT *`, breaking partial-select semantics.
### Issue Context
`RelationIdLoader` supports `OneToMany` / `OneToOneNotOwner` and `ManyToMany` via additional queries, so a `@RelationId` property can be meaningful for these relations. Even if you decide not to fully support them in `FindOptions.select`, silently turning a partial select into `SELECT *` is undesirable.
### Fix Focus Areas
- src/query-builder/SelectQueryBuilder.ts[4048-4101]
- src/query-builder/SelectQueryBuilder.ts[3248-3271]
- src/query-builder/SelectQueryBuilder.ts[2255-2328]
- src/query-builder/SelectQueryBuilder.ts[2957-2992]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
commit: |
Code Review by Qodo
1. RelationId-only select misbehaves
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of change
findAndCount()andfind()throwEntityPropertyNotFoundErrorwhenselectincludes a@RelationId()decorated field.Root cause:
buildSelect()inSelectQueryBuildervalidatesselectproperties against columns, embeds, and relations — but@RelationId()properties are none of those. They are stored separately inmetadata.relationIds.Fix: Add a check for
metadata.relationIdsinbuildSelect(). When a@RelationId()property is detected, select the underlying join columns of its associated relation so thatRelationIdLoadercan populate the value from the raw query results.How to reproduce (before fix):
Verified: Tests pass against PostgreSQL 17 (Docker).
Pull-Request Checklist
masterbranchtest/functional/find-options/relation-id-select/)