-
Notifications
You must be signed in to change notification settings - Fork 2k
Introduce morsel-driven Parquet scan #20481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
34963ed
a67f9ac
d0da5da
32eec3c
5dc895c
cc73788
d517b5d
de1606d
950f6db
7f57317
fd6d7fd
37126bf
2d3c33e
98f0ea9
a389b02
4065448
415315d
13b4977
a30c3f8
8b32ca8
876c296
d2df36b
869b7d3
67ea9ab
e845675
6885981
3384b8f
211d4fc
2db61f1
c859d6a
24b95fb
80fa1ec
1dcd401
04b08a6
9799b96
de29e40
aa27a43
9a4aa84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -227,8 +227,58 @@ impl RunQueryResult { | |
| format!("{}", pretty_format_batches(&self.result).unwrap()) | ||
| } | ||
|
|
||
| /// Extract ORDER BY column names from the query. | ||
| /// The query format is always: | ||
| /// `SELECT * FROM test_table ORDER BY <col> <dir> <nulls>, ... LIMIT <n>` | ||
| fn sort_columns(&self) -> Vec<String> { | ||
| let order_by_start = self.query.find("ORDER BY").unwrap() + "ORDER BY".len(); | ||
| let limit_start = self.query.rfind(" LIMIT").unwrap(); | ||
| self.query[order_by_start..limit_start] | ||
| .trim() | ||
| .split(',') | ||
| .map(|part| part.split_whitespace().next().unwrap().to_string()) | ||
| .collect() | ||
| } | ||
|
|
||
| /// Project `batches` to only include the named columns. | ||
| fn project_columns(batches: &[RecordBatch], cols: &[String]) -> Vec<RecordBatch> { | ||
| batches | ||
| .iter() | ||
| .map(|b| { | ||
| let schema = b.schema(); | ||
| let indices: Vec<usize> = cols | ||
| .iter() | ||
| .filter_map(|c| schema.index_of(c).ok()) | ||
| .collect(); | ||
| let columns: Vec<_> = | ||
| indices.iter().map(|&i| Arc::clone(b.column(i))).collect(); | ||
| let fields: Vec<_> = | ||
| indices.iter().map(|&i| schema.field(i).clone()).collect(); | ||
| let new_schema = Arc::new(Schema::new(fields)); | ||
| RecordBatch::try_new(new_schema, columns).unwrap() | ||
| }) | ||
| .collect() | ||
| } | ||
|
|
||
| fn is_ok(&self) -> bool { | ||
| self.expected_formatted() == self.result_formatted() | ||
| if self.expected_formatted() == self.result_formatted() { | ||
| return true; | ||
| } | ||
| // If the full results differ, compare only the ORDER BY column values. | ||
| // | ||
| // For queries with ORDER BY <col> LIMIT k, multiple rows may tie on the | ||
| // sort key (e.g. two rows with id=27 for ORDER BY id DESC LIMIT 1). | ||
| // SQL permits returning any of the tied rows, so with vs without dynamic | ||
| // filter pushdown may legitimately return different tied rows. | ||
| // | ||
| // The dynamic filter must not change the *sort-key values* of the top-k | ||
| // result. We verify correctness by projecting both results down to only | ||
| // the ORDER BY columns and comparing those. | ||
| let sort_cols = self.sort_columns(); | ||
| let expected_keys = Self::project_columns(&self.expected, &sort_cols); | ||
| let result_keys = Self::project_columns(&self.result, &sort_cols); | ||
| format!("{}", pretty_format_batches(&expected_keys).unwrap()) | ||
| == format!("{}", pretty_format_batches(&result_keys).unwrap()) | ||
| } | ||
|
Comment on lines
263
to
282
|
||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The json_shredding example disables morsel_driven execution at line 96, likely because the example relies on specific row group pruning metrics for assertions. However, there's no comment explaining why this is necessary. Consider adding a comment explaining that morsel_driven execution changes the metrics behavior and thus is disabled for this example's assertions to pass. This will help future maintainers understand why this configuration is needed.