Add partitioned exact filter stage#2207
Conversation
Signed-off-by: Vibhu Jawa <vjawa@nvidia.com>
311db4b to
538b0dc
Compare
| reference = cudf.read_parquet( | ||
| reference_file, | ||
| columns=self.key_fields, | ||
| **self.reference_read_kwargs, | ||
| ) | ||
| self._require_key_fields(left, "left") | ||
| self._require_key_fields(reference, "reference") |
There was a problem hiding this comment.
_require_key_fields(reference, ...) is unreachable in the error case
Because cudf.read_parquet is called with columns=self.key_fields, cuDF will raise its own error (e.g. RuntimeError) if any requested column is absent from the Parquet schema — before execution ever reaches _require_key_fields(reference, "reference"). When the read succeeds, the returned frame has exactly self.key_fields as its columns, so the check trivially passes. The user therefore always gets a raw cuDF error instead of the more descriptive "reference partition is missing exact key columns" message. Moving the schema check to before the read (or reading without the columns constraint and selecting afterwards) would let the custom error fire correctly.
| _NON_SINGLE_FILE_WRITE_KWARGS = [ | ||
| "index", | ||
| "metadata_file_path", | ||
| "partition_cols", | ||
| "partition_file_name", | ||
| "partition_offsets", | ||
| ] |
There was a problem hiding this comment.
"index" is semantically misplaced in _NON_SINGLE_FILE_WRITE_KWARGS
partition_cols, metadata_file_path, partition_offsets, and partition_file_name are genuinely multi-file/partitioning kwargs. "index" is unrelated to that — it controls whether the DataFrame index is written as a column. Grouping it here means the docstring claim ("Dataset-partitioning and auxiliary-metadata output options are not accepted") does not describe index. A caller blocked by this list gets a confusing error, and the list name would mislead future maintainers. Consider separating it into its own constant (e.g. _CONTROLLED_WRITE_KWARGS = ["index"]) or at least adding a comment.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if output_file in task.data: | ||
| msg = "output partition would overwrite its left input partition" | ||
| raise ValueError(msg) |
There was a problem hiding this comment.
Overwrite guard uses naive string equality on paths
output_file in task.data is an exact string comparison. If task.data paths come from the shuffler with a different representation than self.output_path — for example, with a double slash, a resolved symlink, or a differently-cased URI — the check silently passes and the stage overwrites its own input. Since ShuffleStage derives its output paths from an external C++ library, path normalisation is not guaranteed to be consistent with rstrip("/"). Using os.path.realpath (or fsspec's _strip_protocol + normpath) on both sides before comparing would make the guard robust.
Description
Adds a small GPU stage for exact bucket-local left-semi and left-anti filtering after two datasets have been partitioned by compatible ShuffleStage runs.
The stage reads only the matching reference partition, preserves every matching or non-matching left row and its multiplicity, and avoids broadcasting a large reference set. It validates matching partition metadata, rejects row-subsetting reads and multi-file writes that would violate the complete-partition contract, and emits deterministic part.N.parquet outputs.
Callers must use the same key fields, hash behavior, and partition count for both upstream shuffles. ShuffleStage emits every partition, including empty partitions, so a missing matching reference file is treated as an incomplete shuffle and fails closed.
Usage
Use mode="leftsemi" to emit the exact overlap instead.
Validation
This stage is intentionally separate from persistent membership filtering in interleaved batches: it reduces compatible disk-shuffled FileGroupTask partitions.
Checklist