You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "sgilmore10 (via GitHub)" <gi...@apache.org> on 2023/09/01 14:36:55 UTC

[GitHub] [arrow] sgilmore10 opened a new issue, #37515: [C++] `ChunkedArray::Equals()` treats NaNs as equal by default

sgilmore10 opened a new issue, #37515:
URL: https://github.com/apache/arrow/issues/37515

   ### Describe the bug, including details regarding any error messages, version, and platform.
   
   While working on #37448, I noticed the `Equals()` method of `ChunkedArray` treats NaNs as equal by default. I found this behavior surprising because the `Equals()` method of `Array` doesn't treat NaNs as equal by default.
   
   Below is a program to reproduce this behavior: 
   
   ```c++
   #include <arrow/api.h>
   #include <arrow/type.h>
   
   #include <iostream>
   #include <math.h>
   #include <sstream>
   
   arrow::Result<std::shared_ptr<arrow::ChunkedArray>> make_chunked_array() {
       arrow::NumericBuilder<arrow::DoubleType> builder;
       
       std::shared_ptr<arrow::Array> array;
       ARROW_RETURN_NOT_OK(builder.AppendValues({0, 1, NAN, 2, 4}));
       ARROW_RETURN_NOT_OK(builder.Finish(&array));
       
       return arrow::ChunkedArray::Make({array});
   }
   
   int main(int argc, char *argv[])
   {
       auto maybe_chunked_array = make_chunked_array();
       if (!maybe_chunked_array.ok()) {
           return -1;
       }
       auto chunked_array = std::move(maybe_chunked_array).ValueUnsafe();
       auto array = chunked_array->chunk(0);
       
       std::stringstream stream;
       stream << "array contents: ";
       stream << "\n\n";
       stream << array->ToString();
       stream << "\n\n";
       stream << "array->Equals(array): ";
       stream << array->Equals(array);
       stream << "\n\n";
   
       stream << "chunked_array contents: ";
       stream << "\n\n";
       stream << chunked_array->ToString();
       stream << "\n\n";
       stream << "chunked_array->Equals(chunked_array): ";
       stream << chunked_array->Equals(chunked_array);
       
       std::cout << stream.str() << std::endl;
   }
   ```
   
   Here is the output of this program:
   
   ```shell
   array contents: 
   
   [
     0,
     1,
     nan,
     2,
     4
   ]
   
   array->Equals(array): 0
   
   chunked_array contents: 
   
   [
     [
       0,
       1,
       nan,
       2,
       4
     ]
   ]
   
   chunked_array->Equals(chunked_array): 1
   ```
   
   Am I correct in that this is a bug?
   
   ### Component(s)
   
   C++


-- 
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: issues-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] sgilmore10 commented on issue #37515: [C++] `ChunkedArray::Equals()` treats NaNs as equal by default

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on issue #37515:
URL: https://github.com/apache/arrow/issues/37515#issuecomment-1706602891

   take


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] sgilmore10 commented on issue #37515: [C++] `ChunkedArray::Equals()` treats NaNs as equal by default

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on issue #37515:
URL: https://github.com/apache/arrow/issues/37515#issuecomment-1702894110

   Ah, thanks for pointing that out! It's a bit weird that `Equals(ChunkedArray& other)` and `Equals(std::shared_ptr<ChunkedArray>& other)` may return different results.  For example, if you modified the program above to also call `Equals(ChunkedArray& other)`, this would be the output:
   
   ```shell
   chunked_array->Equals(chunked_array): 1
   chunked_array->Equals(*chunked_array): 0
   ```
   
   I wonder if it's worth removing this optimization for consistency?


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] kou closed issue #37515: [C++] In some cases `ChunkedArray::Equals(std::shared_ptr& other)` does not treat `NaN` values as unequal consistently

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou closed issue #37515: [C++] In some cases `ChunkedArray::Equals(std::shared_ptr<arrow::ChunkedArray>& other)` does not treat `NaN` values as unequal consistently
URL: https://github.com/apache/arrow/issues/37515


-- 
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: issues-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] js8544 commented on issue #37515: [C++] `ChunkedArray::Equals()` treats NaNs as equal by default

Posted by "js8544 (via GitHub)" <gi...@apache.org>.
js8544 commented on issue #37515:
URL: https://github.com/apache/arrow/issues/37515#issuecomment-1702871220

   It doesn't treat NaNs as equal. But it does assumes that chunked arrays at the same address are equal, where `Array` doesn't have such assumption. See https://github.com/apache/arrow/blob/main/cpp/src/arrow/chunked_array.cc#L115


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] js8544 commented on issue #37515: [C++] `ChunkedArray::Equals()` treats NaNs as equal by default

Posted by "js8544 (via GitHub)" <gi...@apache.org>.
js8544 commented on issue #37515:
URL: https://github.com/apache/arrow/issues/37515#issuecomment-1704554967

   I agree that this optimization can be removed for the sake of consistency.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [arrow] sgilmore10 commented on issue #37515: [C++] `ChunkedArray::Equals()` treats NaNs as equal by default

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on issue #37515:
URL: https://github.com/apache/arrow/issues/37515#issuecomment-1706602791

   I'll work on this.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org