You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by li...@apache.org on 2022/11/28 21:16:50 UTC

[arrow-nanoarrow] branch main updated: [C] Fix AppendDouble to accept all values on float arrays (#77)

This is an automated email from the ASF dual-hosted git repository.

lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-nanoarrow.git


The following commit(s) were added to refs/heads/main by this push:
     new 3d9bcca  [C] Fix AppendDouble to accept all values on float arrays (#77)
3d9bcca is described below

commit 3d9bcca1e1effe1590437e8f8d9a77fef2ac4d59
Author: David Li <li...@gmail.com>
AuthorDate: Mon Nov 28 16:16:45 2022 -0500

    [C] Fix AppendDouble to accept all values on float arrays (#77)
    
    * [C] Fix AppendDouble to accept all values on float arrays
    
    Fixes #75.
    
    * Update src/nanoarrow/array_inline.h
    
    Co-authored-by: Dewey Dunnington <de...@fishandwhistle.net>
    
    Co-authored-by: Dewey Dunnington <de...@fishandwhistle.net>
---
 src/nanoarrow/array_inline.h |  3 +--
 src/nanoarrow/array_test.cc  | 58 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/src/nanoarrow/array_inline.h b/src/nanoarrow/array_inline.h
index 5a127b1..751955d 100644
--- a/src/nanoarrow/array_inline.h
+++ b/src/nanoarrow/array_inline.h
@@ -304,8 +304,7 @@ static inline ArrowErrorCode ArrowArrayAppendDouble(struct ArrowArray* array,
       NANOARROW_RETURN_NOT_OK(ArrowBufferAppend(data_buffer, &value, sizeof(double)));
       break;
     case NANOARROW_TYPE_FLOAT:
-      _NANOARROW_CHECK_RANGE(value, FLT_MIN, FLT_MAX);
-      NANOARROW_RETURN_NOT_OK(ArrowBufferAppendFloat(data_buffer, value));
+      NANOARROW_RETURN_NOT_OK(ArrowBufferAppendFloat(data_buffer, (float)value));
       break;
     default:
       return EINVAL;
diff --git a/src/nanoarrow/array_test.cc b/src/nanoarrow/array_test.cc
index ae1d0e9..2696a5e 100644
--- a/src/nanoarrow/array_test.cc
+++ b/src/nanoarrow/array_test.cc
@@ -16,12 +16,14 @@
 // under the License.
 
 #include <gtest/gtest.h>
+#include <math.h>
 
 #include <arrow/array.h>
 #include <arrow/array/builder_binary.h>
 #include <arrow/array/builder_nested.h>
 #include <arrow/array/builder_primitive.h>
 #include <arrow/c/bridge.h>
+#include <arrow/compare.h>
 
 #include "nanoarrow/nanoarrow.h"
 
@@ -553,18 +555,33 @@ TEST(ArrayTest, ArrayTestAppendToDoubleArray) {
   EXPECT_EQ(ArrowArrayAppendNull(&array, 2), NANOARROW_OK);
   EXPECT_EQ(ArrowArrayAppendUInt(&array, 3), NANOARROW_OK);
   EXPECT_EQ(ArrowArrayAppendDouble(&array, 3.14), NANOARROW_OK);
+  EXPECT_EQ(ArrowArrayAppendDouble(&array, std::numeric_limits<double>::max()),
+            NANOARROW_OK);
+  EXPECT_EQ(ArrowArrayAppendDouble(&array, NAN), NANOARROW_OK);
+  EXPECT_EQ(ArrowArrayAppendDouble(&array, INFINITY), NANOARROW_OK);
+  EXPECT_EQ(ArrowArrayAppendDouble(&array, -INFINITY), NANOARROW_OK);
+  EXPECT_EQ(ArrowArrayAppendDouble(&array, -1), NANOARROW_OK);
+  EXPECT_EQ(ArrowArrayAppendDouble(&array, 0), NANOARROW_OK);
   EXPECT_EQ(ArrowArrayFinishBuilding(&array, nullptr), NANOARROW_OK);
 
-  EXPECT_EQ(array.length, 5);
+  EXPECT_EQ(array.length, 11);
   EXPECT_EQ(array.null_count, 2);
   auto validity_buffer = reinterpret_cast<const uint8_t*>(array.buffers[0]);
   auto data_buffer = reinterpret_cast<const double*>(array.buffers[1]);
-  EXPECT_EQ(validity_buffer[0], 0x01 | 0x08 | 0x10);
+  EXPECT_EQ(validity_buffer[0], 0b11111001);
+  EXPECT_EQ(validity_buffer[1], 0b00000111);
   EXPECT_EQ(data_buffer[0], 1);
   EXPECT_EQ(data_buffer[1], 0);
   EXPECT_EQ(data_buffer[2], 0);
   EXPECT_EQ(data_buffer[3], 3);
   EXPECT_DOUBLE_EQ(data_buffer[4], 3.14);
+  EXPECT_FLOAT_EQ(data_buffer[4], 3.14);
+  EXPECT_FLOAT_EQ(data_buffer[5], std::numeric_limits<double>::max());
+  EXPECT_TRUE(std::isnan(data_buffer[6])) << data_buffer[6];
+  EXPECT_FLOAT_EQ(data_buffer[7], INFINITY);
+  EXPECT_FLOAT_EQ(data_buffer[8], -INFINITY);
+  EXPECT_FLOAT_EQ(data_buffer[9], -1);
+  EXPECT_FLOAT_EQ(data_buffer[10], 0);
 
   auto arrow_array = ImportArray(&array, float64());
   ARROW_EXPECT_OK(arrow_array);
@@ -574,9 +591,16 @@ TEST(ArrayTest, ArrayTestAppendToDoubleArray) {
   ARROW_EXPECT_OK(builder.AppendNulls(2));
   ARROW_EXPECT_OK(builder.Append(3));
   ARROW_EXPECT_OK(builder.Append(3.14));
+  ARROW_EXPECT_OK(builder.Append(std::numeric_limits<double>::max()));
+  ARROW_EXPECT_OK(builder.Append(NAN));
+  ARROW_EXPECT_OK(builder.Append(INFINITY));
+  ARROW_EXPECT_OK(builder.Append(-INFINITY));
+  ARROW_EXPECT_OK(builder.Append(-1));
+  ARROW_EXPECT_OK(builder.Append(0));
   auto expected_array = builder.Finish();
 
-  EXPECT_TRUE(arrow_array.ValueUnsafe()->Equals(expected_array.ValueUnsafe()));
+  auto options = arrow::EqualOptions::Defaults().nans_equal(true);
+  EXPECT_TRUE(arrow_array.ValueUnsafe()->Equals(expected_array.ValueUnsafe(), options));
 }
 
 TEST(ArrayTest, ArrayTestAppendToFloatArray) {
@@ -588,19 +612,32 @@ TEST(ArrayTest, ArrayTestAppendToFloatArray) {
   EXPECT_EQ(ArrowArrayAppendNull(&array, 2), NANOARROW_OK);
   EXPECT_EQ(ArrowArrayAppendUInt(&array, 3), NANOARROW_OK);
   EXPECT_EQ(ArrowArrayAppendDouble(&array, 3.14), NANOARROW_OK);
-  EXPECT_EQ(ArrowArrayAppendDouble(&array, std::numeric_limits<double>::max()), EINVAL);
+  EXPECT_EQ(ArrowArrayAppendDouble(&array, std::numeric_limits<double>::max()),
+            NANOARROW_OK);
+  EXPECT_EQ(ArrowArrayAppendDouble(&array, NAN), NANOARROW_OK);
+  EXPECT_EQ(ArrowArrayAppendDouble(&array, INFINITY), NANOARROW_OK);
+  EXPECT_EQ(ArrowArrayAppendDouble(&array, -INFINITY), NANOARROW_OK);
+  EXPECT_EQ(ArrowArrayAppendDouble(&array, -1), NANOARROW_OK);
+  EXPECT_EQ(ArrowArrayAppendDouble(&array, 0), NANOARROW_OK);
   EXPECT_EQ(ArrowArrayFinishBuilding(&array, nullptr), NANOARROW_OK);
 
-  EXPECT_EQ(array.length, 5);
+  EXPECT_EQ(array.length, 11);
   EXPECT_EQ(array.null_count, 2);
   auto validity_buffer = reinterpret_cast<const uint8_t*>(array.buffers[0]);
   auto data_buffer = reinterpret_cast<const float*>(array.buffers[1]);
-  EXPECT_EQ(validity_buffer[0], 0x01 | 0x08 | 0x10);
+  EXPECT_EQ(validity_buffer[0], 0b11111001);
+  EXPECT_EQ(validity_buffer[1], 0b00000111);
   EXPECT_EQ(data_buffer[0], 1);
   EXPECT_EQ(data_buffer[1], 0);
   EXPECT_EQ(data_buffer[2], 0);
   EXPECT_EQ(data_buffer[3], 3);
   EXPECT_FLOAT_EQ(data_buffer[4], 3.14);
+  EXPECT_FLOAT_EQ(data_buffer[5], std::numeric_limits<float>::max());
+  EXPECT_TRUE(std::isnan(data_buffer[6])) << data_buffer[6];
+  EXPECT_FLOAT_EQ(data_buffer[7], INFINITY);
+  EXPECT_FLOAT_EQ(data_buffer[8], -INFINITY);
+  EXPECT_FLOAT_EQ(data_buffer[9], -1);
+  EXPECT_FLOAT_EQ(data_buffer[10], 0);
 
   auto arrow_array = ImportArray(&array, float32());
   ARROW_EXPECT_OK(arrow_array);
@@ -610,9 +647,16 @@ TEST(ArrayTest, ArrayTestAppendToFloatArray) {
   ARROW_EXPECT_OK(builder.AppendNulls(2));
   ARROW_EXPECT_OK(builder.Append(3));
   ARROW_EXPECT_OK(builder.Append(3.14));
+  ARROW_EXPECT_OK(builder.Append(std::numeric_limits<double>::max()));
+  ARROW_EXPECT_OK(builder.Append(NAN));
+  ARROW_EXPECT_OK(builder.Append(INFINITY));
+  ARROW_EXPECT_OK(builder.Append(-INFINITY));
+  ARROW_EXPECT_OK(builder.Append(-1));
+  ARROW_EXPECT_OK(builder.Append(0));
   auto expected_array = builder.Finish();
 
-  EXPECT_TRUE(arrow_array.ValueUnsafe()->Equals(expected_array.ValueUnsafe()));
+  auto options = arrow::EqualOptions::Defaults().nans_equal(true);
+  EXPECT_TRUE(arrow_array.ValueUnsafe()->Equals(expected_array.ValueUnsafe(), options));
 }
 
 TEST(ArrayTest, ArrayTestAppendToBoolArray) {