chore: minor fixes on qc parameterization #5747
Open
Conversation
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
Remove `ScalarCondition::InTemplate` and `ScalarCondition::NotInTemplate` and instead add placeholders as the third variant in `ConditionListValue`. This simplifies the implementation and makes some illegal states unrepresentable (previously it was possible to construct `ScalarCondition::InTemplate`/`ScalarCondition::NotInTemplate` with arbitrary `PrismaValue`s and with field references).
Require placeholder types to be specified in the JSON protocol representation of the placholders and parse them in `JsonProtocolAdapter`.
Generator calls should only be parsed when we expect a scalar. This change also adds some debug assertions to ensure that the return types are correct. For performance and size reasons, we do not compile these checks in release builds because generators do not come from user input and cannot be represented in the JSON protocol.
Extract the repetitive `ValidationError::invalid_argument_type` construction code and mark it as `#[inline(never)]` to save some extra bytes in the compiled binary since it's an error path and therefore not performance critical.
Fix a typo in invalid argument value validation error: remove a duplicate "argument" word that also had a typo in it.
`matches`/`not_matches` operations needed to be adjusted to accept placeholders. The corresponding fields in DMMF were already marked as parameterizable so QC panicked in Client tests here.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates fixes for query compiler (QC) parameterization, specifically addressing panic issues when placeholders are used in full-text search operations. The changes refactor how placeholders are handled throughout the query pipeline, remove the unused Param scalar type, and add proper validation to prevent placeholders from being used in non-parameterizable fields.
Changes:
- Refactored placeholder handling by removing
InTemplate/NotInTemplatevariants in favor of usingPlaceholderwithin existingIn/NotInvariants - Added
is_parameterizableflag to input fields with comprehensive marking of which fields accept placeholders (filters, data fields) vs which don't (take, skip, orderBy) - Fixed full-text search to properly handle placeholder expressions instead of just string values
- Added validation to reject placeholders in non-parameterizable locations with helpful error messages
Reviewed changes
Copilot reviewed 44 out of 46 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| query-engine/connectors/mongodb-query-connector/src/filter.rs | Added unimplemented handlers for Placeholder in list conditions (MongoDB doesn't support QC yet) |
| query-compiler/schema/src/query_schema.rs | Removed unused Param scalar type variant |
| query-compiler/schema/src/input_types.rs | Added is_parameterizable field and methods to InputField with tests |
| query-compiler/schema/src/build/input_types/objects/*.rs | Marked appropriate filter and order-by fields as parameterizable |
| query-compiler/schema/src/build/input_types/fields/*.rs | Marked data input fields and filter fields as parameterizable where appropriate |
| query-compiler/request-handlers/src/protocols/json/protocol_adapter.rs | Updated placeholder parsing to deserialize full Placeholder struct |
| query-compiler/query-structure/src/order_by.rs | Changed search field type from String to PrismaValue to support placeholders |
| query-compiler/query-structure/src/filter/scalar/* | Removed InTemplate/NotInTemplate variants, added Placeholder to ConditionListValue |
| query-compiler/query-compiler/src/translate/query/read.rs | Updated to use Placeholder struct and In variant instead of InTemplate |
| query-compiler/query-builders/sql-query-builder/src/*.rs | Added support for converting placeholder values to SQL expressions in filters and ordering |
| quaint/src/visitor/*.rs | Changed matches signature to accept Expression instead of Cow for placeholder support |
| quaint/src/ast/*.rs | Updated Compare, Comparable trait, and related types for Expression-based matches |
| query-compiler/dmmf/src/*.rs | Added is_parameterizable field to DMMF serialization |
| query-compiler/core/src/query_document/parser.rs | Added comprehensive placeholder validation with type checking |
| query-compiler/core-tests/tests/*.rs | Added test cases for placeholder validation in take/skip fields |
| libs/user-facing-errors/src/query_engine/validation.rs | Fixed typo: "agument" to "argument" |
| libs/prisma-value/src/lib.rs | Added serde flatten and into_placeholder helper method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
Wasm Query Compiler File Size
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR exists for a couple of reasons:
prisma/prisma