From b51bd1a2cd858238edad1097eaa646f3fbcb863b Mon Sep 17 00:00:00 2001 From: tsushanth <78000697+tsushanth@users.noreply.github.com> Date: Tue, 16 Jun 2026 12:05:01 -0700 Subject: [PATCH 1/2] fix(query-compiler): return None instead of panicking when a batched QuerySingle has zero keys (prisma/prisma#29642) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `QuerySingle::new` validated against `has_many_keys()` but never against `has_no_keys()`, so when the TS client batched concurrent `findUnique` calls and at least one had `where: { id: undefined }`, the stripped filter arrived here as an empty `QueryFilters`. Falling through to let (key, value) = first.get_single_key().unwrap(); panicked (`called Option::unwrap() on a None value at selection.rs:218`). The single-call path correctly rejects this with `PrismaClientValidationError`, but the dataloader/batched path bypasses that validation; in production we saw 28 PrismaClientRustPanicError crashes in a 42-second window, and innocent queries sharing the batch window were taken out as collateral when the panic poisoned the batch. This commit narrows the precondition check on `QuerySingle::new` to reject empty filters alongside many-key ones, and adds a `has_no_keys()` helper to make the intent obvious at the call site. `SelectionSet::new` already handles the None-result case — it maps to `SelectionSet::Empty` when every filter is empty, or `SelectionSet::Many(filters)` otherwise — so the upstream behaviour is preserved: the panic becomes a graceful fallthrough that downstream layers can already deal with. This is the lower half of the fix. The higher-level concern from the issue — that the batched path should reject `undefined` unique values with `PrismaClientValidationError` exactly like the single-call path — lives in the TS client and is left for a follow-up; once that lands, the path that reaches `QuerySingle::new` with an empty filter goes away entirely. The guard here is the safety net for any other code path that might construct an empty filter in the future. - query-compiler/core/src/query_document/selection.rs: new `QueryFilters::has_no_keys()` helper; extend the precondition in `QuerySingle::new` to short-circuit on empty filters; rewrite the docstring to call out the bug shape and link the issue - Added a `#[cfg(test)] mod tests` covering: (1) empty filter returns None (the direct regression), (2) mixed-batch with one empty filter returns None (the production shape), (3) the new helper distinguishes empty from populated, (4) well-formed single-key batches still coalesce (behaviour preservation) Verified locally: `cargo test -p query-core --lib selection` → 4 passed. `cargo build -p query-core` clean. --- .../core/src/query_document/selection.rs | 82 ++++++++++++++++++- 1 file changed, 80 insertions(+), 2 deletions(-) diff --git a/query-compiler/core/src/query_document/selection.rs b/query-compiler/core/src/query_document/selection.rs index de02c857f58..bebacf8ca4e 100644 --- a/query-compiler/core/src/query_document/selection.rs +++ b/query-compiler/core/src/query_document/selection.rs @@ -186,6 +186,10 @@ impl QueryFilters { self.0.len() > 1 } + pub fn has_no_keys(&self) -> bool { + self.0.is_empty() + } + pub fn get_single_key(&self) -> Option<&(String, ArgumentValue)> { self.0.first() } @@ -204,13 +208,26 @@ pub struct QuerySingle(String, Vec); impl QuerySingle { /// Attempt at building a single query filter from multiple query filters. - /// Returns `None` if one of the query filters have more than one key. + /// Returns `None` if the input is empty, if any of the query filters carry + /// more than one key, or if any of them carry zero keys. + /// + /// The zero-keys case fires when the TS client batches concurrent + /// `findUnique` calls and at least one of them has an `undefined` unique + /// value: validation catches that on the single-query path but the + /// batched path strips `undefined` keys and arrives here with an empty + /// `QueryFilters`. Falling through to `first.get_single_key().unwrap()` + /// in that state panicked on every batched concurrent call and took out + /// every other request sharing the batch window. See + /// https://github.com/prisma/prisma/issues/29642. pub fn new(query_filters: &[QueryFilters]) -> Option { if query_filters.is_empty() { return None; } - if query_filters.iter().any(|query_filters| query_filters.has_many_keys()) { + if query_filters + .iter() + .any(|query_filters| query_filters.has_many_keys() || query_filters.has_no_keys()) + { return None; } @@ -399,3 +416,64 @@ fn single_to_multi_filter(obj: ArgumentValueObject) -> ArgumentValueObject { new_obj } + +#[cfg(test)] +mod tests { + use super::*; + use crate::ArgumentValue; + + fn make_filter(pairs: &[(&str, i64)]) -> QueryFilters { + QueryFilters( + pairs + .iter() + .map(|(k, v)| ((*k).to_owned(), ArgumentValue::int(*v))) + .collect(), + ) + } + + // Regression for https://github.com/prisma/prisma/issues/29642. + // + // When the TS client batches concurrent `findUnique` calls and at least + // one has an `undefined` unique value, the value is stripped and the + // batched path arrives at `QuerySingle::new` with one or more empty + // `QueryFilters`. Before the fix, `has_many_keys()` was the only + // pre-condition checked, so empty filters fell through to + // `first.get_single_key().unwrap()` and panicked, taking out every other + // request sharing the batch window as collateral. Now they return None, + // which `SelectionSet::new` maps to `SelectionSet::Many(filters)` (or + // `SelectionSet::Empty` for the all-empty case) without panicking. + #[test] + fn query_single_new_returns_none_for_empty_filter_instead_of_panicking() { + let filters = vec![QueryFilters(Vec::new())]; + assert!(QuerySingle::new(&filters).is_none()); + } + + #[test] + fn query_single_new_returns_none_when_any_filter_in_batch_is_empty() { + // The bug surfaces specifically when *concurrent* batched callers + // mix valid + invalid filters; the unwrap fires on the first empty + // one regardless of position. + let filters = vec![ + make_filter(&[("id", 1)]), + QueryFilters(Vec::new()), // the bad one + make_filter(&[("id", 2)]), + ]; + assert!(QuerySingle::new(&filters).is_none()); + } + + #[test] + fn query_filters_has_no_keys_distinguishes_empty_from_populated() { + assert!(QueryFilters(Vec::new()).has_no_keys()); + assert!(!make_filter(&[("id", 1)]).has_no_keys()); + } + + #[test] + fn query_single_new_still_works_for_well_formed_single_key_batch() { + // Behaviour-preservation sanity check — the fix narrows what's + // accepted, but valid single-key batches must continue to coalesce. + let filters = vec![make_filter(&[("id", 1)]), make_filter(&[("id", 2)])]; + let single = QuerySingle::new(&filters).expect("valid batch should coalesce"); + assert_eq!(single.0, "id"); + assert_eq!(single.1.len(), 2); + } +} From 73f1408390db1867a7badf6cd1c1f5e4a39606b1 Mon Sep 17 00:00:00 2001 From: tsushanth <78000697+tsushanth@users.noreply.github.com> Date: Wed, 17 Jun 2026 07:02:16 -0700 Subject: [PATCH 2/2] docs(query-compiler): correct regression-test comment per code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment claimed the all-empty case maps to `SelectionSet::Empty`, but that branch is only taken when the input `Vec` itself is empty (selection.rs:277). A non-empty Vec full of empty filters falls into `SelectionSet::Many(filters)` instead (selection.rs:278). No behavior change — just a more accurate comment. --- query-compiler/core/src/query_document/selection.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/query-compiler/core/src/query_document/selection.rs b/query-compiler/core/src/query_document/selection.rs index bebacf8ca4e..0f08c2b8810 100644 --- a/query-compiler/core/src/query_document/selection.rs +++ b/query-compiler/core/src/query_document/selection.rs @@ -440,8 +440,10 @@ mod tests { // pre-condition checked, so empty filters fell through to // `first.get_single_key().unwrap()` and panicked, taking out every other // request sharing the batch window as collateral. Now they return None, - // which `SelectionSet::new` maps to `SelectionSet::Many(filters)` (or - // `SelectionSet::Empty` for the all-empty case) without panicking. + // which `SelectionSet::new` maps to `SelectionSet::Many(filters)` for any + // non-empty input vector (the all-empty case falls into the same branch — + // `SelectionSet::Empty` is only reached when the input `Vec` itself is + // empty, see `SelectionSet::new` above). #[test] fn query_single_new_returns_none_for_empty_filter_instead_of_panicking() { let filters = vec![QueryFilters(Vec::new())];