You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "WillAyd (via GitHub)" <gi...@apache.org> on 2023/10/05 22:20:22 UTC

Re: [PR] feat(c/driver/postgresql): Floating point types for Copy Writer [arrow-adbc]

WillAyd commented on code in PR #1170:
URL: https://github.com/apache/arrow-adbc/pull/1170#discussion_r1348054883


##########
c/driver/postgresql/postgres_copy_reader.h:
##########
@@ -1164,9 +1164,20 @@ class PostgresCopyNetworkEndianFieldWriter : public PostgresCopyFieldWriter {
       return ADBC_STATUS_OK;
     }
 
-    const T value =
-        static_cast<T>(ArrowArrayViewGetIntUnsafe(array_view_, index)) - kOffset;
-    NANOARROW_RETURN_NOT_OK(WriteChecked<T>(buffer, value, error));
+    if constexpr (std::is_same<T, float>::value) {
+      uint32_t value;
+      std::memcpy(&value, &array_view_->buffer_views[1].data.as_float[index],
+                  sizeof(float));
+      NANOARROW_RETURN_NOT_OK(WriteChecked<uint32_t>(buffer, value, error));
+    } else if constexpr (std::is_same<T, double>::value) {
+      uint64_t value;
+      std::memcpy(&value, &array_view_->buffer_views[1].data.as_double[index],
+                  sizeof(double));
+      NANOARROW_RETURN_NOT_OK(WriteChecked<uint64_t>(buffer, value, error));
+    } else {
+      const T value = static_cast<T>(ArrowArrayViewGetIntUnsafe(array_view_, index));

Review Comment:
   Using the `ArrowArrayViewGet...` functions would require some kind of casting across int64, uint64 or double types and I don't _think_ these can preserve the bit patterns we need to write. The COPY reader only ever deals with raw bytes so it has different templating behavior than what is in this PR.
   
   We could alternately keep this class as is and implement a `PostgresCopyFloatWriter` class instead for float/double; the advantage of that is we wouldn't need to remove the `T kOffset = 0` template parameter that dates in theory should be able to use.
   
   Either case would be different from the reader, which operates using raw bytes so doesn't have problems with float <> integral casts



-- 
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