Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
andishgar closed pull request #45663: GH-45474: [C++] Add support for building statistics array for nested types URL: https://github.com/apache/arrow/pull/45663 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
kou commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-3609082296 1. No problem :-) 2. Let's do it. 3. We can't assume the number of columns. Let's choose easier way as the initial implementation. We can improve the internal algorithm with benchmark results later. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2776091150 @kou After reading a recent discussion on the [Arrow mailing list,](https://lists.apache.org/thread/3wv5v3c3mboq0ynf8zkd6rolnob0t8n8) I understand the benefit of smaller pull requests—they make the problem simpler, allowing reviewers to focus better and provide more useful feedback. Would you mind if I break this pull request into smaller ones as I had planned? > 1. Solve the problem related to `StringView` and `BinaryView` [[C++] Add StringView and BinaryView to ArrayStatistics #45664](https://github.com/apache/arrow/issues/45664) which needs some fixes in the test case, test tools, and `MakeStatisticsArray` (the solution with relevant tests exists in my last commit). > 2. Add the `MakeStatisticsArray` method to Array, which needs the relocation of tools and the making of some files (like the same job that I did in my third commit), as the current algorithm for making statistics array needs some justification (the solution exists in statistics.cc for the `MakeStatisticsArray` method). > 3. Add missing attributes. I mean this issue [[C++] implement Statistic Schema attribute in C++ #45639](https://github.com/apache/arrow/issues/45639) > 4. Finally, adding testing tools (`TestStatisticsArray` in` statistics_test_util.h`) and support for nested types. > The problem is that I use new tools for testing statistics(I have not commited yet). I'll send pull for it after this pull. BTW, sorry for my previous response. I sometimes tend to overcomplicate things for myself -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2739795116 I changed and relocated methods to make it easier to create a statistics array and subsequent tests for Array in another pull request. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
kou commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2777464915 Thanks for the suggestion. Let's try the suggestion! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2766720947 > If the new tools are needed for this and other pull requests, how about opening a pull request for it BEFORE (not after) this and other pull requests? I sent my commit to make our discussion more clear. The problem is that the testing tools which are defined in `statistics_test_utils.h ` are based on the new behaviour that the `MakeStatisticsArray` method has (which is currently defined in` statistics.cc `and used in the `MakeStatisticsArray` method of `RecordBath`). The problem is that when I took this issue I thought it was solved just by implementing a depth-first search and relevant tests (like the work that I did in the first and second commits). However, as I progressed I figured out more codes were needed. If I should break down this pull request into smaller ones, I suggest let's do it in that manner. 1. Solve the problem related to `StringView` and `BinaryView` #45664 which needs some fixes in the test case, test tools, and `MakeStatisticsArray` (the solution with relevant tests exists in my last commit). 2. Add the `MakeStatisticsArray` method to Array, which needs the relocation of tools and the making of some files (like the same job that I did in my third commit), as the current algorithm for making statistics array needs some justification (the solution exists in statistics.cc for the `MakeStatisticsArray` method). 3. Add missing attributes. I mean this issue #45639 4. Finally, adding testing tools, I mean adding TestStatisticsArray in statistics_test_util.h and support for nested types. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
kou commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2764842369 If the new tools are needed for this and other pull requests, how about opening a pull request for it BEFORE (not after) this and other pull requests? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2764761090 I found some bugs and flaws in the current implementations which are relevant to these issues. For example, the current code considers all string types as utf8 in testing tools and making statistics or I create some changes in the statistics tools and MakeStatisticsArray to support Array. I think it's better #45664 is solved in this pull request, and #45804 in another pull -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2764785521 The problem is that I use new tools for testing statistics(I have not commited yet). I'll send pull for it after this pull. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
kou commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2764770012 How about opening a new PR for #45664 instead of reusing this PR? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
kou commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2764753892 Is it difficult that we use separated PRs for them...? In general, a large PR is difficult to review... -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2764751085 I will resolve [#45664](https://github.com/apache/arrow/issues/45664) and [#45804](https://github.com/apache/arrow/issues/45804) in this pull request. I will send a commit to solve them in the following days. Close with comment Comment -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
kou commented on code in PR #45663:
URL: https://github.com/apache/arrow/pull/45663#discussion_r1992767086
##
cpp/src/arrow/array/statistics_test.cc:
##
@@ -91,6 +93,28 @@ TEST(ArrayStatisticsTest, TestEquality) {
ASSERT_NE(statistics1, statistics2);
statistics2.max = static_cast(-29);
ASSERT_EQ(statistics1, statistics2);
+ statistics2.max = static_cast(2);
+ ASSERT_NE(statistics1, statistics2);
+
+ statistics1.max = std::nullopt;
+ statistics2.max = std::nullopt;
+ // check the state of both of them are std::nullopt
+ ASSERT_EQ(statistics1.max, statistics2.max);
+ // the state of one of them is std::nullopt
Review Comment:
```suggestion
// the state of one of them is std::nullopt
```
##
cpp/src/arrow/array/statistics_test.cc:
##
@@ -91,6 +93,28 @@ TEST(ArrayStatisticsTest, TestEquality) {
ASSERT_NE(statistics1, statistics2);
statistics2.max = static_cast(-29);
ASSERT_EQ(statistics1, statistics2);
+ statistics2.max = static_cast(2);
+ ASSERT_NE(statistics1, statistics2);
+
+ statistics1.max = std::nullopt;
+ statistics2.max = std::nullopt;
+ // check the state of both of them are std::nullopt
+ ASSERT_EQ(statistics1.max, statistics2.max);
+ // the state of one of them is std::nullopt
+ statistics1.max = std::shared_ptr();
+ ASSERT_NE(statistics1, statistics2);
+ // the state of both of them are nullptr
Review Comment:
```suggestion
// the state of both of them are empty shared_ptr
```
##
cpp/src/arrow/record_batch_test.cc:
##
@@ -1454,6 +1462,94 @@ TEST_F(TestRecordBatch, MakeStatisticsArrayString) {
AssertArraysEqual(*expected_statistics_array, *statistics_array, true);
}
+TEST_F(TestRecordBatch, MakeStatisticsArrayNestedType) {
+ auto struct_type = struct_({field("a", int64()), field("b", int64())});
+ auto struct_array = ArrayFromJSON(
+ struct_type,
+
R"([{"a":1,"b":6},{"a":2,"b":7},{"a":3,"b":8},{"a":4,"b":9},{"a":5,"b":10}])");
+ ASSERT_OK_AND_ASSIGN(auto struct_nested_stat,
Review Comment:
Can we use better name? This is not a statistics array, right?
##
cpp/src/arrow/record_batch_test.cc:
##
@@ -1454,6 +1462,94 @@ TEST_F(TestRecordBatch, MakeStatisticsArrayString) {
AssertArraysEqual(*expected_statistics_array, *statistics_array, true);
}
+TEST_F(TestRecordBatch, MakeStatisticsArrayNestedType) {
+ auto struct_type = struct_({field("a", int64()), field("b", int64())});
+ auto struct_array = ArrayFromJSON(
+ struct_type,
+
R"([{"a":1,"b":6},{"a":2,"b":7},{"a":3,"b":8},{"a":4,"b":9},{"a":5,"b":10}])");
+ ASSERT_OK_AND_ASSIGN(auto struct_nested_stat,
+ struct_array->CopyTo(default_cpu_memory_manager()));
+ auto statistics_struct = std::make_shared();
+ ASSERT_OK_AND_ASSIGN(statistics_struct->max, struct_array->GetScalar(4));
+ statistics_struct->null_count = 0;
+ auto struct_array_data = struct_array->data();
+ auto statistics_struct_child_a = std::make_shared();
+ statistics_struct_child_a->min = int64_t{1};
+ struct_array_data->statistics = statistics_struct;
+ struct_array_data->child_data[0]->statistics = statistics_struct_child_a;
+ auto array_c = ArrayFromJSON(int64(), R"([11,12,13,14,15])");
+ array_c->data()->statistics = std::make_shared();
+ array_c->data()->statistics->max = int64_t{15};
+ auto array_d = ArrayFromJSON(int64(), R"([16,17,18,19,20])");
+ auto nested_child = struct_nested_stat->data()->child_data[0];
+ nested_child->statistics = std::make_shared();
+ nested_child->statistics->max = int64_t{5};
+ nested_child->statistics->is_max_exact = true;
+
+ auto rb_schema =
+ schema({field("struct_a_b", struct_type), field("c", int64()),
field("d", int64()),
+ field("struct_copy", struct_nested_stat->type())});
+ auto rb = RecordBatch::Make(rb_schema, 5,
+ {struct_array, array_c, array_d,
struct_nested_stat});
+
+ auto expected_scalar = internal::checked_pointer_cast(
+ ScalarFromJSON(struct_type, R"([5,10])"));
+ auto a =
ArrayStatistics::ValueType{std::static_pointer_cast(expected_scalar)};
Review Comment:
Is this used?
##
cpp/src/arrow/record_batch_test.cc:
##
@@ -1454,6 +1462,94 @@ TEST_F(TestRecordBatch, MakeStatisticsArrayString) {
AssertArraysEqual(*expected_statistics_array, *statistics_array, true);
}
+TEST_F(TestRecordBatch, MakeStatisticsArrayNestedType) {
+ auto struct_type = struct_({field("a", int64()), field("b", int64())});
Review Comment:
Could you add a comment that explains what record batch is building?
For example:
```cpp
// Schema:
// struct_a_b: struct {a: int64, b: int64}
// c: int64
// d: int64
// struct_copy: struct {a: int64, b: int64}
//
// Data:
// [
// {struct_a_b: {a: 1, b: 6}, c: 11, d: 16, struct_copy: {a: 1, b: 6}},
// ...,
// ]
//
// Statistics:
// struct_a_b:
// max: {a:
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2720399021 Thanks for your feedback. I'll review it and apply the suggested changes -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663:
URL: https://github.com/apache/arrow/pull/45663#issuecomment-2718449700
@kou When I run `IWYU` on record_batch.cc on `record_batch.cc` I got the
following message. Should I pay attention ton
```
/media/arashandishgar/7ea4b859-cfd7-41cb-8466-dc8dbc34284b/ARROW/arrow/cpp/iwyu$
$IWYU_SH match arrow/record_batch.cc
include-what-you-use 0.18 (git:abd5d2b) based on Ubuntu clang version 14.0.6
Running IWYU on
/media/arashandishgar/7ea4b859-cfd7-41cb-8466-dc8dbc34284b/ARROW/arrow/cpp/src/arrow/record_batch.cc
/media/arashandishgar/7ea4b859-cfd7-41cb-8466-dc8dbc34284b/ARROW/arrow/cpp/src/arrow/record_batch.h
should add these lines:
#include// for ptrdiff_t
#include // for input_iterator_tag
#include "arrow/util/compare.h" // for operator==
namespace arrow { class Array; }
namespace arrow { class Field; }
namespace arrow { class KeyValueMetadata; }
namespace arrow { class MemoryPool; }
namespace arrow { class Schema; }
namespace arrow { class StructArray; }
namespace arrow { class Table; }
namespace arrow { class Tensor; }
namespace arrow { struct ArrayData; }
namespace arrow { template class Iterator; }
namespace arrow { template struct IterationTraits; }
/media/arashandishgar/7ea4b859-cfd7-41cb-8466-dc8dbc34284b/ARROW/arrow/cpp/src/arrow/record_batch.h
should remove these lines:
- #include "arrow/util/iterator.h" // lines 30-30
/media/arashandishgar/7ea4b859-cfd7-41cb-8466-dc8dbc34284b/ARROW/arrow/cpp/src/arrow/record_batch.cc
should add these lines:
#include // for strlen
#include // for function
#include // for optional, nullopt
#include// for variant, visit
#include "arrow/array/array_base.h" // for Array
#include "arrow/array/array_nested.h"// for StructArray
#include "arrow/array/data.h"// for ArrayData
#include "arrow/array/statistics.h" // for ArrayStatistics,
ArrayStatis...
#include "arrow/array/util.h"// for MakeArray, MakeEmptyArray
#include "arrow/util/checked_cast.h" // for checked_pointer_cast
namespace arrow { class KeyValueMetadata; }
namespace arrow { class MemoryPool; }
namespace arrow { struct Scalar; }
/media/arashandishgar/7ea4b859-cfd7-41cb-8466-dc8dbc34284b/ARROW/arrow/cpp/src/arrow/record_batch.cc
should remove these lines:
- #include // lines 21-21
- #include // lines 22-22
- #include "arrow/array.h" // lines 29-29
- #include "arrow/visit_type_inline.h" // lines 45-45
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
kou commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2710213569 Could you rebase on main for MinGW? I'll push Python related changes to this branch later. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
kou commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2709052986 Python bindings: We need to update the followings: https://github.com/apache/arrow/blob/3c8fe098c7f5e0e40bd06bc6afca8412eb81f56e/python/pyarrow/array.pxi#L782-L802 https://github.com/apache/arrow/blob/3c8fe098c7f5e0e40bd06bc6afca8412eb81f56e/python/pyarrow/includes/libarrow.pxd#L104-L111 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
kou commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2709049425 MinGW: Lets' rebase this after we merge https://github.com/apache/arrow/pull/45697 . -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2709089072 it seems, there is no CI failure realated to my code. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2709072091 > Python bindings: We need to update the followings: > > https://github.com/apache/arrow/blob/3c8fe098c7f5e0e40bd06bc6afca8412eb81f56e/python/pyarrow/array.pxi#L782-L802 > > https://github.com/apache/arrow/blob/3c8fe098c7f5e0e40bd06bc6afca8412eb81f56e/python/pyarrow/includes/libarrow.pxd#L104-L111 Could I ask another person to work on the Python side? I do not have any experience in Cython. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2709071176 > It seems that AppVeyor failure is related to this change. Could you check it again? Sorry For this, It seems I should be far more cautious when I send a pull request. I will inform you When I am sure that my code pass CI. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
kou commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2709048999 OK. https://ci.appveyor.com/project/ApacheSoftwareFoundation/arrow/builds/51658917#L1627 ```text FAILED: src/arrow/CMakeFiles/arrow-table-test.dir/Unity/unity_0_cxx.cxx.obj C:\Mambaforge\envs\arrow\Library\bin\ccache.exe C:\PROGRA~2\MICROS~1\2019\COMMUN~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe /nologo /TP -DARROW_HAVE_RUNTIME_AVX2 -DARROW_HAVE_RUNTIME_AVX512 -DARROW_HAVE_RUNTIME_BMI2 -DARROW_HAVE_RUNTIME_SSE4_2 -DARROW_HAVE_SSE4_2 -DGTEST_LINKED_AS_SHARED_LIBRARY=1 -DPROTOBUF_USE_DLLS -DUSE_IMPORT_EXPORT -D_CRT_SECURE_NO_WARNINGS -IC:\projects\arrow\cpp\build\src -IC:\projects\arrow\cpp\src -IC:\projects\arrow\cpp\src\generated -external:IC:\projects\arrow\cpp\thirdparty\flatbuffers\include -external:IC:\Mambaforge\envs\arrow\Library\include -external:W0 /DWIN32 /D_WINDOWS /EHsc /D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING /EHsc /wd5105 /bigobj /utf-8 /W3 /wd4800 /wd4996 /wd4065 /WX /MP /MD /Od /UNDEBUG -std:c++17 -MD /showIncludes /Fosrc\arrow\CMakeFiles\arrow-table-test.dir\Unity\unity_0_cxx.cxx.obj /Fdsrc\arrow\CMakeFiles\arrow-table-test.dir\ /FS -c C:\projects\arrow\cpp\build\src\arrow\CMakeFiles\arrow-table-test.dir\Unity\unity_0_cx x.cxx C:/projects/arrow/cpp/src/arrow/record_batch_test.cc(1477): error C2679: binary '=': no operator found which takes a right-hand operand of type 'int' (or there is no acceptable conversion) C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\optional(442): note: could be 'std::optional &std::optional::operator =(std::optional &&)' C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\optional(442): note: or 'std::optional &std::optional::operator =(const std::optional &)' C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\optional(255): note: or 'std::optional &std::optional::operator =(std::nullopt_t) noexcept' C:/projects/arrow/cpp/src/arrow/record_batch_test.cc(1477): note: while trying to match the argument list '(std::optional, int)' C:/projects/arrow/cpp/src/arrow/record_batch_test.cc(1482): error C2679: binary '=': no operator found which takes a right-hand operand of type 'int' (or there is no acceptable conversion) C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\optional(442): note: could be 'std::optional &std::optional::operator =(std::optional &&)' C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\optional(442): note: or 'std::optional &std::optional::operator =(const std::optional &)' C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\optional(255): note: or 'std::optional &std::optional::operator =(std::nullopt_t) noexcept' C:/projects/arrow/cpp/src/arrow/record_batch_test.cc(1482): note: while trying to match the argument list '(std::optional, int)' C:/projects/arrow/cpp/src/arrow/record_batch_test.cc(1486): error C2679: binary '=': no operator found which takes a right-hand operand of type 'int' (or there is no acceptable conversion) C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\optional(442): note: could be 'std::optional &std::optional::operator =(std::optional &&)' C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\optional(442): note: or 'std::optional &std::optional::operator =(const std::optional &)' C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\optional(255): note: or 'std::optional &std::optional::operator =(std::nullopt_t) noexcept' C:/projects/arrow/cpp/src/arrow/record_batch_test.cc(1486): note: while trying to match the argument list '(std::optional, int)' C:/projects/arrow/cpp/src/arrow/record_batch_test.cc(1544): error C2664: 'arrow::Result> arrow::`anonymous-namespace'::MakeStatisticsArray(const std::string &,const std::vector>,std::allocator>>> &,const std::vector>,std::allocator>>> &)': cannot convert argument 3 from 'initializer list' to 'const std::vector>,std::allocator>>> &' C:/projects/arrow/cpp/src/arrow/record_batch_test.cc(1548): note: Reason: cannot convert from 'initializer list' to 'const std::vector>,std::allocator>>>' C:/projects/arrow/cpp/src/arrow/record_batch_test.cc(1548): note: No constructor could take the source type, or constructor overload resolution was ambiguous C:/projects/arrow/cpp/src/arrow/record_batch_test.cc(1097): note: see declaration of 'arrow::`anonymous-namespace'::MakeStatisticsArray' C:/projects/arrow/cpp/src/arrow/record_batch_test.cc(1544): error C2530: '_error_or_value116': referen
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
kou commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2709049168 It seems that AppVeyor failure is related to this change. Could you check it again? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2708770117 > > Could you check CI failures? > > For Python-related builds, it seems the problem originates from the new type I added (Unfortunately, I do not know anything about Python/c binding to resolve it). For MSVC the problem is rooted in implicit conversion. I'm going to resolve in my next commit. For MinGW, it seems the download has failed :). The problem regarding Msvc is solved. The MinGW problem is relatedd Gandiva which is not related My code. @kou should I do anything regarding Python binding? the failure is related to a new type that I added to `ArrayStatistics::ValueType` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
kou commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2708702868 Could you check CI failures? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2708723247 > Could you check CI failures? For Python-related builds, it seems the problem originates from the new type I added (Unfortunately, I do not know anything about Python/c binding to resolve it). For MSVC the problem is rooted in implicit conversion. I'm going to resolve my further commitment. For MinGW, it seems the download has failed :). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]
arashandishgar commented on PR #45663: URL: https://github.com/apache/arrow/pull/45663#issuecomment-2708356412 @kou could you review my pull request? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
