You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/08/24 20:49:50 UTC
[GitHub] [arrow-rs] xrl opened a new pull request #712: Parquet Derive: remove obscure feature flags, make chrono time emit converted type
xrl opened a new pull request #712:
URL: https://github.com/apache/arrow-rs/pull/712
# Which issue does this PR close?
Closes #711
# Rationale for this change
I could not upgrade parquet-rs for my derive-heavy project.
# What changes are included in this PR?
* Remove obscure features which hide the useful tests
* Add support for converted_type so we get `TIMESTAMP_MILLIS`
# Are there any user-facing changes?
I have tested it on my code base and it works without changes. Users may want to remove the feature flags in the Cargo.toml.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] xrl commented on pull request #712: Parquet Derive: remove obscure feature flags, make chrono time emit converted type
Posted by GitBox <gi...@apache.org>.
xrl commented on pull request #712:
URL: https://github.com/apache/arrow-rs/pull/712#issuecomment-905700942
@nevi-me it seems like you've taken to proc macro hacking in here. perhaps this is something you could review?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] xrl commented on pull request #712: Parquet Derive: remove obscure feature flags, make chrono time emit converted type
Posted by GitBox <gi...@apache.org>.
xrl commented on pull request #712:
URL: https://github.com/apache/arrow-rs/pull/712#issuecomment-909861336
@alamb thanks for the merge! I was just coming back to tackle a split up. I would love to get this core functionality in to `5.x`. How can I help do that? I can prepare a PR for it.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on pull request #712: Parquet Derive: remove obscure feature flags, make chrono time emit converted type
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #712:
URL: https://github.com/apache/arrow-rs/pull/712#issuecomment-907611998
@xrl let me know if it would be helpful to try and backport this into the arrow 5.x releases (e.g. arrow 5.4.0 )
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on a change in pull request #712: Parquet Derive: remove obscure feature flags, make chrono time emit converted type
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #712:
URL: https://github.com/apache/arrow-rs/pull/712#discussion_r696022605
##########
File path: parquet_derive/src/parquet_field.rs
##########
@@ -534,13 +545,23 @@ impl Type {
})) }
}
"NaiveDate" => quote! { Some(LogicalType::DATE(Default::default())) },
+ "NaiveDateTime" => quote! { None },
Review comment:
I double checked and I agree there does not seem to be any `DATETIME` logical type in https://sourcegraph.com/github.com/apache/arrow-rs/-/blob/parquet/src/basic.rs?L166
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb commented on pull request #712: Parquet Derive: remove obscure feature flags, make chrono time emit converted type
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #712:
URL: https://github.com/apache/arrow-rs/pull/712#issuecomment-916390836
Hi @xrl -- sorry for the delay in responding -- I was on vacation and am now catching back up.
I made a cherry-pick PR without the feature flag changes here: https://github.com/apache/arrow-rs/pull/757
Any chance you can give it a look?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] xrl commented on pull request #712: Parquet Derive: remove obscure feature flags, make chrono time emit converted type
Posted by GitBox <gi...@apache.org>.
xrl commented on pull request #712:
URL: https://github.com/apache/arrow-rs/pull/712#issuecomment-909861336
@alamb thanks for the merge! I was just coming back to tackle a split up. I would love to get this core functionality in to `5.x`. How can I help do that? I can prepare a PR for it.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb merged pull request #712: Parquet Derive: remove obscure feature flags, make chrono time emit converted type
Posted by GitBox <gi...@apache.org>.
alamb merged pull request #712:
URL: https://github.com/apache/arrow-rs/pull/712
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] codecov-commenter commented on pull request #712: Parquet Derive: remove obscure feature flags, make chrono time emit converted type
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #712:
URL: https://github.com/apache/arrow-rs/pull/712#issuecomment-904985801
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/712?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#712](https://codecov.io/gh/apache/arrow-rs/pull/712?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (55fdd4f) into [master](https://codecov.io/gh/apache/arrow-rs/commit/8308615d40e14caa5cdbee118ecc2f46696b920f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8308615) will **increase** coverage by `0.00%`.
> The diff coverage is `48.27%`.
> :exclamation: Current head 55fdd4f differs from pull request most recent head 7568e71. Consider uploading reports for the commit 7568e71 to get more accurate results
[](https://codecov.io/gh/apache/arrow-rs/pull/712?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #712 +/- ##
=======================================
Coverage 82.46% 82.47%
=======================================
Files 168 168
Lines 47419 47461 +42
=======================================
+ Hits 39104 39143 +39
- Misses 8315 8318 +3
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/712?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [parquet\_derive/src/parquet\_field.rs](https://codecov.io/gh/apache/arrow-rs/pull/712/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldF9kZXJpdmUvc3JjL3BhcnF1ZXRfZmllbGQucnM=) | `68.86% <44.44%> (+2.69%)` | :arrow_up: |
| [parquet\_derive\_test/src/lib.rs](https://codecov.io/gh/apache/arrow-rs/pull/712/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldF9kZXJpdmVfdGVzdC9zcmMvbGliLnJz) | `100.00% <100.00%> (ø)` | |
| [arrow/src/array/equal\_json.rs](https://codecov.io/gh/apache/arrow-rs/pull/712/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2VxdWFsX2pzb24ucnM=) | `84.92% <0.00%> (-0.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/712?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/712?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8308615...7568e71](https://codecov.io/gh/apache/arrow-rs/pull/712?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org