fix(policy): post-update issue when no rows are updated

[ymc9]

Summary by CodeRabbit

• Refactor
  • Optimized post-update policy checks to run only when an update actually modifies records, reducing unnecessary processing.
  • Skips building and executing follow-up queries when no rows are affected, improving runtime efficiency.
  • Streamlines control flow for updates with zero affected rows, leading to faster no-op operations.
  • No changes to behavior, configuration, or error handling; existing functionality remains intact.
  • Users may notice slightly improved performance for updates that do not alter any data.

[coderabbitai]

Walkthrough

The post-mutation policy evaluation now checks both hasPostUpdatePolicies and that result.rows.length > 0 before executing post-update logic, skipping query construction and execution when no rows are affected. No other logic or error handling was modified.

Changes

Cohort / File(s)	Summary of Changes
Policy handler update logic packages/runtime/src/plugins/policy/policy-handler.ts	Added a guard to require result.rows.length > 0 along with hasPostUpdatePolicies before running post-update policy logic, preventing unnecessary query building when zero rows are updated.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A nibble of logic, hop, don’t run—
If rows are zero, the work is none.
I twitch my whiskers, checks aligned,
Skip the query, save some time.
Carrot saved, cycles won—commit is done! 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly describes the core change by indicating a fix in the policy plugin that addresses an issue when no rows are updated after a post-update operation, matching the code change which adds a check for zero affected rows before executing post‐update logic.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/post-update-fix

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 546ba8e and 2283d2f.

📒 Files selected for processing (1)

• packages/runtime/src/plugins/policy/policy-handler.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

{packages,samples,tests}/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place packages only under packages/, samples/, or tests/

Files:

• packages/runtime/src/plugins/policy/policy-handler.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: build-test (20.x, postgresql)
• GitHub Check: build-test (20.x, sqlite)
• GitHub Check: claude-review

🔇 Additional comments (1)

packages/runtime/src/plugins/policy/policy-handler.ts (1)

127-127: Good defensive check to prevent unnecessary post-update query execution.

Adding result.rows.length > 0 correctly skips post-update policy evaluation when no rows were affected. This prevents:

• Building meaningless ID conditions from an empty row set (line 129)
• Executing a validation query that would trivially succeed with a count comparison against 0 (line 164)
• Potential edge cases with empty result handling

The fix aligns with the semantic expectation that post-update policies only apply when rows are actually updated.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Copilot]

Pull Request Overview

This PR fixes a policy handler issue where post-update policy checks were being executed even when no rows were updated, which could cause unnecessary processing or potential errors.

• Added a condition to check that rows were actually updated before running post-update policies
• Prevents post-update policy execution when the update operation affects zero rows

---

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

[claude]

Claude finished @ymc9's task —— View job

---

Pull Request Review

I've analyzed the pull request and here's my comprehensive review:

Review Tasks

• Read and analyze the changed file
• Examine surrounding code context
• Check for related test files
• Assess code quality and best practices
• Identify potential bugs or issues
• Evaluate performance considerations
• Review security concerns
• Assess test coverage needs
• Provide comprehensive feedback

---

Summary

This PR fixes a critical logic issue in the policy handler where post-update policy checks were being executed even when no rows were actually updated. The fix adds a simple condition result.rows.length > 0 to prevent unnecessary post-update policy evaluation.

Code Quality ✅

Strengths:

• Minimal and focused: The change is surgical and addresses the specific issue without side effects
• Clear intent: The condition result.rows.length > 0 clearly communicates that post-update policies should only run when rows were actually updated
• Consistent with existing patterns: The code follows the established style and uses the same result inspection pattern used elsewhere in the file

Location: packages/runtime/src/plugins/policy/policy-handler.ts:127

Bug Analysis ✅

Issue Fixed:

• Root cause: Post-update policies were being evaluated even when result.rows.length === 0, causing unnecessary computation and potential logical errors
• Impact: Could lead to policy violations being checked against empty result sets, potentially causing confusing error messages or performance overhead
• Fix effectiveness: The added condition properly guards the post-update logic to only execute when there are actual updated rows to validate

Performance Considerations ✅

Improvements:

• Reduced overhead: Prevents building complex policy filter queries when no rows were updated
• Database query elimination: Avoids the expensive postUpdateQuery execution on lines 179-186 when unnecessary
• Early return: Efficiently short-circuits the post-update policy evaluation path

No performance regressions identified.

Security Concerns ✅

Assessment:

• No security implications: This is a defensive programming improvement that actually enhances security by preventing potential edge case exploits
• Policy integrity maintained: The fix ensures post-update policies are only evaluated when logically appropriate
• No bypass concerns: The change doesn't weaken policy enforcement, only optimizes when it's applied

Test Coverage Assessment ⚠️

Current State:

• Extensive post-update policy tests exist in /packages/runtime/test/policy/crud/post-update.test.ts
• Tests cover various scenarios including simple rules, relation conditions, and before() function usage
• Tests verify both successful updates and policy rejections

Gaps Identified:

• Missing edge case: No specific test for the scenario where an update operation matches zero rows
• Recommended addition: A test case that verifies post-update policies are not evaluated when updateMany() or update() operations match no existing records

Recommendations

1. Add test case for the fixed scenario:

it('skips post-update policy when no rows are updated', async () => {
    const db = await createPolicyTestClient(`
        model Foo {
            id Int @id
            x  Int
            @@allow('all', true)
            @@allow('post-update', x > 100) // restrictive condition
        }
    `);
    
    // No matching records to update
    await expect(db.foo.updateMany({ 
        where: { id: 999 }, 
        data: { x: 1 } 
    })).resolves.toMatchObject({ count: 0 });
});

2. Consider logging: For debugging purposes, consider adding a debug log when post-update policies are skipped due to zero rows

Verdict: ✅ APPROVED

This is a well-executed bug fix that:

• Solves a real logical issue
• Improves performance
• Maintains security
• Uses clean, readable code
• Requires minimal test coverage enhancement

The change is production-ready and should be merged.

---