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/09/15 03:29:02 UTC
[GitHub] [arrow-rs] b41sh opened a new pull request #779: Add MONTH_DAY_NANO interval type
b41sh opened a new pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779
# Which issue does this PR close?
<!---
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #732
# Rationale for this change
<!---
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
<!---
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
Add MONTH_DAY_NANO interval type
# Are there any user-facing changes?
<!---
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
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] b41sh commented on a change in pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r711573906
##########
File path: parquet/src/arrow/converter.rs
##########
@@ -167,6 +168,46 @@ impl Converter<Vec<Option<FixedLenByteArray>>, IntervalDayTimeArray>
}
}
+/// An Arrow Interval converter, which reads the last 16 bytes of a Parquet interval,
+/// and interprets it as an i128 value representing the Arrow MonthDayNano value
+pub struct IntervalMonthDayNanoArrayConverter {}
+
+impl IntervalMonthDayNanoArrayConverter {
+ fn from_bytes_to_i128(b: &[u8]) -> i128 {
+ assert!(
+ b.len() <= 16,
+ "IntervalMonthDayNanoArray supports only up to size 16"
+ );
+ let first_bit = b[0] & 128u8 == 128u8;
+ let mut result = if first_bit { [255u8; 16] } else { [0u8; 16] };
+ for (i, v) in b.iter().enumerate() {
+ result[i + (16 - b.len())] = *v;
+ }
+ i128::from_be_bytes(result)
+ }
+}
+
+impl Converter<Vec<Option<FixedLenByteArray>>, IntervalMonthDayNanoArray>
+ for IntervalMonthDayNanoArrayConverter
+{
+ fn convert(
+ &self,
+ source: Vec<Option<FixedLenByteArray>>,
+ ) -> Result<IntervalMonthDayNanoArray> {
+ let mut builder = IntervalMonthDayNanoBuilder::new(source.len());
+ for v in source {
+ match v {
+ Some(array) => {
+ builder.append_value(Self::from_bytes_to_i128(array.data()))
+ }
+ None => builder.append_null(),
+ }?
+ }
+
+ Ok(builder.finish())
+ }
+}
+
Review comment:
`IntervalMonthDayNanoArray` can't converted to parquet format, as `Interval` in parquet is fixed 12 bytes, but `IntervalMonthDayNano` is 16 bytes. I have deleted parquet related codes.
--
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] b41sh commented on a change in pull request #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r763739631
##########
File path: arrow/src/compute/kernels/sort.rs
##########
@@ -243,6 +243,11 @@ pub fn sort_to_indices(
DataType::Interval(IntervalUnit::DayTime) => {
sort_primitive::<IntervalDayTimeType, _>(values, v, n, cmp, &options, limit)
}
+ DataType::Interval(IntervalUnit::MonthDayNano) => {
+ sort_primitive::<IntervalMonthDayNanoType, _>(
Review comment:
Can I write a function like `sort_month_day_nanos` 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] codecov-commenter edited a comment on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-919675813
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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 [#779](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (adc36f1) into [master](https://codecov.io/gh/apache/arrow-rs/commit/72c9d1ea2bcd3a10b37671ddc87141f07b9388a4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (72c9d1e) will **decrease** coverage by `0.07%`.
> The diff coverage is `44.32%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/779?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 #779 +/- ##
==========================================
- Coverage 82.31% 82.23% -0.08%
==========================================
Files 168 168
Lines 49031 49120 +89
==========================================
+ Hits 40360 40394 +34
- Misses 8671 8726 +55
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIucnM=) | `86.54% <0.00%> (-0.11%)` | :arrow_down: |
| [arrow/src/array/data.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2RhdGEucnM=) | `80.85% <0.00%> (-0.32%)` | :arrow_down: |
| [arrow/src/array/equal/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2VxdWFsL21vZC5ycw==) | `93.13% <0.00%> (-0.17%)` | :arrow_down: |
| [arrow/src/array/ord.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L29yZC5ycw==) | `67.15% <0.00%> (-0.50%)` | :arrow_down: |
| [arrow/src/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `84.73% <0.00%> (-0.51%)` | :arrow_down: |
| [arrow/src/compute/kernels/sort.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9zb3J0LnJz) | `94.81% <0.00%> (-0.14%)` | :arrow_down: |
| [arrow/src/datatypes/types.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2RhdGF0eXBlcy90eXBlcy5ycw==) | `88.88% <ø> (ø)` | |
| [arrow/src/ipc/gen/Schema.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9nZW4vU2NoZW1hLnJz) | `41.60% <ø> (ø)` | |
| [arrow/src/ipc/reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9yZWFkZXIucnM=) | `85.74% <ø> (ø)` | |
| [arrow/src/util/display.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL3V0aWwvZGlzcGxheS5ycw==) | `19.81% <0.00%> (-0.19%)` | :arrow_down: |
| ... and [14 more](https://codecov.io/gh/apache/arrow-rs/pull/779/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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/779?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 [72c9d1e...adc36f1](https://codecov.io/gh/apache/arrow-rs/pull/779?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
[GitHub] [arrow-rs] b41sh commented on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
b41sh commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-928017662
> A trick I use in arrow2 for this is to have [a git patch](https://github.com/jorgecarleitao/arrow2/blob/main/integration-testing/unskip.patch) and apply it to arrow during execution [here](https://github.com/jorgecarleitao/arrow2/blob/main/.github/workflows/integration-ipc.yml#L30). An alternative is to create a PR in apache/arrow and point to that PR on our CI [here](https://github.com/apache/arrow-rs/blob/master/.github/workflows/integration.yml#L32). Once tests pass here, merge this PR without that change here and afterwards the PR in apache/arrow.
! [remote rejected] interval-MonthDayNano -> interval-MonthDayNano (refusing to allow a Personal Access Token to create or update workflow `.github/workflows/integration.yml` without `workflow` scope)
I don't have permission to modify the files in the workflow directory, can you help me start the integration test? @houqp @jorgecarleitao
--
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] ovr commented on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
ovr commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-984373354
Any news on 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 #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-991052871
The rust compiler thing is fixed in https://github.com/apache/arrow-rs/pull/1023 -- I'll try and merge to this PR
--
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 edited a comment on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-919675813
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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 [#779](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (74abfcb) into [master](https://codecov.io/gh/apache/arrow-rs/commit/72c9d1ea2bcd3a10b37671ddc87141f07b9388a4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (72c9d1e) will **decrease** coverage by `0.07%`.
> The diff coverage is `45.74%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/779?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 #779 +/- ##
==========================================
- Coverage 82.31% 82.24% -0.08%
==========================================
Files 168 168
Lines 49031 49117 +86
==========================================
+ Hits 40360 40394 +34
- Misses 8671 8723 +52
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIucnM=) | `86.54% <0.00%> (-0.11%)` | :arrow_down: |
| [arrow/src/array/data.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2RhdGEucnM=) | `80.85% <0.00%> (-0.32%)` | :arrow_down: |
| [arrow/src/array/equal/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2VxdWFsL21vZC5ycw==) | `93.13% <0.00%> (-0.17%)` | :arrow_down: |
| [arrow/src/array/ord.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L29yZC5ycw==) | `67.15% <0.00%> (-0.50%)` | :arrow_down: |
| [arrow/src/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `85.00% <0.00%> (-0.24%)` | :arrow_down: |
| [arrow/src/compute/kernels/sort.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9zb3J0LnJz) | `94.81% <0.00%> (-0.14%)` | :arrow_down: |
| [arrow/src/datatypes/types.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2RhdGF0eXBlcy90eXBlcy5ycw==) | `88.88% <ø> (ø)` | |
| [arrow/src/ipc/gen/Schema.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9nZW4vU2NoZW1hLnJz) | `41.60% <ø> (ø)` | |
| [arrow/src/ipc/reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9yZWFkZXIucnM=) | `85.74% <ø> (ø)` | |
| [arrow/src/util/display.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL3V0aWwvZGlzcGxheS5ycw==) | `19.81% <0.00%> (-0.19%)` | :arrow_down: |
| ... and [14 more](https://codecov.io/gh/apache/arrow-rs/pull/779/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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/779?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 [72c9d1e...74abfcb](https://codecov.io/gh/apache/arrow-rs/pull/779?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
[GitHub] [arrow-rs] codecov-commenter edited a comment on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-919675813
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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 [#779](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2ad60f1) into [master](https://codecov.io/gh/apache/arrow-rs/commit/72c9d1ea2bcd3a10b37671ddc87141f07b9388a4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (72c9d1e) will **decrease** coverage by `0.03%`.
> The diff coverage is `61.42%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/779?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 #779 +/- ##
==========================================
- Coverage 82.31% 82.28% -0.04%
==========================================
Files 168 168
Lines 49031 49093 +62
==========================================
+ Hits 40360 40395 +35
- Misses 8671 8698 +27
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIucnM=) | `86.54% <0.00%> (-0.11%)` | :arrow_down: |
| [arrow/src/array/data.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2RhdGEucnM=) | `80.85% <0.00%> (-0.32%)` | :arrow_down: |
| [arrow/src/array/equal/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2VxdWFsL21vZC5ycw==) | `93.13% <0.00%> (-0.17%)` | :arrow_down: |
| [arrow/src/array/ord.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L29yZC5ycw==) | `67.15% <0.00%> (-0.50%)` | :arrow_down: |
| [arrow/src/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `84.86% <0.00%> (-0.38%)` | :arrow_down: |
| [arrow/src/compute/kernels/sort.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9zb3J0LnJz) | `94.81% <0.00%> (-0.14%)` | :arrow_down: |
| [arrow/src/datatypes/types.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2RhdGF0eXBlcy90eXBlcy5ycw==) | `88.88% <ø> (ø)` | |
| [arrow/src/ipc/gen/Schema.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9nZW4vU2NoZW1hLnJz) | `41.60% <ø> (ø)` | |
| [arrow/src/ipc/reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9yZWFkZXIucnM=) | `85.74% <ø> (ø)` | |
| [arrow/src/util/display.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL3V0aWwvZGlzcGxheS5ycw==) | `19.81% <0.00%> (-0.19%)` | :arrow_down: |
| ... and [13 more](https://codecov.io/gh/apache/arrow-rs/pull/779/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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/779?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 [72c9d1e...2ad60f1](https://codecov.io/gh/apache/arrow-rs/pull/779?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
[GitHub] [arrow-rs] codecov-commenter commented on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-919675813
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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 [#779](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7e14a24) into [master](https://codecov.io/gh/apache/arrow-rs/commit/5c3ed6123d9ea0130a1eca95a0aae776b458208f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5c3ed61) will **decrease** coverage by `0.12%`.
> The diff coverage is `38.46%`.
> :exclamation: Current head 7e14a24 differs from pull request most recent head 8c7e4e5. Consider uploading reports for the commit 8c7e4e5 to get more accurate results
[](https://codecov.io/gh/apache/arrow-rs/pull/779?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 #779 +/- ##
==========================================
- Coverage 82.54% 82.42% -0.13%
==========================================
Files 168 168
Lines 47912 48026 +114
==========================================
+ Hits 39551 39585 +34
- Misses 8361 8441 +80
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/array/array.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2FycmF5LnJz) | `83.13% <0.00%> (-0.50%)` | :arrow_down: |
| [arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIucnM=) | `86.09% <0.00%> (-0.11%)` | :arrow_down: |
| [arrow/src/array/data.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2RhdGEucnM=) | `73.12% <0.00%> (-0.64%)` | :arrow_down: |
| [arrow/src/array/equal/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2VxdWFsL21vZC5ycw==) | `93.29% <0.00%> (-0.17%)` | :arrow_down: |
| [arrow/src/array/ord.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L29yZC5ycw==) | `62.50% <0.00%> (-0.84%)` | :arrow_down: |
| [arrow/src/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `85.69% <0.00%> (-0.24%)` | :arrow_down: |
| [arrow/src/compute/kernels/sort.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9zb3J0LnJz) | `94.52% <0.00%> (-0.14%)` | :arrow_down: |
| [arrow/src/datatypes/types.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2RhdGF0eXBlcy90eXBlcy5ycw==) | `88.88% <ø> (ø)` | |
| [arrow/src/ipc/gen/Schema.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9nZW4vU2NoZW1hLnJz) | `42.54% <ø> (ø)` | |
| [arrow/src/ipc/reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9yZWFkZXIucnM=) | `84.69% <ø> (ø)` | |
| ... and [14 more](https://codecov.io/gh/apache/arrow-rs/pull/779/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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/779?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 [5c3ed61...8c7e4e5](https://codecov.io/gh/apache/arrow-rs/pull/779?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
[GitHub] [arrow-rs] alamb commented on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-985647875
I don't have any news
--
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] jorgecarleitao commented on a change in pull request #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r764597928
##########
File path: arrow/src/compute/kernels/sort.rs
##########
@@ -243,6 +243,11 @@ pub fn sort_to_indices(
DataType::Interval(IntervalUnit::DayTime) => {
sort_primitive::<IntervalDayTimeType, _>(values, v, n, cmp, &options, limit)
}
+ DataType::Interval(IntervalUnit::MonthDayNano) => {
+ sort_primitive::<IntervalMonthDayNanoType, _>(
Review comment:
I think that in general the time intervals do not have an natural order without an associated datetime pinning them to a specific time line. The conversion `month,days,nanos -> seconds` is lossy because one day is not 24 hours (some days are 23 and others 25).
--
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 edited a comment on pull request #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-919675813
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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 [#779](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (841321a) into [master](https://codecov.io/gh/apache/arrow-rs/commit/72c9d1ea2bcd3a10b37671ddc87141f07b9388a4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (72c9d1e) will **decrease** coverage by `0.07%`.
> The diff coverage is `46.23%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/779?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 #779 +/- ##
==========================================
- Coverage 82.31% 82.23% -0.08%
==========================================
Files 168 168
Lines 49031 49116 +85
==========================================
+ Hits 40360 40392 +32
- Misses 8671 8724 +53
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIucnM=) | `86.54% <0.00%> (-0.11%)` | :arrow_down: |
| [arrow/src/array/data.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2RhdGEucnM=) | `80.85% <0.00%> (-0.32%)` | :arrow_down: |
| [arrow/src/array/equal/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2VxdWFsL21vZC5ycw==) | `93.13% <0.00%> (-0.17%)` | :arrow_down: |
| [arrow/src/array/ord.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L29yZC5ycw==) | `67.15% <0.00%> (-0.50%)` | :arrow_down: |
| [arrow/src/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `84.86% <0.00%> (-0.38%)` | :arrow_down: |
| [arrow/src/compute/kernels/sort.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9zb3J0LnJz) | `94.81% <0.00%> (-0.14%)` | :arrow_down: |
| [arrow/src/datatypes/types.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2RhdGF0eXBlcy90eXBlcy5ycw==) | `88.88% <ø> (ø)` | |
| [arrow/src/ipc/gen/Schema.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9nZW4vU2NoZW1hLnJz) | `41.60% <ø> (ø)` | |
| [arrow/src/ipc/reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9yZWFkZXIucnM=) | `85.74% <ø> (ø)` | |
| [arrow/src/util/display.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL3V0aWwvZGlzcGxheS5ycw==) | `19.81% <0.00%> (-0.19%)` | :arrow_down: |
| ... and [16 more](https://codecov.io/gh/apache/arrow-rs/pull/779/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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/779?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 [72c9d1e...841321a](https://codecov.io/gh/apache/arrow-rs/pull/779?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
[GitHub] [arrow-rs] jorgecarleitao commented on a change in pull request #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r764594354
##########
File path: arrow/src/datatypes/numeric.rs
##########
@@ -348,6 +348,7 @@ make_numeric_type!(Time64MicrosecondType, i64, i64x8, m64x8);
make_numeric_type!(Time64NanosecondType, i64, i64x8, m64x8);
make_numeric_type!(IntervalYearMonthType, i32, i32x16, m32x16);
make_numeric_type!(IntervalDayTimeType, i64, i64x8, m64x8);
+make_numeric_type!(IntervalMonthDayNanoType, i128, i128x4, m128x4);
Review comment:
Yeap, that seems a bug to me.
--
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 edited a comment on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-919675813
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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 [#779](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4e19eb8) into [master](https://codecov.io/gh/apache/arrow-rs/commit/72c9d1ea2bcd3a10b37671ddc87141f07b9388a4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (72c9d1e) will **decrease** coverage by `0.03%`.
> The diff coverage is `61.42%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/779?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 #779 +/- ##
==========================================
- Coverage 82.31% 82.27% -0.04%
==========================================
Files 168 168
Lines 49031 49093 +62
==========================================
+ Hits 40360 40393 +33
- Misses 8671 8700 +29
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIucnM=) | `86.54% <0.00%> (-0.11%)` | :arrow_down: |
| [arrow/src/array/data.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2RhdGEucnM=) | `80.85% <0.00%> (-0.32%)` | :arrow_down: |
| [arrow/src/array/equal/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2VxdWFsL21vZC5ycw==) | `93.13% <0.00%> (-0.17%)` | :arrow_down: |
| [arrow/src/array/ord.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L29yZC5ycw==) | `67.15% <0.00%> (-0.50%)` | :arrow_down: |
| [arrow/src/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `84.86% <0.00%> (-0.38%)` | :arrow_down: |
| [arrow/src/compute/kernels/sort.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9zb3J0LnJz) | `94.81% <0.00%> (-0.14%)` | :arrow_down: |
| [arrow/src/datatypes/types.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2RhdGF0eXBlcy90eXBlcy5ycw==) | `88.88% <ø> (ø)` | |
| [arrow/src/ipc/gen/Schema.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9nZW4vU2NoZW1hLnJz) | `41.60% <ø> (ø)` | |
| [arrow/src/ipc/reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9yZWFkZXIucnM=) | `85.74% <ø> (ø)` | |
| [arrow/src/util/display.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL3V0aWwvZGlzcGxheS5ycw==) | `19.81% <0.00%> (-0.19%)` | :arrow_down: |
| ... and [12 more](https://codecov.io/gh/apache/arrow-rs/pull/779/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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/779?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 [72c9d1e...4e19eb8](https://codecov.io/gh/apache/arrow-rs/pull/779?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
[GitHub] [arrow-rs] b41sh commented on a change in pull request #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r763576726
##########
File path: integration-testing/src/lib.rs
##########
@@ -280,6 +280,56 @@ fn array_from_json(
}
Ok(Arc::new(b.finish()))
}
+ DataType::Interval(IntervalUnit::MonthDayNano) => {
+ let mut b = IntervalMonthDayNanoBuilder::new(json_col.count);
+ for (is_valid, value) in json_col
+ .validity
+ .as_ref()
+ .unwrap()
+ .iter()
+ .zip(json_col.data.unwrap())
+ {
+ match is_valid {
+ 1 => b.append_value(match value {
+ Value::Object(v) => {
+ let months = v.get("months").unwrap();
+ let days = v.get("days").unwrap();
+ let nanoseconds = v.get("nanoseconds").unwrap();
+ println!(
+ "months={:?} days={:?} nanos={:?}",
+ months, days, nanoseconds
+ );
+ match (months, days, nanoseconds) {
+ (
+ Value::Number(months),
+ Value::Number(days),
+ Value::Number(nanoseconds),
+ ) => {
+ let months = months.as_i64().unwrap() as i32;
+ let days = days.as_i64().unwrap() as i32;
+ let nanoseconds = nanoseconds.as_i64().unwrap();
+ let months_days_ns: i128 = ((nanoseconds as i128)
+ & 0xFFFFFFFFFFFFFFFF)
+ << 64
+ | ((days as i128) & 0xFFFFFFFF) << 32
+ | ((months as i128) & 0xFFFFFFFF);
+ println!("months_days_ns={:?}", months_days_ns);
+ months_days_ns
+ }
+ (_, _, _) => {
+ panic!("Unable to parse {:?} as MonthDayNano", v)
+ }
+ }
+ }
+ _ => panic!("Unable to parse {:?} as MonthDayNano", value),
+ }),
+ _ => b.append_null(),
+ }?;
+ }
+ let array = Arc::new(b.finish()) as ArrayRef;
+ arrow::compute::cast(&array, field.data_type())
Review comment:
This is not needed, I have deleted 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 a change in pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r713233616
##########
File path: arrow/src/array/array_primitive.rs
##########
@@ -624,6 +625,22 @@ mod tests {
assert!(arr.is_null(1));
assert_eq!(-5, arr.value(2));
assert_eq!(-5, arr.values()[2]);
+
+ // a month_day_nano interval contains months, days and nanoseconds,
+ // but we do not yet have accessors for the values
Review comment:
Is it worth adding some ticket explaining what type of accessors would be valuable? Namely `month`, `day` and `nanos`?
##########
File path: arrow/Cargo.toml
##########
@@ -38,7 +38,7 @@ path = "src/lib.rs"
[dependencies]
serde = { version = "1.0", features = ["rc"] }
serde_derive = "1.0"
-serde_json = { version = "1.0", features = ["preserve_order"] }
+serde_json = { version = "1.0", features = ["preserve_order", "arbitrary_precision"] }
Review comment:
There is often much concern about adding new dependencies to arrow - however, this feature does not seem to add any new dependencies: https://github.com/serde-rs/json/blob/master/Cargo.toml#L74
##########
File path: arrow/src/util/display.rs
##########
@@ -106,6 +106,45 @@ macro_rules! make_string_interval_day_time {
}};
}
+macro_rules! make_string_interval_month_day_nano {
+ ($column: ident, $row: ident) => {{
+ let array = $column
+ .as_any()
+ .downcast_ref::<array::IntervalMonthDayNanoArray>()
+ .unwrap();
+
+ let s = if array.is_null($row) {
+ "NULL".to_string()
+ } else {
+ let value: u128 = array.value($row) as u128;
+
+ let months_part: i32 =
Review comment:
it would be nice to break this logic out into accessors, as you say, rather than have the field extraction in the stringification. But that could be done as a follow on PR I think
##########
File path: arrow/src/datatypes/datatype.rs
##########
@@ -158,14 +158,22 @@ pub enum TimeUnit {
Nanosecond,
}
-/// YEAR_MONTH or DAY_TIME interval in SQL style.
+/// YEAR_MONTH, DAY_TIME, MonthDayNano interval in SQL style.
Review comment:
```suggestion
/// YEAR_MONTH, DAY_TIME, MONTH_DAY_NANO interval in SQL style.
```
##########
File path: arrow/src/datatypes/native.rs
##########
@@ -293,6 +328,29 @@ impl ArrowNativeType for u64 {
}
}
+impl JsonSerializable for u128 {
Review comment:
What uses `u128`? It seems like this PR only uses `i128`. I may be missing something
##########
File path: parquet/src/arrow/arrow_writer.rs
##########
@@ -426,6 +426,14 @@ fn write_leaf(
.unwrap();
get_interval_dt_array_slice(array, &indices)
}
+ _ => {
+ return Err(ParquetError::NYI(
+ format!(
+ "Attempting to write an Arrow type {:?} to parquet that is not yet implemented",
Review comment:
```suggestion
"Attempting to write an Arrow interval type {:?} to parquet that is not yet implemented",
```
--
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 #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r763469634
##########
File path: arrow/src/array/array_primitive.rs
##########
@@ -649,6 +650,23 @@ mod tests {
assert!(arr.is_null(1));
assert_eq!(-5, arr.value(2));
assert_eq!(-5, arr.values()[2]);
+
+ // a month_day_nano interval contains months, days and nanoseconds,
+ // but we do not yet have accessors for the values.
+ // TODO: implement month, day, and nanos access method for month_day_nano.
Review comment:
👍 When this PR is merged, I will try and file a ticket for adding these accessors -- I think it would be a fairly good "first PR" type change for new contributors
##########
File path: integration-testing/unskip.patch
##########
@@ -0,0 +1,32 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+diff --git a/dev/archery/archery/integration/datagen.py b/dev/archery/archery/integration/datagen.py
+index 6a077a893..104c49c52 100644
+--- a/dev/archery/archery/integration/datagen.py
++++ b/dev/archery/archery/integration/datagen.py
+@@ -1579,8 +1579,7 @@ def get_generated_json_files(tempdir=None):
+
+ generate_month_day_nano_interval_case()
+ .skip_category('C#')
+- .skip_category('JS')
+- .skip_category('Rust'),
++ .skip_category('JS'),
Review comment:
🎉
so is the intent that after we merge this PR into `arrow-rs` we would then go upstream and apply this patch in the arrow repo?
##########
File path: integration-testing/src/lib.rs
##########
@@ -280,6 +280,56 @@ fn array_from_json(
}
Ok(Arc::new(b.finish()))
}
+ DataType::Interval(IntervalUnit::MonthDayNano) => {
+ let mut b = IntervalMonthDayNanoBuilder::new(json_col.count);
+ for (is_valid, value) in json_col
+ .validity
+ .as_ref()
+ .unwrap()
+ .iter()
+ .zip(json_col.data.unwrap())
+ {
+ match is_valid {
+ 1 => b.append_value(match value {
+ Value::Object(v) => {
+ let months = v.get("months").unwrap();
+ let days = v.get("days").unwrap();
+ let nanoseconds = v.get("nanoseconds").unwrap();
+ println!(
+ "months={:?} days={:?} nanos={:?}",
+ months, days, nanoseconds
+ );
+ match (months, days, nanoseconds) {
+ (
+ Value::Number(months),
+ Value::Number(days),
+ Value::Number(nanoseconds),
+ ) => {
+ let months = months.as_i64().unwrap() as i32;
+ let days = days.as_i64().unwrap() as i32;
+ let nanoseconds = nanoseconds.as_i64().unwrap();
+ let months_days_ns: i128 = ((nanoseconds as i128)
+ & 0xFFFFFFFFFFFFFFFF)
+ << 64
+ | ((days as i128) & 0xFFFFFFFF) << 32
+ | ((months as i128) & 0xFFFFFFFF);
+ println!("months_days_ns={:?}", months_days_ns);
+ months_days_ns
+ }
+ (_, _, _) => {
+ panic!("Unable to parse {:?} as MonthDayNano", v)
+ }
+ }
+ }
+ _ => panic!("Unable to parse {:?} as MonthDayNano", value),
+ }),
+ _ => b.append_null(),
+ }?;
+ }
+ let array = Arc::new(b.finish()) as ArrayRef;
+ arrow::compute::cast(&array, field.data_type())
Review comment:
I am curious why the cast is needed
--
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] jorgecarleitao commented on a change in pull request #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r763640120
##########
File path: arrow/src/compute/kernels/sort.rs
##########
@@ -243,6 +243,11 @@ pub fn sort_to_indices(
DataType::Interval(IntervalUnit::DayTime) => {
sort_primitive::<IntervalDayTimeType, _>(values, v, n, cmp, &options, limit)
}
+ DataType::Interval(IntervalUnit::MonthDayNano) => {
+ sort_primitive::<IntervalMonthDayNanoType, _>(
Review comment:
The i128 order relationship does not hold for `months,days,nanos`. AFAIK `month,days,nanos` do not have a partial order relationship.
##########
File path: arrow/src/datatypes/numeric.rs
##########
@@ -348,6 +348,7 @@ make_numeric_type!(Time64MicrosecondType, i64, i64x8, m64x8);
make_numeric_type!(Time64NanosecondType, i64, i64x8, m64x8);
make_numeric_type!(IntervalYearMonthType, i32, i32x16, m32x16);
make_numeric_type!(IntervalDayTimeType, i64, i64x8, m64x8);
+make_numeric_type!(IntervalMonthDayNanoType, i128, i128x4, m128x4);
Review comment:
Semantically, the numerics of an `i128` are not the same as the numerics of `(months,days,nanos)` since `i128 + i128 != (months, days,nanos) + (months, days,nanos)`.
The consequence of defining this type numerically here is that arithmetic kernels will accept this type, but they will yield a semantically incorrect result (e.g. `i128 + i128` to sum two intervals of 1 month each).
--
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 #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-997997142
Follow on tickets: https://github.com/apache/arrow-rs/issues/1065 and https://github.com/apache/arrow-rs/issues/1066 and https://github.com/apache/arrow-rs/issues/1067
--
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 #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
alamb merged pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779
--
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 #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-925244855
I think it looks good to go. @jorgecarleitao / @nevi-me / @houqp do you have any interest in reviewing this PR?
--
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] b41sh commented on a change in pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r711573957
##########
File path: parquet/src/arrow/converter.rs
##########
@@ -167,6 +168,46 @@ impl Converter<Vec<Option<FixedLenByteArray>>, IntervalDayTimeArray>
}
}
+/// An Arrow Interval converter, which reads the last 16 bytes of a Parquet interval,
+/// and interprets it as an i128 value representing the Arrow MonthDayNano value
+pub struct IntervalMonthDayNanoArrayConverter {}
+
+impl IntervalMonthDayNanoArrayConverter {
+ fn from_bytes_to_i128(b: &[u8]) -> i128 {
+ assert!(
+ b.len() <= 16,
+ "IntervalMonthDayNanoArray supports only up to size 16"
+ );
+ let first_bit = b[0] & 128u8 == 128u8;
+ let mut result = if first_bit { [255u8; 16] } else { [0u8; 16] };
+ for (i, v) in b.iter().enumerate() {
+ result[i + (16 - b.len())] = *v;
+ }
+ i128::from_be_bytes(result)
+ }
+}
+
+impl Converter<Vec<Option<FixedLenByteArray>>, IntervalMonthDayNanoArray>
+ for IntervalMonthDayNanoArrayConverter
+{
+ fn convert(
+ &self,
+ source: Vec<Option<FixedLenByteArray>>,
+ ) -> Result<IntervalMonthDayNanoArray> {
+ let mut builder = IntervalMonthDayNanoBuilder::new(source.len());
+ for v in source {
+ match v {
+ Some(array) => {
+ builder.append_value(Self::from_bytes_to_i128(array.data()))
+ }
+ None => builder.append_null(),
+ }?
+ }
+
+ Ok(builder.finish())
+ }
+}
+
Review comment:
`IntervalMonthDayNanoArray` can't converted to parquet format, as `Interval` in parquet is fixed 12 bytes, but `IntervalMonthDayNano` is 16 bytes. I have deleted parquet related codes.
--
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] b41sh commented on a change in pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r714168370
##########
File path: arrow/src/datatypes/native.rs
##########
@@ -293,6 +328,29 @@ impl ArrowNativeType for u64 {
}
}
+impl JsonSerializable for u128 {
Review comment:
`u128` is useless, I have deleted 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] jorgecarleitao commented on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-985702794
uhm, I can't understand the error message: you are pushing to your own fork of the repo, so you have full control over its CI (it is under your name, right?). Could it be that you are pushing via https and using a personal token that does not have the workflow scope active (on github)?
--
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 edited a comment on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-919675813
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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 [#779](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cb5a237) into [master](https://codecov.io/gh/apache/arrow-rs/commit/72c9d1ea2bcd3a10b37671ddc87141f07b9388a4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (72c9d1e) will **decrease** coverage by `0.03%`.
> The diff coverage is `61.42%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/779?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 #779 +/- ##
==========================================
- Coverage 82.31% 82.28% -0.04%
==========================================
Files 168 168
Lines 49031 49093 +62
==========================================
+ Hits 40360 40394 +34
- Misses 8671 8699 +28
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIucnM=) | `86.54% <0.00%> (-0.11%)` | :arrow_down: |
| [arrow/src/array/data.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2RhdGEucnM=) | `80.85% <0.00%> (-0.32%)` | :arrow_down: |
| [arrow/src/array/equal/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2VxdWFsL21vZC5ycw==) | `93.13% <0.00%> (-0.17%)` | :arrow_down: |
| [arrow/src/array/ord.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L29yZC5ycw==) | `67.15% <0.00%> (-0.50%)` | :arrow_down: |
| [arrow/src/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `85.00% <0.00%> (-0.24%)` | :arrow_down: |
| [arrow/src/compute/kernels/sort.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9zb3J0LnJz) | `94.81% <0.00%> (-0.14%)` | :arrow_down: |
| [arrow/src/datatypes/types.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2RhdGF0eXBlcy90eXBlcy5ycw==) | `88.88% <ø> (ø)` | |
| [arrow/src/ipc/gen/Schema.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9nZW4vU2NoZW1hLnJz) | `41.60% <ø> (ø)` | |
| [arrow/src/ipc/reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9yZWFkZXIucnM=) | `85.74% <ø> (ø)` | |
| [arrow/src/util/display.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL3V0aWwvZGlzcGxheS5ycw==) | `19.81% <0.00%> (-0.19%)` | :arrow_down: |
| ... and [13 more](https://codecov.io/gh/apache/arrow-rs/pull/779/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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/779?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 [72c9d1e...cb5a237](https://codecov.io/gh/apache/arrow-rs/pull/779?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
[GitHub] [arrow-rs] codecov-commenter edited a comment on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-919675813
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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 [#779](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b2065ad) into [master](https://codecov.io/gh/apache/arrow-rs/commit/72c9d1ea2bcd3a10b37671ddc87141f07b9388a4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (72c9d1e) will **decrease** coverage by `0.06%`.
> The diff coverage is `47.77%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/779?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 #779 +/- ##
==========================================
- Coverage 82.31% 82.24% -0.07%
==========================================
Files 168 168
Lines 49031 49113 +82
==========================================
+ Hits 40360 40395 +35
- Misses 8671 8718 +47
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIucnM=) | `86.54% <0.00%> (-0.11%)` | :arrow_down: |
| [arrow/src/array/data.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2RhdGEucnM=) | `80.85% <0.00%> (-0.32%)` | :arrow_down: |
| [arrow/src/array/equal/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2VxdWFsL21vZC5ycw==) | `93.13% <0.00%> (-0.17%)` | :arrow_down: |
| [arrow/src/array/ord.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L29yZC5ycw==) | `67.15% <0.00%> (-0.50%)` | :arrow_down: |
| [arrow/src/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `84.86% <0.00%> (-0.38%)` | :arrow_down: |
| [arrow/src/compute/kernels/sort.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9zb3J0LnJz) | `94.81% <0.00%> (-0.14%)` | :arrow_down: |
| [arrow/src/datatypes/types.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2RhdGF0eXBlcy90eXBlcy5ycw==) | `88.88% <ø> (ø)` | |
| [arrow/src/ipc/gen/Schema.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9nZW4vU2NoZW1hLnJz) | `41.60% <ø> (ø)` | |
| [arrow/src/ipc/reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9yZWFkZXIucnM=) | `85.74% <ø> (ø)` | |
| [arrow/src/util/display.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL3V0aWwvZGlzcGxheS5ycw==) | `19.81% <0.00%> (-0.19%)` | :arrow_down: |
| ... and [14 more](https://codecov.io/gh/apache/arrow-rs/pull/779/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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/779?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 [72c9d1e...b2065ad](https://codecov.io/gh/apache/arrow-rs/pull/779?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
[GitHub] [arrow-rs] alamb commented on pull request #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-987392633
@houqp and @jorgecarleitao are you interested in reviewing / re-reviewing this PR?
--
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 edited a comment on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-919675813
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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 [#779](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4791a97) into [master](https://codecov.io/gh/apache/arrow-rs/commit/5c3ed6123d9ea0130a1eca95a0aae776b458208f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5c3ed61) will **decrease** coverage by `0.04%`.
> The diff coverage is `58.22%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/779?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 #779 +/- ##
==========================================
- Coverage 82.54% 82.50% -0.05%
==========================================
Files 168 168
Lines 47912 47983 +71
==========================================
+ Hits 39551 39587 +36
- Misses 8361 8396 +35
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIucnM=) | `86.09% <0.00%> (-0.11%)` | :arrow_down: |
| [arrow/src/array/data.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2RhdGEucnM=) | `73.12% <0.00%> (-0.64%)` | :arrow_down: |
| [arrow/src/array/equal/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2VxdWFsL21vZC5ycw==) | `93.29% <0.00%> (-0.17%)` | :arrow_down: |
| [arrow/src/array/ord.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L29yZC5ycw==) | `62.50% <0.00%> (-0.84%)` | :arrow_down: |
| [arrow/src/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `85.69% <0.00%> (-0.24%)` | :arrow_down: |
| [arrow/src/compute/kernels/sort.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9zb3J0LnJz) | `94.52% <0.00%> (-0.14%)` | :arrow_down: |
| [arrow/src/datatypes/types.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2RhdGF0eXBlcy90eXBlcy5ycw==) | `88.88% <ø> (ø)` | |
| [arrow/src/ipc/gen/Schema.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9nZW4vU2NoZW1hLnJz) | `42.54% <ø> (ø)` | |
| [arrow/src/ipc/reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9yZWFkZXIucnM=) | `84.69% <ø> (ø)` | |
| [arrow/src/util/display.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL3V0aWwvZGlzcGxheS5ycw==) | `23.80% <0.00%> (-0.23%)` | :arrow_down: |
| ... and [11 more](https://codecov.io/gh/apache/arrow-rs/pull/779/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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/779?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 [5c3ed61...4791a97](https://codecov.io/gh/apache/arrow-rs/pull/779?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
[GitHub] [arrow-rs] alamb commented on a change in pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r710519807
##########
File path: parquet/src/arrow/converter.rs
##########
@@ -167,6 +168,46 @@ impl Converter<Vec<Option<FixedLenByteArray>>, IntervalDayTimeArray>
}
}
+/// An Arrow Interval converter, which reads the last 16 bytes of a Parquet interval,
+/// and interprets it as an i128 value representing the Arrow MonthDayNano value
+pub struct IntervalMonthDayNanoArrayConverter {}
+
+impl IntervalMonthDayNanoArrayConverter {
+ fn from_bytes_to_i128(b: &[u8]) -> i128 {
+ assert!(
+ b.len() <= 16,
+ "IntervalMonthDayNanoArray supports only up to size 16"
+ );
+ let first_bit = b[0] & 128u8 == 128u8;
+ let mut result = if first_bit { [255u8; 16] } else { [0u8; 16] };
+ for (i, v) in b.iter().enumerate() {
+ result[i + (16 - b.len())] = *v;
+ }
+ i128::from_be_bytes(result)
+ }
+}
+
+impl Converter<Vec<Option<FixedLenByteArray>>, IntervalMonthDayNanoArray>
+ for IntervalMonthDayNanoArrayConverter
+{
+ fn convert(
+ &self,
+ source: Vec<Option<FixedLenByteArray>>,
+ ) -> Result<IntervalMonthDayNanoArray> {
+ let mut builder = IntervalMonthDayNanoBuilder::new(source.len());
+ for v in source {
+ match v {
+ Some(array) => {
+ builder.append_value(Self::from_bytes_to_i128(array.data()))
+ }
+ None => builder.append_null(),
+ }?
+ }
+
+ Ok(builder.finish())
+ }
+}
+
Review comment:
I wonder if it would be possible to implement some tests for this logic (showing for example that round tripping from `IntervalMonthDayNanoArray` to `FixedLenByteArray` and back produces an equivalent array?
--
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] b41sh commented on pull request #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
b41sh commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-989733760
```
thread 'rustc' panicked at 'assertion failed: sentinel == STR_SENTINEL', /rustc/e6b883c74f49f32cb5d1cbad3457f2b8805a4a38/compiler/rustc_serialize/src/opaque.rs:669:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md
note: rustc 1.59.0-nightly (e6b883c74 2021-12-08) running on x86_64-unknown-linux-gnu
note: compiler flags: -C embed-bitcode=no -C debuginfo=2 -C debuginfo=1 --crate-type lib
note: some of the compiler flags provided by cargo are hidden
query stack during panic:
end of query stack
error: could not compile `flatbuffers`
warning: build failed, waiting for other jobs to finish...
error: build failed
Error: Process completed with exit code 101.
```
It looks like a rust bug https://github.com/rust-lang/rust/issues/91663
--
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] b41sh commented on a change in pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r714170942
##########
File path: arrow/src/array/array_primitive.rs
##########
@@ -624,6 +625,22 @@ mod tests {
assert!(arr.is_null(1));
assert_eq!(-5, arr.value(2));
assert_eq!(-5, arr.values()[2]);
+
+ // a month_day_nano interval contains months, days and nanoseconds,
+ // but we do not yet have accessors for the values
Review comment:
I added some TODO comments, PTAL
--
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] liukun4515 commented on pull request #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
liukun4515 commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-987533246
It's is great to implement i128 as ArrowNativeType.
@b41sh
--
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] b41sh commented on a change in pull request #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r763573345
##########
File path: integration-testing/unskip.patch
##########
@@ -0,0 +1,32 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+diff --git a/dev/archery/archery/integration/datagen.py b/dev/archery/archery/integration/datagen.py
+index 6a077a893..104c49c52 100644
+--- a/dev/archery/archery/integration/datagen.py
++++ b/dev/archery/archery/integration/datagen.py
+@@ -1579,8 +1579,7 @@ def get_generated_json_files(tempdir=None):
+
+ generate_month_day_nano_interval_case()
+ .skip_category('C#')
+- .skip_category('JS')
+- .skip_category('Rust'),
++ .skip_category('JS'),
Review comment:
yes, once this PR is merged, we can merge [#11238](https://github.com/apache/arrow/pull/11238) in the arrow repo.
--
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 #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-997984189
Thanks again @b41sh -- sorry for the delay in merging
--
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] b41sh commented on pull request #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
b41sh commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-989605395
> For this PR, I propose:
>
> 1. We file follow on tickets for the bugs @jorgecarleitao identified above (I can do so)
> 2. We merge this PR as is and work on those tickets as we have time.
>
> Thoughts?
Can I delete the `integration-testing/unskip.patch` file? As the test has passed, this file was originally temporary, we can directly use the arrow repo test later.
--
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] jorgecarleitao commented on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-925511541
A trick I use in arrow2 for this is to have [a git patch](https://github.com/jorgecarleitao/arrow2/blob/main/integration-testing/unskip.patch) and apply it to arrow during execution [here](https://github.com/jorgecarleitao/arrow2/blob/main/.github/workflows/integration-ipc.yml#L30). An alternative is to create a PR in apache/arrow and point to that PR on our CI [here](https://github.com/apache/arrow-rs/blob/master/.github/workflows/integration.yml#L32). Once tests pass here, merge this PR without that change here and afterwards the PR in apache/arrow.
--
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] b41sh commented on a change in pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r714164187
##########
File path: arrow/Cargo.toml
##########
@@ -38,7 +38,7 @@ path = "src/lib.rs"
[dependencies]
serde = { version = "1.0", features = ["rc"] }
serde_derive = "1.0"
-serde_json = { version = "1.0", features = ["preserve_order"] }
+serde_json = { version = "1.0", features = ["preserve_order", "arbitrary_precision"] }
Review comment:
We need this feature because we need to deserialize i128 numbers
https://github.com/serde-rs/json/blob/master/src/number.rs#L534
--
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] b41sh commented on a change in pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r711171079
##########
File path: parquet/src/arrow/converter.rs
##########
@@ -167,6 +168,46 @@ impl Converter<Vec<Option<FixedLenByteArray>>, IntervalDayTimeArray>
}
}
+/// An Arrow Interval converter, which reads the last 16 bytes of a Parquet interval,
+/// and interprets it as an i128 value representing the Arrow MonthDayNano value
+pub struct IntervalMonthDayNanoArrayConverter {}
+
+impl IntervalMonthDayNanoArrayConverter {
+ fn from_bytes_to_i128(b: &[u8]) -> i128 {
+ assert!(
+ b.len() <= 16,
+ "IntervalMonthDayNanoArray supports only up to size 16"
+ );
+ let first_bit = b[0] & 128u8 == 128u8;
+ let mut result = if first_bit { [255u8; 16] } else { [0u8; 16] };
+ for (i, v) in b.iter().enumerate() {
+ result[i + (16 - b.len())] = *v;
+ }
+ i128::from_be_bytes(result)
+ }
+}
+
+impl Converter<Vec<Option<FixedLenByteArray>>, IntervalMonthDayNanoArray>
+ for IntervalMonthDayNanoArrayConverter
+{
+ fn convert(
+ &self,
+ source: Vec<Option<FixedLenByteArray>>,
+ ) -> Result<IntervalMonthDayNanoArray> {
+ let mut builder = IntervalMonthDayNanoBuilder::new(source.len());
+ for v in source {
+ match v {
+ Some(array) => {
+ builder.append_value(Self::from_bytes_to_i128(array.data()))
+ }
+ None => builder.append_null(),
+ }?
+ }
+
+ Ok(builder.finish())
+ }
+}
+
Review comment:
ok, I will add some tests 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] ovr commented on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
ovr commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-985697336
> I don't have permission to modify the files in the workflow directory, can you help me start the integration test? @houqp @jorgecarleitao
Just a friendly ping. Can anyone help @b41sh to finish this PR?
--
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] b41sh commented on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
b41sh commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-986072084
Sorry for the long delay, the problem is caused by my github ssh settings and I have fixed 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] codecov-commenter edited a comment on pull request #779: Add MONTH_DAY_NANO interval type
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-919675813
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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 [#779](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a608be5) into [master](https://codecov.io/gh/apache/arrow-rs/commit/72c9d1ea2bcd3a10b37671ddc87141f07b9388a4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (72c9d1e) will **decrease** coverage by `0.03%`.
> The diff coverage is `61.42%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/779?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 #779 +/- ##
==========================================
- Coverage 82.31% 82.28% -0.04%
==========================================
Files 168 168
Lines 49031 49093 +62
==========================================
+ Hits 40360 40395 +35
- Misses 8671 8698 +27
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/779?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/array/builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIucnM=) | `86.54% <0.00%> (-0.11%)` | :arrow_down: |
| [arrow/src/array/data.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2RhdGEucnM=) | `80.85% <0.00%> (-0.32%)` | :arrow_down: |
| [arrow/src/array/equal/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L2VxdWFsL21vZC5ycw==) | `93.13% <0.00%> (-0.17%)` | :arrow_down: |
| [arrow/src/array/ord.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L29yZC5ycw==) | `67.15% <0.00%> (-0.50%)` | :arrow_down: |
| [arrow/src/array/transform/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9tb2QucnM=) | `84.86% <0.00%> (-0.38%)` | :arrow_down: |
| [arrow/src/compute/kernels/sort.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9zb3J0LnJz) | `94.81% <0.00%> (-0.14%)` | :arrow_down: |
| [arrow/src/datatypes/types.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2RhdGF0eXBlcy90eXBlcy5ycw==) | `88.88% <ø> (ø)` | |
| [arrow/src/ipc/gen/Schema.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9nZW4vU2NoZW1hLnJz) | `41.60% <ø> (ø)` | |
| [arrow/src/ipc/reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL2lwYy9yZWFkZXIucnM=) | `85.74% <ø> (ø)` | |
| [arrow/src/util/display.rs](https://codecov.io/gh/apache/arrow-rs/pull/779/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-YXJyb3cvc3JjL3V0aWwvZGlzcGxheS5ycw==) | `19.81% <0.00%> (-0.19%)` | :arrow_down: |
| ... and [14 more](https://codecov.io/gh/apache/arrow-rs/pull/779/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/779?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/779?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 [72c9d1e...a608be5](https://codecov.io/gh/apache/arrow-rs/pull/779?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
[GitHub] [arrow-rs] b41sh commented on a change in pull request #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r763752994
##########
File path: arrow/src/datatypes/numeric.rs
##########
@@ -348,6 +348,7 @@ make_numeric_type!(Time64MicrosecondType, i64, i64x8, m64x8);
make_numeric_type!(Time64NanosecondType, i64, i64x8, m64x8);
make_numeric_type!(IntervalYearMonthType, i32, i32x16, m32x16);
make_numeric_type!(IntervalDayTimeType, i64, i64x8, m64x8);
+make_numeric_type!(IntervalMonthDayNanoType, i128, i128x4, m128x4);
Review comment:
Do you mean we can't use `IntervalMonthDayNanoType` as a numeric type? But `IntervalDayTimeType` is also a numeric type here, the sum of two `IntervalDayTimeType` will also produce an incorrect result.
--
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 #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-991015331
Filed https://github.com/apache/arrow-rs/issues/1022 to track CI failure in "nightly" builds
--
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 #779: Add MONTH_DAY_NANO interval type, impl `ArrowNativeType` for `i128`
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#issuecomment-989129356
For this PR, I propose:
1. We file follow on tickets for the bugs @jorgecarleitao identified above (I can do so)
2. We merge this PR as is and work on those tickets as we have time.
Thoughts?
--
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