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 5aa0eb38eafd7..3849cf7bdf9a5 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 89213592e7ea2..aaa25db0a9f8d 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 13e00abc70a84..8848c7dd59378 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 4533e62bce037..59545c4703dfb 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 92e94c6c97637..0f2a675dc1682 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 086705dbb9cf4..39ac546a39d6a 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 0aeb7cfa2b434..ea8983804c7cc 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 9ac0ebc40835e..4085551ca166d 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 753efe61d048e..79f31d7c850b4 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 b221e8c5d8d5b..4beb87657f34c 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*