Skip to content

Commit 4767407

Browse files
committed
fix(sql): apply join LIMIT after post-join WHERE filters
A hash join with a post-join WHERE predicate truncated its output to the user LIMIT during the probe, before `filter_and_project` evaluated the predicate. The first `n` ON-matched rows were emitted and then filtered, so rows satisfying the WHERE clause that sorted after the first `n` probe matches were lost — frequently emptying the result entirely. Real-collection joins push their WHERE into the scanned side, leaving `post_filters` empty, so the ordering was harmless there. Virtual catalog joins keep both sides as inline ProviderScans and carry the WHERE as a join post-filter, which is where the truncation bit: e.g. `SELECT t.typname FROM pg_type t LEFT JOIN pg_range r ON t.oid = r.rngtypid WHERE t.typname = 'int4' LIMIT 5` returned 0 rows while the same query without LIMIT returned the row. The probe now runs under the byte-budget ceiling whenever post-filters are present, and the user LIMIT is applied after filtering — so LIMIT still truncates (narrow LIMIT) but never under-fills. Tests: pg_catalog_reflection gains catalog join + WHERE + wide/narrow LIMIT cases; sql_join_correctness gains the real-collection analogues.
1 parent 81169d3 commit 4767407

3 files changed

Lines changed: 155 additions & 1 deletion

File tree

nodedb/src/data/executor/handlers/join/hash_handlers.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,17 @@ impl CoreLoop {
333333
// budget) and, if the probe fills it, surface a deterministic
334334
// `ResourcesExhausted` rather than dropping the excess rows. A budget
335335
// of 0 means "unlimited" → truly unbounded output.
336-
let (probe_limit, enforce_output_budget) = if join.limit != usize::MAX {
336+
//
337+
// A user LIMIT may only cap the probe when there are no post-join
338+
// WHERE filters: `filter_and_project` runs AFTER the probe, so
339+
// truncating to `n` first would emit the first `n` ON-matched rows and
340+
// then discard those failing the WHERE clause — under-filling (or
341+
// emptying) the result even though later probe rows match. With
342+
// post-filters present the probe runs under the budget ceiling and the
343+
// user LIMIT is applied after filtering, below.
344+
let has_post_filters = !join.post_filter_bytes.is_empty();
345+
let (probe_limit, enforce_output_budget) = if join.limit != usize::MAX && !has_post_filters
346+
{
337347
(join.limit, false)
338348
} else if budget == 0 {
339349
(usize::MAX, false)
@@ -374,6 +384,13 @@ impl CoreLoop {
374384
);
375385
}
376386

387+
// Deferred user LIMIT: when post-join WHERE filters exist the probe
388+
// ran unbounded (see above) so the LIMIT must be applied here, after
389+
// the filters have retained the matching rows.
390+
if has_post_filters && join.limit != usize::MAX {
391+
results.truncate(join.limit);
392+
}
393+
377394
let payload = super::super::super::response_codec::encode_binary_rows(&results);
378395
self.response_with_payload(join.task, payload)
379396
}

nodedb/tests/pg_catalog_reflection.rs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,86 @@ async fn cross_vtable_join_filters_on_joined_column() {
548548
);
549549
}
550550

551+
/// A JOIN carrying BOTH a WHERE predicate and a LIMIT must still return the
552+
/// matching rows. The WHERE clause on a catalog join is evaluated after the
553+
/// join (there is no scan to push it into), so a LIMIT applied during the
554+
/// probe would cap the output before filtering and drop — often empty — the
555+
/// rows that actually match.
556+
#[tokio::test]
557+
async fn cross_vtable_join_with_where_and_limit_returns_matching_rows() {
558+
let srv = TestServer::start().await;
559+
srv.exec("CREATE COLLECTION reflect_join_where_limit (id INTEGER PRIMARY KEY)")
560+
.await
561+
.expect("create collection");
562+
563+
let unlimited = srv
564+
.query_text(
565+
"SELECT t.typname FROM pg_type t \
566+
LEFT JOIN pg_range r ON t.oid = r.rngtypid \
567+
WHERE t.typname = 'int4'",
568+
)
569+
.await
570+
.expect("join with WHERE evaluates");
571+
assert_eq!(
572+
unlimited,
573+
vec!["int4".to_string()],
574+
"baseline: the unlimited join must return the matching type"
575+
);
576+
577+
let limited = srv
578+
.query_text(
579+
"SELECT t.typname FROM pg_type t \
580+
LEFT JOIN pg_range r ON t.oid = r.rngtypid \
581+
WHERE t.typname = 'int4' LIMIT 5",
582+
)
583+
.await
584+
.expect("join with WHERE and LIMIT evaluates");
585+
assert_eq!(
586+
limited, unlimited,
587+
"a LIMIT wider than the result must not drop rows that satisfy the WHERE clause"
588+
);
589+
}
590+
591+
/// A LIMIT narrower than the filtered result set still truncates — the LIMIT
592+
/// moves after the post-join filter, it is not discarded.
593+
#[tokio::test]
594+
async fn cross_vtable_join_with_where_honors_narrow_limit() {
595+
let srv = TestServer::start().await;
596+
srv.exec("CREATE COLLECTION reflect_narrow_limit_a (id INTEGER PRIMARY KEY)")
597+
.await
598+
.expect("create collection");
599+
srv.exec("CREATE COLLECTION reflect_narrow_limit_b (id INTEGER PRIMARY KEY)")
600+
.await
601+
.expect("create collection");
602+
603+
let unlimited = srv
604+
.query_text(
605+
"SELECT c.relname FROM pg_class c \
606+
JOIN pg_namespace n ON n.oid = c.relnamespace \
607+
WHERE n.nspname = 'public'",
608+
)
609+
.await
610+
.expect("join with WHERE evaluates");
611+
assert!(
612+
unlimited.len() > 1,
613+
"test needs a multi-row baseline to check truncation, got {unlimited:?}"
614+
);
615+
616+
let limited = srv
617+
.query_text(
618+
"SELECT c.relname FROM pg_class c \
619+
JOIN pg_namespace n ON n.oid = c.relnamespace \
620+
WHERE n.nspname = 'public' LIMIT 1",
621+
)
622+
.await
623+
.expect("join with WHERE and LIMIT evaluates");
624+
assert_eq!(
625+
limited.len(),
626+
1,
627+
"LIMIT 1 must truncate the filtered rows to one: {limited:?}"
628+
);
629+
}
630+
551631
/// The three-way `pg_class ⋈ pg_attribute ⋈ pg_type` join is the literal
552632
/// shape `\d <table>` emits to describe a relation's columns and their types.
553633
#[tokio::test]

nodedb/tests/sql_join_correctness.rs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,3 +338,60 @@ async fn natural_join_is_not_cartesian() {
338338
}
339339
}
340340
}
341+
342+
// ---------------------------------------------------------------------------
343+
// LIMIT with post-join filters
344+
// ---------------------------------------------------------------------------
345+
346+
/// A join carrying both a post-join WHERE predicate and a LIMIT must return
347+
/// the matching rows: the LIMIT applies to the filtered result, never to the
348+
/// pre-filter probe output.
349+
#[tokio::test]
350+
async fn join_with_post_filter_and_wide_limit_keeps_matching_rows() {
351+
let server = TestServer::start().await;
352+
setup_join_tables(&server).await;
353+
354+
let unlimited = server
355+
.query_text(
356+
"SELECT j_t2.id FROM j_t1 \
357+
JOIN j_t2 ON j_t2.t1_id = j_t1.id \
358+
WHERE j_t2.y = 200",
359+
)
360+
.await
361+
.expect("join with WHERE evaluates");
362+
assert_eq!(unlimited, vec!["q".to_string()]);
363+
364+
let limited = server
365+
.query_text(
366+
"SELECT j_t2.id FROM j_t1 \
367+
JOIN j_t2 ON j_t2.t1_id = j_t1.id \
368+
WHERE j_t2.y = 200 LIMIT 5",
369+
)
370+
.await
371+
.expect("join with WHERE and LIMIT evaluates");
372+
assert_eq!(
373+
limited, unlimited,
374+
"a LIMIT wider than the filtered result must not drop matching rows"
375+
);
376+
}
377+
378+
/// A LIMIT narrower than the filtered result still truncates.
379+
#[tokio::test]
380+
async fn join_with_post_filter_honors_narrow_limit() {
381+
let server = TestServer::start().await;
382+
setup_join_tables(&server).await;
383+
384+
let rows = server
385+
.query_text(
386+
"SELECT j_t2.id FROM j_t1 \
387+
JOIN j_t2 ON j_t2.t1_id = j_t1.id \
388+
WHERE j_t2.y > 0 LIMIT 1",
389+
)
390+
.await
391+
.expect("join with WHERE and LIMIT evaluates");
392+
assert_eq!(
393+
rows.len(),
394+
1,
395+
"LIMIT 1 must truncate the filtered rows to one: {rows:?}"
396+
);
397+
}

0 commit comments

Comments
 (0)