Re: [PR] GH-45474: [C++] Add support for building statistics array for nested types [arrow]

2025-12-03 Thread via GitHub


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]

2025-12-03 Thread via GitHub


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]

2025-04-05 Thread via GitHub


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]

2025-04-05 Thread via GitHub


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]

2025-04-03 Thread via GitHub


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]

2025-03-31 Thread via GitHub


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]

2025-03-30 Thread via GitHub


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]

2025-03-30 Thread via GitHub


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]

2025-03-30 Thread via GitHub


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]

2025-03-30 Thread via GitHub


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]

2025-03-30 Thread via GitHub


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]

2025-03-30 Thread via GitHub


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]

2025-03-13 Thread via GitHub


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]

2025-03-13 Thread via GitHub


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]

2025-03-12 Thread via GitHub


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]

2025-03-10 Thread via GitHub


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]

2025-03-09 Thread via GitHub


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]

2025-03-09 Thread via GitHub


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]

2025-03-09 Thread via GitHub


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]

2025-03-09 Thread via GitHub


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]

2025-03-09 Thread via GitHub


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]

2025-03-09 Thread via GitHub


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]

2025-03-09 Thread via GitHub


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]

2025-03-09 Thread via GitHub


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]

2025-03-09 Thread via GitHub


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]

2025-03-08 Thread via GitHub


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]

2025-03-08 Thread via GitHub


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]