ARROW-17871: [Go] initial binary arithmetic implementation
Only implements Addition and Subtraction for integral types and float32/float64. Temporal types and others will come afterwards.
@pitrou Can you re-review when you get a chance and let me know if there's any other changes you request / if you're good with the current version?
@cyb70289 any other comments or questions here?
Anyone familiar enough with MSYS2 and MinGW to help out here? The error is:
:: Proceed with installation? [Y/n]
:: Retrieving packages...
error: failed retrieving file 'mingw-w64-x86_64-gcc-objc-12.2.0-4-any.pkg.tar.zst' from mirror.msys2.org : Operation too slow. Less than 1 bytes/sec transferred the last 10 seconds
mingw-w64-x86_64-gcc-12.2.0-4-any downloading...
error: failed retrieving file 'mingw-w64-x86_64-gmp-6.2.1-3-any.pkg.tar.zst.sig' from mirror2.sandyriver.net : Operation too slow. Less than 1 bytes/sec transferred the last 10 seconds
mingw-w64-x86_64-arrow-9.0.0-7-any downloading...
warning: failed to retrieve some files
mingw-w64-x86_64-icu-71.1-1-any downloading...
error: failed to commit transaction (unexpected error)
It looks like something is having an issue downloading the standard packages.
I restarted the failed build.
@pitrou looks like it still failed to load those packages via mingw which then caused the test failures. The error exit status 0xc0000139 is Windows MinGW saying that it couldn't load the dynamic library at runtime, which this test is testing using CGO to use the Arrow C++ memory pool as the allocator for Go's memory. One of the packages it failed to install and load was arrow-9.0.0 which would explain why it couldn't load the dynamic library at runtime. I'm not quite sure what to do here as this is the third restarted build in a row that has experienced this issue with MSYS2
@cyb70289 @pitrou
It looks like the current failure on the mingw64 cgo build is attributable to an existing issue with the MSYS2 mingw64 build having an issue loading libaws dlls and not something caused by the Go changes. So it is unrelated here but should likely be looked into.
It's most likely related to https://github.com/awslabs/aws-c-io/commit/550e3192b9d1c7ac865d3f71273bfd7729460900 due to the following erors:
Procedure entry point aws_byte_buf_append_encoding_uri_param could not be located in dynamic link library C:\msys64\mingw64\bin\libaws-c-s3.dll
Procedure entry point aws_byte_buf_append_decoding_uri could not be located in dynamic link library C:\msys64\mingw64\bin\libaws-c-auth.dll
Procedure entry point aws_uri_clean_up could not be located in dynamic link library C:\msys64\mingw64\bin\libaws-crt-cpp.dll
Similar error from Windows MinGW 64 C++ job. https://github.com/apache/arrow/actions/runs/3274965496/jobs/5389199341
~~Maybe create a jira issue (if not created yet)?~~ related issue: https://issues.apache.org/jira/browse/ARROW-18095 @kou
I think that this is related to MSYS2:
- https://github.com/msys2/MINGW-packages/pull/13618
- https://github.com/msys2/MINGW-packages/pull/13619
- https://github.com/msys2/MINGW-packages/pull/13620
BTW... @zeroshade How did you generate the error messages!? I want to know how to detect which symbol is missing to load a DLL when DLL load is failed!!!
@kou If you add the mingw64 path to your System level path, reboot, and then try running an exe through the Run Dialog (windows key+R) or through file explorer, then it'll give you those error messages as popup dialog boxes. I don't know how to get it to do so from the command line unfortunately. But this worked out pretty well! :smile:
@pitrou if you don't have any objections, i'll merge this tomorrow.
Can we wait until after the 10.0.0 release? Or will this only be in 11.0.0?
Question: is it useful to have both go/arrow/compute/internal/kernels/_lib/base_arithmetic_avx2_amd64.s and go/arrow/compute/internal/kernels/base_arithmetic_avx2_amd64.s? Is the latter generated from the former?
@pitrou The plan is that this would only go to 11.0.0, the maint-10.0.0 branch was already created right? So merges to master won't get included in the RC?
Question: is it useful to have both go/arrow/compute/internal/kernels/_lib/base_arithmetic_avx2_amd64.s and go/arrow/compute/internal/kernels/base_arithmetic_avx2_amd64.s? Is the latter generated from the former?
Yes, the latter is generated from the former, able to be regenerated via make assembly in the go/arrow/compute/internal/kernels/ directory. That said, the reason to keep the former around is for easier regeneration of the Go assembly allowing it to be generated without needing a C++ compiler/clang/etc. It ensures consistency where possible (and follows the pattern used elsewhere in the repo for our other use cases of this). Technically it isn't strictly necessary to keep it, I just do for the aforementioned reasons.
The latter must be kept around as it needs to be accessible when a user runs go get so that Go can assemble it during build. When running go get it only builds the files that are there, there is no expectation for a consumer to have to run make before building, it "should just work".
The plan is that this would only go to 11.0.0, the maint-10.0.0 branch was already created right? So merges to master won't get included in the RC?
Oh, you're right. Nevermind.
@kou If you add the mingw64 path to your System level path, reboot, and then try running an exe through the Run Dialog (windows key+R) or through file explorer, then it'll give you those error messages as popup dialog boxes. I don't know how to get it to do so from the command line unfortunately. But this worked out pretty well! :smile:
Thanks!!! It's what I want to know!!!