feat(scheduler): add csv support for get_file_metadata grpc method #1011
Conversation
| @@ -297,13 +298,24 @@ impl<T: 'static + AsLogicalPlan, U: 'static + AsExecutionPlan> SchedulerGrpc | |||
| // TODO shouldn't this take a ListingOption object as input? | |||
|
|
|||
| let GetFileMetadataParams { path, file_type } = request.into_inner(); | |||
| let file_format: Arc<dyn FileFormat> = match file_type.as_str() { | |||
| let file_format: Result<Arc<dyn FileFormat>, Status> = match file_type.as_str() { | |||
| "parquet" => Ok(Arc::new(ParquetFormat::default())), | |||
There was a problem hiding this comment.
I've noticed that in the benchmark there's slightly different default for parquet:
ParquetFormat::default().with_enable_pruning(Some(true))I wonder if we need consistency or it's ok to leave it as is?
There was a problem hiding this comment.
I think pruning is default already, so in practice it will be the same.
| _ => Err(tonic::Status::unimplemented( | ||
| "get_file_metadata unsupported file type", | ||
| )), | ||
| }?; |
There was a problem hiding this comment.
I had to drop the "short circuit" from here to resolve the rust compilation error:
Type mismatch [E0308] expected `Result<Arc<CsvFormat>, Status>`, but found `Result<Arc<ParquetFormat>, Status>` please let me know if there's a better way of doing it?
|
@andygrove
|
|
@andygrove just a ping in case this notification is lost. |
|
Thank you for the feedback @Dandandan |
|
@Dandandan @etolbakov can you please elaborate what was the point of it ? |
|
@milenkovicm @Dandandan |
|
As project was not actively maintained and lot of work accumulated we decide to change project's scope and remove code so maintainers have smaller code base to maintain ( some details in #1067 ). This specific functionality has been removed as there is no clear benefit of keeping it. Currently, there is active work ongoing towards #974 @etolbakov if you wish to contribute please join discord channel, there are ongoing discussions on tasks and priority |
|
@milenkovicm makes sense! Thanks! |
Which issue does this PR close?
Closes #.
I was exploring the code and came across the
TODOabout thecsvfile format support. So decided to address it.Rationale for this change
The functionality extension.
What changes are included in this PR?
csv/tlbfile format supportAre there any user-facing changes?
No