-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-41349: [C#] Optimize DecimalUtility.GetBytes(SqlDecimal) on .NET 7+ #41391
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…m link libraries (apache#41341) ### Rationale for this change We should use `.lib` (import library) not `.dll` for linking. So `.dll` is wrong. (But it seems working... Why...?) ### What changes are included in this PR? Remove `.dll` from link libraries because CMake generates suitable link options from library name automatically: https://cmake.org/cmake/help/latest/command/target_link_libraries.html#command:target_link_libraries > A plain library name: The generated link line will ask the linker to search for the library (e.g. `foo` becomes `-lfoo` or `foo.lib`). ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * GitHub Issue: apache#41340 Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
) ### Rationale for this change Ruby 2.7 doesn't exist on `macos-latest` (`macos-14`). ### What changes are included in this PR? Use `ruby` as the Ruby version to use the latest Ruby. ### Are these changes tested? Yes. ### Are there any user-facing changes? No. * GitHub Issue: apache#41371 Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]>
…ing dict (apache#41344) ### Rationale for this change `dictionary_length = 0` is used when page doesn't have dictionary, however, this should be a nop. ### What changes are included in this PR? Change `dictionary_length = 0` to `*dictionary_length = 0`. ### Are these changes tested? No? ### Are there any user-facing changes? no Authored-by: mwish <[email protected]> Signed-off-by: mwish <[email protected]>
…f `macos-latest` change to `macos-14` (apache#41384) ### Rationale for this change * The MATLAB macOS CI workflow is failing because of the recent change to `macos-latest` from `macos-12` to `macos-14`. * In https://github.blog/changelog/2024-01-30-github-actions-macos-14-sonoma-is-now-available/ it is mentioned that the transition to `macos-14` was planned for April-June 2024. * As noted in the [Platform Road Map](https://www.mathworks.com/support/requirements/platform-road-map.html), MATLAB R2023a is not officially supported with `macos-14`. * Until we can move to building against MATLAB R2024a (latest available version of MATLAB), it makes sense to pin back the macOS version to version 14. ### What changes are included in this PR? 1. Pin back MATLAB CI workflow to use `macos-12`. 2. Pin back MATLAB crossbow packaging workflow to use `macos-12`. ### Are these changes tested? I will run a crossbow job and ensure that the MATLAB CI workflow passes successfully before merging this PR. ### Are there any user-facing changes? No. ### Future Directions 1. apache#41385 ### Notes 1. Thanks @ sgilmore10 for your help with this pull request! * GitHub Issue: apache#41370 Authored-by: Kevin Gurney <[email protected]> Signed-off-by: Kevin Gurney <[email protected]>
### Rationale for this change We don't have write permission for `/usr/local` on macos-14. ### What changes are included in this PR? Use `/tmp/local` instead. ### Are these changes tested? Yes. ### Are there any user-facing changes? No. * GitHub Issue: apache#41369 Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
) ### Rationale for this change It's useful to detect type difference. ### What changes are included in this PR? Add `:show_column_type` option to `Arrow::Table#to_s` and enables it by default. This is a backward incompatible change but this'll help users. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. **This PR includes breaking changes to public APIs.** * GitHub Issue: apache#41327 Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
…obuf.cmake (apache#41360) ### Rationale for this change `protobuf::libprotobuf` provided by `FindProtobuf.cmake` (provided by CMake) may not provide needed dependencies such as Abseil. ### What changes are included in this PR? Try `protobuf-config.cmake` provided by Protobuf before `FindProtobuf.cmake`. `protobuf::libprotobuf` provided by `protobuf-config.cmake` must have needed dependencies. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * GitHub Issue: apache#41333 Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
…t if it exists (apache#41305) ### Rationale for this change When we created the `16.1.0` milestone the merge script started prompting it instead of `17.0.0` we want to default to the next major release. ### What changes are included in this PR? Update archery logic to default to major versions online. ### Are these changes tested? I've tested locally and now it defaults to `17.0.0`: ``` Enter fix version [17.0.0]: ``` ### Are there any user-facing changes? No * GitHub Issue: apache#41282 Authored-by: Raúl Cumplido <[email protected]> Signed-off-by: Raúl Cumplido <[email protected]>
### What changes are included in this PR? Changes to workflow infrastructure and projects to install and target net8.0 instead of net7.0. ### Are these changes tested? Yes ### Are there any user-facing changes? Users will need to install .NET 8 to run tests and examples. No impact on product code. Closes apache#41375 * GitHub Issue: apache#41375 Lead-authored-by: Curt Hagenlocher <[email protected]> Co-authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Curt Hagenlocher <[email protected]>
CurtHagenlocher
force-pushed
the
GH-41349
branch
from
June 14, 2024 02:22
d9b52f2
to
e576f54
Compare
CurtHagenlocher
requested review from
assignUser,
kou,
raulcd and
wgtmac
as code owners
June 14, 2024 02:22
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes are included in this PR?
Adds code to avoid an allocation when converting from SqlDecimal to Decimal128.
Adds a .NET 8 target to Apache.Arrow.csproj to enable the optimization.
Are these changes tested?
Yes.
Are there any user-facing changes?
No.
Closes #41349