From 5ec7493a0976520cbfa519310e14065d910e462c Mon Sep 17 00:00:00 2001 From: DenisTarasyuk <131180287+DenisTarasyuk@users.noreply.github.com> Date: Tue, 30 Apr 2024 03:59:51 +0300 Subject: [PATCH] GH-41433: [C++][Gandiva] Fix ascii_utf8 function to return same result on x86 and Arm (#41434) Fixing ascii_utf8 function that has different return result on x86 and Arm due to default char type sign difference on those platforms. Added tests to cover existing x86 behavior for ascii symbols with code >127. 1. Added type cast to signed char to save existing x86 behavior on Arm platform. 2. Added tests cases for negative results. UT included. None * GitHub Issue: #41433 Authored-by: DenisTarasyuk Signed-off-by: Sutou Kouhei --- cpp/src/gandiva/precompiled/string_ops.cc | 2 +- cpp/src/gandiva/precompiled/string_ops_test.cc | 2 ++ dev/tasks/docker-tests/github.linux.yml | 2 +- dev/tasks/java-jars/github.yml | 6 +++--- dev/tasks/r/github.devdocs.yml | 2 +- dev/tasks/r/github.linux.arrow.version.back.compat.yml | 2 +- dev/tasks/r/github.linux.cran.yml | 2 +- dev/tasks/r/github.linux.offline.build.yml | 4 ++-- dev/tasks/r/github.linux.versions.yml | 2 +- dev/tasks/r/github.macos-linux.local.yml | 2 +- 10 files changed, 14 insertions(+), 12 deletions(-) diff --git a/cpp/src/gandiva/precompiled/string_ops.cc b/cpp/src/gandiva/precompiled/string_ops.cc index 5aa0eb38eaf..3849cf7bdf9 100644 --- a/cpp/src/gandiva/precompiled/string_ops.cc +++ b/cpp/src/gandiva/precompiled/string_ops.cc @@ -1377,7 +1377,7 @@ gdv_int32 ascii_utf8(const char* data, gdv_int32 data_len) { if (data_len == 0) { return 0; } - return static_cast(data[0]); + return static_cast(static_cast(data[0])); } // Returns the ASCII character having the binary equivalent to A. diff --git a/cpp/src/gandiva/precompiled/string_ops_test.cc b/cpp/src/gandiva/precompiled/string_ops_test.cc index 89213592e7e..aaa25db0a9f 100644 --- a/cpp/src/gandiva/precompiled/string_ops_test.cc +++ b/cpp/src/gandiva/precompiled/string_ops_test.cc @@ -51,6 +51,8 @@ TEST(TestStringOps, TestAscii) { EXPECT_EQ(ascii_utf8("", 0), 0); EXPECT_EQ(ascii_utf8("123", 3), 49); EXPECT_EQ(ascii_utf8("999", 3), 57); + EXPECT_EQ(ascii_utf8("\x80", 1), -128); + EXPECT_EQ(ascii_utf8("\xFF", 1), -1); } TEST(TestStringOps, TestChrBigInt) { diff --git a/dev/tasks/docker-tests/github.linux.yml b/dev/tasks/docker-tests/github.linux.yml index 13e00abc70a..8848c7dd593 100644 --- a/dev/tasks/docker-tests/github.linux.yml +++ b/dev/tasks/docker-tests/github.linux.yml @@ -61,7 +61,7 @@ jobs: done - name: Save the R test output if: always() - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: test-output path: arrow/r/check/arrow.Rcheck/tests/testthat.Rout* diff --git a/dev/tasks/java-jars/github.yml b/dev/tasks/java-jars/github.yml index 4533e62bce0..59545c4703d 100644 --- a/dev/tasks/java-jars/github.yml +++ b/dev/tasks/java-jars/github.yml @@ -74,7 +74,7 @@ jobs: - name: Compress into single artifact to keep directory structure run: tar -cvzf arrow-shared-libs-linux-{{ arch }}.tar.gz arrow/java-dist/ - name: Upload artifacts - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: ubuntu-shared-lib-{{ arch }} path: arrow-shared-libs-linux-{{ arch }}.tar.gz @@ -154,7 +154,7 @@ jobs: - name: Compress into single artifact to keep directory structure run: tar -cvzf arrow-shared-libs-macos-{{ arch }}.tar.gz arrow/java-dist/ - name: Upload artifacts - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: macos-shared-lib-{{ arch }} path: arrow-shared-libs-macos-{{ arch }}.tar.gz @@ -188,7 +188,7 @@ jobs: shell: bash run: tar -cvzf arrow-shared-libs-windows.tar.gz arrow/java-dist/ - name: Upload artifacts - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: windows-shared-lib path: arrow-shared-libs-windows.tar.gz diff --git a/dev/tasks/r/github.devdocs.yml b/dev/tasks/r/github.devdocs.yml index 92e94c6c976..0f2a675dc16 100644 --- a/dev/tasks/r/github.devdocs.yml +++ b/dev/tasks/r/github.devdocs.yml @@ -68,7 +68,7 @@ jobs: EOF shell: bash -l {0} - name: Save the install script - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: {{ "devdocs-script_os-${{ matrix.os }}_sysinstall-${{ matrix.system-install }}" }} path: arrow/r/vignettes/developers/script.sh diff --git a/dev/tasks/r/github.linux.arrow.version.back.compat.yml b/dev/tasks/r/github.linux.arrow.version.back.compat.yml index 086705dbb9c..39ac546a39d 100644 --- a/dev/tasks/r/github.linux.arrow.version.back.compat.yml +++ b/dev/tasks/r/github.linux.arrow.version.back.compat.yml @@ -58,7 +58,7 @@ jobs: shell: bash - name: Upload the parquet artifacts - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: files path: arrow/r/extra-tests/files diff --git a/dev/tasks/r/github.linux.cran.yml b/dev/tasks/r/github.linux.cran.yml index 0aeb7cfa2b4..ea8983804c7 100644 --- a/dev/tasks/r/github.linux.cran.yml +++ b/dev/tasks/r/github.linux.cran.yml @@ -56,7 +56,7 @@ jobs: if: always() - name: Save the test output if: always() - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: test-output path: arrow/r/check/arrow.Rcheck/tests/testthat.Rout* diff --git a/dev/tasks/r/github.linux.offline.build.yml b/dev/tasks/r/github.linux.offline.build.yml index 9ac0ebc4083..4085551ca16 100644 --- a/dev/tasks/r/github.linux.offline.build.yml +++ b/dev/tasks/r/github.linux.offline.build.yml @@ -41,7 +41,7 @@ jobs: R -e "source('R/install-arrow.R'); create_package_with_all_dependencies(dest_file = 'arrow_with_deps.tar.gz', source_file = \"${built_tar}\")" shell: bash - name: Upload the third party dependency artifacts - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: thirdparty_deps path: arrow/r/arrow_with_deps.tar.gz @@ -91,7 +91,7 @@ jobs: run: cat arrow-tests/testthat.Rout* if: always() - name: Save the test output - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: test-output path: arrow-tests/testthat.Rout* diff --git a/dev/tasks/r/github.linux.versions.yml b/dev/tasks/r/github.linux.versions.yml index 753efe61d04..79f31d7c850 100644 --- a/dev/tasks/r/github.linux.versions.yml +++ b/dev/tasks/r/github.linux.versions.yml @@ -55,7 +55,7 @@ jobs: if: always() - name: Save the test output if: always() - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: test-output path: arrow/r/check/arrow.Rcheck/tests/testthat.Rout* diff --git a/dev/tasks/r/github.macos-linux.local.yml b/dev/tasks/r/github.macos-linux.local.yml index b221e8c5d8d..4beb87657f3 100644 --- a/dev/tasks/r/github.macos-linux.local.yml +++ b/dev/tasks/r/github.macos-linux.local.yml @@ -97,7 +97,7 @@ jobs: run: cat arrow-tests/testthat.Rout* if: failure() - name: Save the test output - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: test-output path: arrow-tests/testthat.Rout*