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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
