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 2022/04/12 13:29:12 UTC
[GitHub] [arrow-rs] HaoYang670 opened a new pull request, #1543: Mark unsafe and rename
HaoYang670 opened a new pull request, #1543:
URL: https://github.com/apache/arrow-rs/pull/1543
Signed-off-by: remzi <13...@gmail.com>
# 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 #1541.
# What changes are included in this PR?
1. Rename the current `substring` function as `substring_bytes_unchecked`.
2. Mark the function as `unsafe`
# Are there any user-facing changes?
Yes! API of `pub substring` function has been changed.
--
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] HaoYang670 closed pull request #1543: Do not merge! Rename the `substring` function and mark it as `unsafe`
Posted by GitBox <gi...@apache.org>.
HaoYang670 closed pull request #1543: Do not merge! Rename the `substring` function and mark it as `unsafe`
URL: https://github.com/apache/arrow-rs/pull/1543
--
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] HaoYang670 commented on pull request #1543: Do not merge! Rename the `substring` function and mark it as `unsafe`
Posted by GitBox <gi...@apache.org>.
HaoYang670 commented on PR #1543:
URL: https://github.com/apache/arrow-rs/pull/1543#issuecomment-1097689280
Strongly agree with you @alamb. But I have some different opintion with you on the renaming after considering more about the back compatibility. Please look at the comment https://github.com/apache/arrow-rs/issues/1531#issuecomment-1097613212
We'd better reach agreement on the renaming before doing the following tasks
--
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 #1543: Rename the `substring` function and mark it as `unsafe`
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #1543:
URL: https://github.com/apache/arrow-rs/pull/1543#issuecomment-1096866838
Thanks @HaoYang670 -- before we merge this, I would like to ensure we have an alternative `substring` implementation available that operates in terms of chars to minimize API churn
--
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] HaoYang670 commented on pull request #1543: Do not merge! Rename the `substring` function and mark it as `unsafe`
Posted by GitBox <gi...@apache.org>.
HaoYang670 commented on PR #1543:
URL: https://github.com/apache/arrow-rs/pull/1543#issuecomment-1100785218
Close this PR as we should add utf8 validation to the current function but not make it unsafe
--
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 #1543: Rename the `substring` function and mark it as `unsafe`
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1543:
URL: https://github.com/apache/arrow-rs/pull/1543#issuecomment-1096762032
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1543?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 [#1543](https://codecov.io/gh/apache/arrow-rs/pull/1543?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ff62c6b) into [master](https://codecov.io/gh/apache/arrow-rs/commit/08f386a1324c632230e108c44c55a6370397e1c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (08f386a) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head ff62c6b differs from pull request most recent head 3c600cb. Consider uploading reports for the commit 3c600cb to get more accurate results
```diff
@@ Coverage Diff @@
## master #1543 +/- ##
==========================================
- Coverage 82.77% 82.77% -0.01%
==========================================
Files 190 190
Lines 54827 54827
==========================================
- Hits 45385 45384 -1
- Misses 9442 9443 +1
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/1543?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/compute/kernels/substring.rs](https://codecov.io/gh/apache/arrow-rs/pull/1543/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-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9zdWJzdHJpbmcucnM=) | `98.31% <100.00%> (ø)` | |
| [parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow-rs/pull/1543/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-cGFycXVldC9zcmMvZW5jb2RpbmdzL2VuY29kaW5nLnJz) | `93.37% <0.00%> (-0.19%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1543?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/1543?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 [08f386a...3c600cb](https://codecov.io/gh/apache/arrow-rs/pull/1543?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