kosiew commented on code in PR #24579:
URL: https://github.com/apache/datafusion/pull/24579#discussion_r3878609299
##########
datafusion/core/tests/fuzz_cases/join_fuzz.rs:
##########
@@ -1500,30 +1503,63 @@ async fn pwmj_collect_ids(
}))
.await;
- let mut ids = Vec::new();
+ let id_column = |batch: &RecordBatch, col: usize| {
+ batch
+ .column(col)
+ .as_any()
+ .downcast_ref::<Int32Array>()
+ .unwrap()
+ .clone()
+ };
+
+ let mut pairs = Vec::new();
for batches in per_partition {
for batch in batches.unwrap().unwrap() {
- let col = batch
- .column(0)
- .as_any()
- .downcast_ref::<Int32Array>()
- .unwrap();
- ids.extend((0..col.len()).map(|i| col.value(i)));
+ let left = id_column(&batch, 0);
+ // Left is (id, k), so the right side's `id` follows it -- when
there is one.
+ let right = (batch.num_columns() > 2).then(|| id_column(&batch,
2));
+ for row in 0..batch.num_rows() {
+ pairs.push((
+ left.is_valid(row).then(|| left.value(row)),
+ right
+ .as_ref()
+ .filter(|r| r.is_valid(row))
+ .map(|r| r.value(row)),
+ ));
+ }
}
}
- ids.sort_unstable();
- ids
+ pairs.sort_unstable();
+ pairs
}
+/// Differential test for every join type `PiecewiseMergeJoin` supports,
against a
+/// `NestedLoopJoin` oracle.
+///
+/// `Left`/`Full` are the ones with teeth: their unmatched buffered rows are
derived from the
+/// shared `min_marked` watermark rather than materialized per row, and that
encoding is only
+/// valid because every match marks a *suffix* of the buffered side. The
dimensions the
+/// cheaper tests do not reach are the ones that matter here -- `pwmj.slt` and
the unit tests
+/// both run the streamed side at a single partition and the default batch
size, so neither
+/// covers several partitions racing to run the final pass, nor the mid-scan
resume path a
+/// small batch size forces.
#[tokio::test(flavor = "multi_thread")]
-async fn fuzz_pwmj_existence_matches_nested_loop() {
- // A small batch size splits the final-pass output across several
coalesced batches even
- // for these tiny inputs, covering that boundary too.
+async fn fuzz_pwmj_matches_nested_loop() {
Review Comment:
Nice expansion of the fuzz coverage here. One optional suggestion: could we
add a named deterministic Left or Full join case that combines NULLs,
duplicates, multiple streamed partitions, and `batch_size=3`? The existing
reproducible seed range already exercises these dimensions and reports the
failing seed and inputs, so I don't think this needs to block the PR. A named
case could just make regressions in this specific path a little easier to
diagnose.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
