Joseph-Rance commented on code in PR #4773:
URL: https://github.com/apache/arrow-rs/pull/4773#discussion_r1328661595
##########
parquet_derive/src/parquet_field.rs:
##########
@@ -682,6 +838,66 @@ impl Type {
}
}
+// returns quote of function needed to convert from parquet
+// types back into the types used in the struct fields.
+pub fn get_convertable_quote() -> proc_macro2::TokenStream {
Review Comment:
Yes we could. The issue I was facing is that the current `Field` struct
contains a fairly complex `ty` field, so if we want to convert based on the
stored type, we are going to have to do something like the code that follows
this line:
https://github.com/apache/arrow-rs/blob/175c7765939c0738defc736426c0b0a93b00bfa8/parquet_derive/src/parquet_field.rs#L87
Except we also need to match on every type path as well.
Since the compiler knows what type we want to return anyway, I figured this
might save us another 200 lines of match statement.
I understand the concern with including this with every quote. However, this
trait will be inserted inside the `read_from_row_group` function, so the
interface exposed to the end user is the same (i.e. the user can't access this
trait when using the macro). I guess there is the further issue of this making
the code somewhat more difficult to extend, but based on what you have said,
there aren't many planned upgrades here anyway.
Happy to instead use the match (or something else if you have any ideas), if
you think that would be better?
--
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]
