Update parquet::basic::LogicalType to be more idomatic
#1612
Conversation
…cale, precision }, Fix String proc macro error
…d_to_u_t_c: bool, unit: TimeUnit }
…mp { is_adjusted_to_u_t_c: bool, unit: TimeUnit }
…idth: i8, is_signed: bool }
Codecov Report
@@ Coverage Diff @@
## master #1612 +/- ##
==========================================
- Coverage 82.95% 82.89% -0.07%
==========================================
Files 193 193
Lines 55439 55464 +25
==========================================
- Hits 45992 45978 -14
- Misses 9447 9486 +39
Continue to review full report at Codecov.
|
parquet/src/arrow/schema.rs
Outdated
| @@ -678,20 +678,20 @@ impl ParquetTypeConverter<'_> { | |||
| (32, false) => Ok(DataType::UInt32), | |||
| _ => Err(ArrowError(format!( | |||
| "Cannot create INT32 physical type from {:?}", | |||
| t | |||
| self.schema.get_basic_info().logical_type(), | |||
There was a problem hiding this comment.
You could use t @ LogicalType::Integer { ... } in the match expression, so that you can use t here
There was a problem hiding this comment.
I've spent some time looking into this. This syntax causes t to be evaluated as a LogicalType rather than a LogicalType::Integer. It then can't use it for match (t.bit_width, t.is_signed) . I also tried t @ LogicalType::Integer { bit_width, is_signed }, but then I run into borrow issues. Is there some order-of-op trick I'm missing?
There was a problem hiding this comment.
Does this help - https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=ec783c91c37289091cce85107031b3df
Btw you can drop the "ref" if you make LogicalType implement Copy
There was a problem hiding this comment.
Ahhhh I missed the ref, thanks. Adding a ref seemed better than deriving Copy for LogicalType, TimeUnit, MilliSeconds, etc.
|
Thanks again for this 👍 |
alamb
changed the title
parquet::basic::LogicalType to be more idomatic
Which issue does this PR close?
Closes #1554.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?
See #1554. I can provide more detail if needed, but nothing really goes beyond the scope of the issue.