GH-35526: [CI][C++] Fixing arrow::internal::IsNullRunEndEncoded redeclared #35527

mapleFU · 2023-05-10T10:49:23Z

Rationale for this change

Fixing arrow::internal::IsNullRunEndEncoded redeclared. The CI will report:

'bool arrow::internal::IsNullRunEndEncoded(const arrow::ArrayData&, int64_t)' redeclared without dllimport attribute after being referenced with dll linkage

And:

warning: 'selection_vector_type' may be used uninitialized

What changes are included in this PR?

Add ARROW_FRIEND_EXPORT for friend function
Initialize pointer in gandiva with nullptr

Are these changes tested?

No

Are there any user-facing changes?

No

Closes: [CI][C++] arrow::internal::IsNullRunEndEncoded redeclared #35526

github-actions · 2023-05-10T10:49:47Z

Closes: [CI][C++] arrow::internal::IsNullRunEndEncoded redeclared #35526

github-actions · 2023-05-10T10:49:49Z

⚠️ GitHub issue #35526 has been automatically assigned in GitHub to PR creator.

mapleFU · 2023-05-10T11:04:24Z

This fixing is wrong, it's not anonymous namespace problem

mapleFU · 2023-05-10T11:09:49Z

@kou Mind take a look?
cc @felipecrv

mapleFU · 2023-05-10T12:00:14Z

Seems this was caused by gandiva or CI changed, I guess this will not produce a nullptr or uninitialized:

  llvm::Type* selection_vector_type;
  switch (selection_vector_mode) {
    case SelectionVector::MODE_NONE:
    case SelectionVector::MODE_UINT16:
      arguments.push_back(types()->ptr_type(types()->i16_type()));
      selection_vector_type = types()->i16_type();
      break;
    case SelectionVector::MODE_UINT32:
      arguments.push_back(types()->i32_ptr_type());
      selection_vector_type = types()->i32_type();
      break;
    case SelectionVector::MODE_UINT64:
      arguments.push_back(types()->i64_ptr_type());
      selection_vector_type = types()->i64_type();
      break;
  }

[125/131] Building CXX object src/gandiva/CMakeFiles/gandiva_shared.dir/Unity/unity_4_cxx.cxx.obj
In file included from D:/a/arrow/arrow/build/cpp/src/gandiva/CMakeFiles/gandiva_shared.dir/Unity/unity_4_cxx.cxx:5:
D:/a/arrow/arrow/cpp/src/gandiva/llvm_generator.cc: In member function 'arrow::Status gandiva::LLVMGenerator::CodeGenExprValue(gandiva::DexPtr, int, gandiva::FieldDescriptorPtr, int, std::string&, gandiva::SelectionVector::Mode)':
D:/a/arrow/arrow/cpp/src/gandiva/llvm_generator.cc:362:27: warning: 'selection_vector_type' may be used uninitialized [-Wmaybe-uninitialized]
  362 |         builder->CreateGEP(selection_vector_type, arg_selection_vector, loop_var);
      |         ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
D:/a/arrow/arrow/cpp/src/gandiva/llvm_generator.cc:282:15: note: 'selection_vector_type' was declared here
  282 |   llvm::Type* selection_vector_type;
      |               ^~~~~~~~~~~~~~~~~~~~~

mapleFU · 2023-05-10T14:09:57Z

[421/916] Building CXX object src/arrow/util/CMakeFiles/arrow-utility-test.dir/iterator_test.cc.obj
In file included from D:/a/_temp/msys64/mingw64/include/c++/13.1.0/bits/shared_ptr.h:53,
                 from D:/a/_temp/msys64/mingw64/include/c++/13.1.0/condition_variable:45,
                 from D:/a/arrow/arrow/cpp/src/arrow/util/iterator_test.cc:21:

Seems that it reports shared_ptr is wrong in this case, another case is that:

    // Move into itself
    moved_moved_ints = std::move(moved_moved_ints);

in small_vector_test.cc. I don't know why they're reported. So I just leave them here.

cpp/src/gandiva/llvm_generator.cc

pitrou · 2023-05-10T15:22:19Z

Rationale for this change

Change namespace to anonymous one

The description here is incorrect, can you fix it to match the PR changes?

mapleFU · 2023-05-10T15:26:47Z

Description changed

pitrou

+1, let's wait for CI now

mapleFU · 2023-05-10T16:33:47Z

These two problems mentioned #35527 (comment) can be seen at:
https://github.com/apache/arrow/actions/runs/4938674895/jobs/8828705192?pr=35527

It breaks "C++ / AMD64 Windows MinGW MINGW32 C++" and "C++ / AMD64 Windows MinGW MINGW64 C++ (pull_request)"

Another problem is that in "Python / AMD64 macOS 12 Python 3":

=================================== FAILURES ===================================
___________________ test_pandas_assertion_error_large_string ___________________

    @pytest.mark.large_memory
    @pytest.mark.pandas
    def test_pandas_assertion_error_large_string():
        # Test AssertionError as pandas does not support "U" type strings
        if Version(pd.__version__) < Version("1.5.0"):
            pytest.skip("__dataframe__ added to pandas in 1.5.0")
    
        data = np.array([b'x'*1024]*(3*1024**2), dtype='object')  # 3GB bytes data
        arr = pa.array(data, type=pa.large_string())
        table = pa.table([arr], names=["large_string"])
    
        from pandas.api.interchange import (
            from_dataframe as pandas_from_dataframe
        )
    
>       with pytest.raises(AssertionError):
E       Failed: DID NOT RAISE <class 'AssertionError'>

/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/pyarrow/tests/interchange/test_conversion.py:294: Failed
=============================== warnings summary ===============================

Which could be seen here: https://github.com/apache/arrow/actions/runs/4938674893/jobs/8828707211?pr=35527

Failed in "R" might be fixed in main branch.

For "C GLib & Ruby / AMD64 Windows MinGW 64 GLib & Ruby (pull_request)", here is:

Running the tests in ‘tests/testthat.R’ failed.

I guess thats the issue

mapleFU · 2023-05-10T16:39:46Z

For Python Pandas assertion, here is a patch: #35530
I wonder if it helps...

pitrou · 2023-05-10T17:22:41Z

Hmm, it seems there's another issue now on MinGW GLib build:
https://github.com/apache/arrow/actions/runs/4938674894/jobs/8829920058?pr=35527

@kou, would you be able to take a look here?


2023-05-10T17:13:11.8253839Z [126/131] Building CXX object src/gandiva/CMakeFiles/gandiva_shared.dir/Unity/unity_2_cxx.cxx.obj
2023-05-10T17:13:11.8256428Z FAILED: src/gandiva/CMakeFiles/gandiva_shared.dir/Unity/unity_2_cxx.cxx.obj 
2023-05-10T17:13:13.7339400Z C:\msys64\ucrt64\bin\ccache.exe C:\msys64\ucrt64\bin\c++.exe -DARROW_HAVE_RUNTIME_AVX2 -DARROW_HAVE_RUNTIME_BMI2 -DARROW_HAVE_RUNTIME_SSE4_2 -DARROW_HAVE_SSE4_2 -DARROW_WITH_RE2 -DARROW_WITH_TIMING_TESTS -DARROW_WITH_UTF8PROC -DAWS_AUTH_USE_IMPORT_EXPORT -DAWS_CAL_USE_IMPORT_EXPORT -DAWS_CHECKSUMS_USE_IMPORT_EXPORT -DAWS_COMMON_USE_IMPORT_EXPORT -DAWS_COMPRESSION_USE_IMPORT_EXPORT -DAWS_CRT_CPP_USE_IMPORT_EXPORT -DAWS_EVENT_STREAM_USE_IMPORT_EXPORT -DAWS_HTTP_USE_IMPORT_EXPORT -DAWS_IO_USE_IMPORT_EXPORT -DAWS_MQTT_USE_IMPORT_EXPORT -DAWS_MQTT_WITH_WEBSOCKETS -DAWS_S3_USE_IMPORT_EXPORT -DAWS_SDKUTILS_USE_IMPORT_EXPORT -DAWS_SDK_VERSION_MAJOR=1 -DAWS_SDK_VERSION_MINOR=11 -DAWS_SDK_VERSION_PATCH=67 -DAWS_USE_IO_COMPLETION_PORTS -DBOOST_ALL_DYN_LINK -DBOOST_ALL_NO_LIB -DBOOST_USE_WINDOWS_H=1 -DGANDIVA_EXPORTING -DGANDIVA_LLVM_VERSION=16 -DUSE_IMPORT_EXPORT -DUSE_IMPORT_EXPORT=1 -DUSE_WINDOWS_DLL_SEMANTICS -D_CRT_SECURE_NO_WARNINGS -D_ENABLE_EXTENDED_ALIGNED_STORAGE -Dgandiva_shared_EXPORTS -ID:/a/arrow/arrow/build/cpp/src -ID:/a/arrow/arrow/cpp/src -ID:/a/arrow/arrow/cpp/src/generated -isystem D:/a/arrow/arrow/cpp/thirdparty/flatbuffers/include -isystem D:/a/arrow/arrow/cpp/thirdparty/hadoop/include -isystem D:/a/arrow/arrow/build/cpp/google_cloud_cpp_ep-install/include -isystem D:/a/arrow/arrow/build/cpp/crc32c_ep-install/include -Wno-noexcept-type -Wno-subobject-linkage  -fdiagnostics-color=always  -Wa,-mbig-obj -Wall -Wno-conversion -Wno-sign-conversion -Wunused-result -fno-semantic-interposition -mxsave -msse4.2  -O3 -DNDEBUG -O2 -ftree-vectorize -std=c++17 -MD -MT src/gandiva/CMakeFiles/gandiva_shared.dir/Unity/unity_2_cxx.cxx.obj -MF src\gandiva\CMakeFiles\gandiva_shared.dir\Unity\unity_2_cxx.cxx.obj.d -o src/gandiva/CMakeFiles/gandiva_shared.dir/Unity/unity_2_cxx.cxx.obj -c D:/a/arrow/arrow/build/cpp/src/gandiva/CMakeFiles/gandiva_shared.dir/Unity/unity_2_cxx.cxx

kou · 2023-05-10T18:44:33Z

Oh, it was failed without error message...

I can try this on my local environment next week. (I attend RubyKaigi 2023 in this week.)

mapleFU · 2023-05-11T07:54:56Z

[ RUN      ] TestDecimalFromReal/1.TestSuccess
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "4611686000000000000"
  expected
    Which is: "4611686018427387904"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-4611686000000000000"
  expected
    Which is: "-4611686018427387904"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "9223372000000000000"
  expected
    Which is: "9223372036854775808"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-9223372000000000000"
  expected
    Which is: "-9223372036854775808"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "18446744000000000000"
  expected
    Which is: "18446744073709551616"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-18446744000000000000"
  expected
    Which is: "-18446744073709551616"
[  FAILED  ] TestDecimalFromReal/1.TestSuccess, where TypeParam = std::pair<arrow::Decimal128, double> (0 ms)

The following tests FAILED:
32 - arrow-utility-test (Failed)
78 - parquet-encryption-test (Failed)

These two test failed, never seen it before. @wgtmac do you have any idea why parquet-encryption-test failed now? And would it failed on windows or could failed on other platforms?

mapleFU · 2023-05-11T14:16:05Z

@pitrou Could we get this fixing merged or need to wait some testing or failed ci passing?

pitrou · 2023-05-11T15:58:45Z

@mapleFU I've restarted some CI jobs now that GitHub seems to be doing a bit better :-)

mapleFU · 2023-05-11T16:02:28Z

Github has problem these three days...But I wonder that why these UT failed is related :-(

pitrou · 2023-05-11T16:53:18Z

The MinGW C++ errors and segfaults don't look related, so I'm going to merge this PR. Thanks @mapleFU !

ursabot · 2023-05-13T03:34:09Z

Benchmark runs are scheduled for baseline = 4fd5c28 and contender = 401ae19. 401ae19 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️1.98% ⬆️0.06%] test-mac-arm
[Finished ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.72% ⬆️0.03%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 401ae190 ec2-t3-xlarge-us-east-2
[Finished] 401ae190 test-mac-arm
[Finished] 401ae190 ursa-i9-9960x
[Finished] 401ae190 ursa-thinkcentre-m75q
[Finished] 4fd5c28e ec2-t3-xlarge-us-east-2
[Finished] 4fd5c28e test-mac-arm
[Finished] 4fd5c28e ursa-i9-9960x
[Finished] 4fd5c28e ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

… redeclared (apache#35527) ### Rationale for this change Fixing arrow::internal::IsNullRunEndEncoded redeclared. The CI will report: ``` 'bool arrow::internal::IsNullRunEndEncoded(const arrow::ArrayData&, int64_t)' redeclared without dllimport attribute after being referenced with dll linkage ``` And: ``` warning: 'selection_vector_type' may be used uninitialized ``` ### What changes are included in this PR? 1. Add `ARROW_FRIEND_EXPORT` for friend function 2. Initialize pointer in gandiva with `nullptr` ### Are these changes tested? No ### Are there any user-facing changes? No * Closes: apache#35526 Authored-by: mwish <maplewish117@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org>

Fixing: using anonymous namespace for IsNull in arrow/array/data.cc

d2d453f

github-actions bot added Component: C++ awaiting review Awaiting review labels May 10, 2023

mapleFU marked this pull request as draft May 10, 2023 11:04

Revert previous and fix

a974a2f

mapleFU changed the title ~~GH-35526: [CI][C++] Fixing: using anonymous namespace for IsNull in arrow/array/data.cc~~ GH-35526: [CI][C++] Fixing arrow::internal::IsNullRunEndEncoded redeclared May 10, 2023

mapleFU marked this pull request as ready for review May 10, 2023 11:10

jorisvandenbossche requested a review from felipecrv May 10, 2023 11:42

fix gandiva compiling in AMD64 macOS 12 Python 3

e6ac511

mapleFU force-pushed the ci/using-anonymous-ns-for-is-null branch from 7a3c1a6 to e6ac511 Compare May 10, 2023 12:57

github-actions bot added the Component: C++ - Gandiva label May 10, 2023

mapleFU mentioned this pull request May 10, 2023

GH-35519: [C++][Parquet] Fixing exception handling in parquet FileSerializer #35520

Merged

mapleFU mentioned this pull request May 10, 2023

MINOR: [C++] Add missing ARROW_EXPORT to IsNull impls #35533

Closed

pitrou reviewed May 10, 2023

View reviewed changes

cpp/src/gandiva/llvm_generator.cc Outdated Show resolved Hide resolved

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels May 10, 2023

address comment

d54cc34

felipecrv approved these changes May 10, 2023

View reviewed changes

pitrou approved these changes May 10, 2023

View reviewed changes

Merge branch 'main' into ci/using-anonymous-ns-for-is-null

f8bcf90

pitrou merged commit 401ae19 into apache:main May 11, 2023
30 of 33 checks passed

mapleFU deleted the ci/using-anonymous-ns-for-is-null branch May 11, 2023 16:58

felipecrv added this to the 12.0.1 milestone May 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-35526: [CI][C++] Fixing arrow::internal::IsNullRunEndEncoded redeclared #35527

GH-35526: [CI][C++] Fixing arrow::internal::IsNullRunEndEncoded redeclared #35527

mapleFU commented May 10, 2023 •

edited

github-actions bot commented May 10, 2023

github-actions bot commented May 10, 2023

mapleFU commented May 10, 2023

mapleFU commented May 10, 2023

mapleFU commented May 10, 2023

mapleFU commented May 10, 2023

pitrou commented May 10, 2023

Rationale for this change

mapleFU commented May 10, 2023

pitrou left a comment

mapleFU commented May 10, 2023

mapleFU commented May 10, 2023

pitrou commented May 10, 2023

kou commented May 10, 2023

mapleFU commented May 11, 2023

mapleFU commented May 11, 2023

pitrou commented May 11, 2023

mapleFU commented May 11, 2023

pitrou commented May 11, 2023

ursabot commented May 13, 2023

GH-35526: [CI][C++] Fixing arrow::internal::IsNullRunEndEncoded redeclared #35527

GH-35526: [CI][C++] Fixing arrow::internal::IsNullRunEndEncoded redeclared #35527

Conversation

mapleFU commented May 10, 2023 • edited

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

github-actions bot commented May 10, 2023

github-actions bot commented May 10, 2023

mapleFU commented May 10, 2023

mapleFU commented May 10, 2023

mapleFU commented May 10, 2023

mapleFU commented May 10, 2023

pitrou commented May 10, 2023

Rationale for this change

mapleFU commented May 10, 2023

pitrou left a comment

Choose a reason for hiding this comment

mapleFU commented May 10, 2023

mapleFU commented May 10, 2023

pitrou commented May 10, 2023

kou commented May 10, 2023

mapleFU commented May 11, 2023

mapleFU commented May 11, 2023

pitrou commented May 11, 2023

mapleFU commented May 11, 2023

pitrou commented May 11, 2023

ursabot commented May 13, 2023

mapleFU commented May 10, 2023 •

edited