perf: avoid unnecessary post-mutation reads

[ymc9]

No description provided.

[claude]

Claude finished @ymc9's task —— View job

---

PR Review: Performance Optimization ✅

I've completed my analysis of this performance optimization PR that aims to avoid unnecessary post-mutation reads.

Todo Checklist:

• Read and analyze core operation files (base.ts, create.ts, delete.ts, update.ts)
• Review test file changes for coverage validation
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security implications
• Provide comprehensive feedback

Overall Assessment

This is a well-implemented performance optimization that introduces intelligent decision-making about when to perform post-mutation reads. The changes are focused, coherent, and maintain backward compatibility while providing significant performance benefits.

Key Changes Analysis

1. New mutationNeedsReadBack Method (base.ts:2144-2194)

Excellent implementation that analyzes whether a read-back is necessary based on:

• Policy enforcement requirements
• Presence of include clauses (relation fetching)
• Polymorphic models (delegate/base model hierarchy)
• Selection of relation or computed fields

The logic correctly identifies when SQL RETURNING clauses are sufficient vs. when full reads are needed.

2. Operation Handler Updates

• Create operations: Now skip read-back when only scalar fields are selected
• Update operations: Enhanced logic with additional check for relation-only updates (needReadBack method in update.ts:181-206)
• Delete operations: Pre-delete reads only when necessary for relation/computed field selection

3. Enhanced Update Logic

The update handler adds a crucial optimization: if an update only touches relation fields (no scalar fields), it forces a read-back since SQL RETURNING won't capture the changes effectively.

Code Quality Assessment

✅ Strengths:

• Clean separation of concerns with dedicated analysis methods
• Consistent error handling and policy enforcement
• Maintains transaction safety
• Well-structured conditional logic
• Proper type safety maintained

✅ Performance Benefits:

• Eliminates unnecessary database round-trips for simple mutations
• Reduces memory usage when full entity data isn't needed
• Maintains efficiency for complex operations that truly need read-backs

✅ Security & Safety:

• Policy enforcement logic is preserved
• Transaction boundaries remain intact
• Error handling maintains same behavior patterns
• No data leakage concerns introduced

Test Coverage Analysis

The test file changes appear minimal and appropriate:

• Tests still verify correct behavior for various mutation scenarios
• Coverage maintains validation of both simple and complex operations
• No regression in existing functionality observed

Potential Considerations

Minor observations:

1. The selectedFields parameter in various methods could benefit from more explicit documentation about its purpose
2. Consider adding metrics/logging to track the performance impact of this optimization in production
3. The needReadBack method name in update.ts could be more specific (e.g., updateNeedsReadBack) to distinguish from the base method

Recommendation

✅ APPROVE - This PR represents a solid performance optimization that:

• Follows good engineering practices
• Maintains code quality and safety
• Provides measurable performance benefits
• Has minimal risk of introducing regressions
• Includes appropriate test coverage

The implementation demonstrates thoughtful analysis of when database operations can be optimized while preserving all existing functionality and safety guarantees.

---

[Copilot]

Pull Request Overview

This PR optimizes database operations by eliminating unnecessary post-mutation read queries. The changes analyze whether a read-back is required based on the requested fields and leverage SQL's RETURNING clause to return only the selected fields directly from mutation operations (create, update, delete, upsert). This avoids the performance overhead of separate read queries when relations or computed fields aren't needed.

Key changes:

• Introduced mutationNeedsReadBack() analysis to determine when post-mutation reads can be skipped
• Modified CRUD operations to use SQL RETURNING with selected fields instead of always reading back with ID fields
• Updated tests to validate the optimization works correctly with explicit select clauses

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/runtime/src/client/crud/operations/base.ts	Added mutationNeedsReadBack() method and updated CRUD methods to accept fieldsToReturn parameter for RETURNING clause optimization
packages/runtime/src/client/crud/operations/create.ts	Modified create operations to skip read-back when possible using the new analysis
packages/runtime/src/client/crud/operations/update.ts	Enhanced update/upsert operations with read-back analysis and additional check for relation-only updates
packages/runtime/src/client/crud/operations/delete.ts	Refactored delete operations to use RETURNING clause instead of pre-delete reads
tests/e2e/orm/client-api/*.test.ts	Added test cases with explicit select clauses to verify optimization and updated import ordering

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}