fix: reject select with only false fields to prevent empty SELECT SQL

[ymc9]

fixes #2344

When all fields in select are set to false, the query builder generates an empty SELECT clause (select from "Table") causing a SQL syntax error. This adds input validation to reject such payloads with a clear error message.

Summary by CodeRabbit

• Bug Fixes
  • Added validation to prevent queries with invalid select operations. Queries must now include at least one field set to true; selections with all fields false will be rejected with a descriptive error message.

[claude]

Claude finished @ymc9's task —— View job

---

Code Review Complete ✅

I've thoroughly reviewed PR #2401 that addresses the empty SELECT SQL issue. Here's my comprehensive feedback:

Overview

This PR successfully fixes issue #2344 by adding validation to reject select objects that contain only false values, which would otherwise generate invalid empty SELECT SQL queries.

✅ Strengths

1. Correct Fix Strategy: The solution properly addresses the root cause by adding input validation at the Zod schema level, preventing invalid queries from reaching the SQL generation stage.

2. Comprehensive Application: The refineForSelectHasTruthyField validation is correctly applied to all relevant schema methods:

   • makeFindSchema (findUnique, findFirst, findMany)
   • makeRelationSelectIncludeSchema (nested selects)
   • makeCreateSchema, makeCreateManyAndReturnSchema
   • makeUpdateSchema, makeUpdateManyAndReturnSchema, makeUpsertSchema
   • makeDeleteSchema

3. Clear Error Message: The validation provides a descriptive error message: "select" must have at least one truthy value

4. Good Test Coverage: The regression test covers:

   • Single field with false value
   • All fields with false values
   • Mixed true/false values (should work)
   • Proper result verification

🔍 Technical Analysis

Validation Logic (factory.ts:2031-2039):

private refineForSelectHasTruthyField(schema: ZodType) {
    return schema.refine((value: any) => {
        const select = value['select'];
        if (!select || typeof select !== 'object') {
            return true;
        }
        return Object.values(select).some((v) => v);
    }, '"select" must have at least one truthy value');
}

Strengths:

• ✅ Correctly handles missing/null select (returns true)
• ✅ Validates that select is an object
• ✅ Uses Object.values().some() to check for truthy values
• ✅ Follows existing pattern consistent with other refinements

Edge Case Considerations:

• ✅ Handles select: null correctly (validation passes)
• ✅ Handles select: undefined correctly (validation passes)
• ✅ Handles select: {} correctly (would fail validation - no values)
• ✅ Handles nested objects like select: { user: { id: true } } correctly

📝 Minor Code Quality Notes

1. Formatting Changes: The PR includes some minor formatting/line length adjustments that are consistent with project style.

2. Method Placement: The new refineForSelectHasTruthyField method is logically placed near other refinement methods.

🏃♂️ Performance Assessment

• Low Impact: The validation runs only during input validation phase, not during query execution
• Efficient Check: Object.values().some() is optimized and stops at first truthy value
• Minimal Overhead: Only adds validation when select is present

🔒 Security Assessment

• No Security Concerns: The validation doesn't introduce any security vulnerabilities
• Input Sanitization: Actually improves input validation by preventing malformed queries

🧪 Test Quality

The regression test is well-structured:

• ✅ Tests the exact error conditions
• ✅ Verifies error messages match expected pattern
• ✅ Includes positive test case (mixed true/false)
• ✅ Validates actual result structure
• ✅ Uses proper async/await pattern

📋 Todo List Status:

• Read and understand the validation implementation in factory.ts
• Review the test coverage in the regression test file
• Check repository guidelines (CLAUDE.md)
• Analyze code quality and best practices
• Identify potential bugs or edge cases
• Assess performance implications
• Check for security concerns
• Provide comprehensive feedback

🎯 Final Verdict

LGTM - This is a solid, well-implemented fix that:

• ✅ Solves the reported issue correctly
• ✅ Follows project conventions and patterns
• ✅ Has comprehensive test coverage
• ✅ Includes proper error messaging
• ✅ Has minimal performance impact
• ✅ Maintains backwards compatibility

The solution is production-ready and should prevent the SQL syntax errors described in issue #2344.

---

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR fixes a bug where ORM queries with select objects containing only false values (e.g., { id: false }) would generate invalid SQL. A new validation step ensures that non-empty select objects contain at least one truthy value and is applied across all CRUD operations. A regression test validates the fix.

Changes

Cohort / File(s)	Summary
Select validation enforcement packages/orm/src/client/zod/factory.ts	Introduced private method refineForSelectHasTruthyField() that enforces at least one truthy value in select objects. Applied consistently across find, create, update, upsert, delete, count, aggregate, groupBy, and relation schemas. Error message: "select" must have at least one truthy value.
Regression test tests/regression/test/issue-2344.test.ts	New test file validating that findMany and findFirst with all-false select objects reject with appropriate error, and that mixed selections work correctly.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A select with false, false, false—how strange!
Now validation saves the day,
At least one true must light the way,
Or SQL syntax goes astray.
With tests in place, we're all okay! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: rejecting select objects with only false fields to prevent empty SELECT SQL.
Linked Issues check	✅ Passed	The code changes directly address issue #2344 by validating that select objects contain at least one truthy value, preventing empty SELECT clauses.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the select validation issue. The factory changes add validation logic and the test file validates the fix.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/issue-2344-select-all-false

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/regression/test/issue-2344.test.ts (1)

1-43: Regression test is well-structured and validates the core fix correctly.

The three cases (all-false reject, all-false reject with full field list, mixed pass) directly correspond to the reported issue and the new refinement logic. No issues found.

One optional improvement: the validation is also applied to create, update, upsert, and delete (all of which accept a select field), but none of those paths are exercised here. Extending the test with at least one mutation operation would ensure regression coverage doesn't erode if a future refactor removes the refinement from a non-find path.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/regression/test/issue-2344.test.ts` around lines 1 - 43, Add a mutation
case to the regression test to exercise validation of select on mutation
operations: update the test in issue-2344.test.ts to include at least one
mutation (e.g., call db.user.update or db.user.create/updateMany/upsert/delete
with a select containing all-false and assert it rejects with the same /"select"
must have at least one truthy value/ error), and also include a mixed true/false
select mutation that succeeds to mirror the find* checks; reference the existing
db.user.create used to seed data and the db.user.update/db.user.upsert (or
db.user.create with select) call sites to locate where to add the new
assertions.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/regression/test/issue-2344.test.ts`:
- Around line 1-43: Add a mutation case to the regression test to exercise
validation of select on mutation operations: update the test in
issue-2344.test.ts to include at least one mutation (e.g., call db.user.update
or db.user.create/updateMany/upsert/delete with a select containing all-false
and assert it rejects with the same /"select" must have at least one truthy
value/ error), and also include a mixed true/false select mutation that succeeds
to mirror the find* checks; reference the existing db.user.create used to seed
data and the db.user.update/db.user.upsert (or db.user.create with select) call
sites to locate where to add the new assertions.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d49c39e and 050ba49.

📒 Files selected for processing (2)

• packages/orm/src/client/zod/factory.ts
• tests/regression/test/issue-2344.test.ts