alamb commented on code in PR #4243:
URL: https://github.com/apache/arrow-datafusion/pull/4243#discussion_r1024264617
##########
datafusion/core/tests/sql/parquet.rs:
##########
@@ -72,10 +72,10 @@ async fn parquet_with_sort_order_specified() {
})
.collect::<Vec<_>>();
- let options_sort = ListingOptions {
- file_sort_order: Some(file_sort_order),
- ..parquet_read_options.to_listing_options(target_partitions)
- };
+ let options_sort = ListingOptions::from_options(
Review Comment:
same comment as above
##########
datafusion/core/src/datasource/listing/table.rs:
##########
@@ -773,10 +788,9 @@ mod tests {
];
for (file_sort_order, expected_result) in cases {
- let options = ListingOptions {
- file_sort_order,
- ..options.clone()
- };
+ let options = ListingOptions::from_options(options.clone())
+ .with_file_sort_order(file_sort_order);
Review Comment:
Would this work? I think we can avoid adding `from_options`
```suggestion
let options = options.clone()
.with_file_sort_order(file_sort_order);
```
##########
datafusion/core/tests/sql/parquet.rs:
##########
@@ -57,10 +57,10 @@ async fn parquet_with_sort_order_specified() {
let target_partitions = 2;
// The sort order is not specified
- let options_no_sort = ListingOptions {
- file_sort_order: None,
- ..parquet_read_options.to_listing_options(target_partitions)
- };
+ let options_no_sort = ListingOptions::from_options(
+ parquet_read_options.to_listing_options(target_partitions),
+ )
+ .with_file_sort_order(None);
Review Comment:
```suggestion
let options_no_sort = parquet_read_options
.to_listing_options(target_partitions)
.with_file_sort_order(None);
```
##########
datafusion/core/src/datasource/listing/table.rs:
##########
@@ -254,6 +249,26 @@ impl ListingOptions {
file_sort_order: None,
}
}
+
+ /// Build ListingOptions from an existing set of ListingOptions
Review Comment:
I think typically in rust one uses `options.clone()` for this operation
I will leave a comment at the usage site
--
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]
