You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/01/21 18:06:03 UTC
[GitHub] [arrow] pitrou commented on a change in pull request #7507: ARROW-8797: [C++] Read RecordBatch in a different endian
pitrou commented on a change in pull request #7507:
URL: https://github.com/apache/arrow/pull/7507#discussion_r562063757
##########
File path: cpp/src/arrow/array/util.h
##########
@@ -56,6 +56,14 @@ Result<std::shared_ptr<Array>> MakeArrayFromScalar(
namespace internal {
+/// \brief Swap endian of each element in a generic ArrayData
+/// \param[in] data the array contents
+/// \param[in] type the array type
+/// \return the resulting Array instance whose elements were swapped
+ARROW_EXPORT
+Result<std::shared_ptr<ArrayData>> SwapEndianArrayData(
+ std::shared_ptr<ArrayData>& data, const std::shared_ptr<DataType>& type);
Review comment:
The first parameter should be `const` as well, it seems?
##########
File path: cpp/src/arrow/array/util.h
##########
@@ -56,6 +56,14 @@ Result<std::shared_ptr<Array>> MakeArrayFromScalar(
namespace internal {
+/// \brief Swap endian of each element in a generic ArrayData
+/// \param[in] data the array contents
+/// \param[in] type the array type
Review comment:
The type is already on the data, is it necessary to pass it separately?
##########
File path: dev/archery/tests/generate_files_for_endian_test.sh
##########
@@ -0,0 +1,28 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+: ${ARROW_DIR:=/arrow}
Review comment:
Can you add a comment explaining how to use this script?
##########
File path: cpp/src/arrow/type.h
##########
@@ -1641,6 +1655,17 @@ class ARROW_EXPORT Schema : public detail::Fingerprintable,
bool Equals(const Schema& other, bool check_metadata = false) const;
bool Equals(const std::shared_ptr<Schema>& other, bool check_metadata = false) const;
+ /// \brief Replace endianness with platform-native endianness in the schema
+ ///
+ /// \return new Schema
+ std::shared_ptr<Schema> WithNativeEndianness() const;
+
+ /// \brief Return endianness in the schema
+ Endianness endianness() const;
+
+ /// \brief Indicate if endianness is equal to platform-native endianness
+ bool IsNativeEndianness() const;
Review comment:
I would call it `is_native_endian()`.
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
Review comment:
And where is the null bitmap handled? Did I miss something?
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
Review comment:
Either `const&` or no reference at all.
##########
File path: ci/scripts/integration_arrow.sh
##########
@@ -24,9 +24,16 @@ source_dir=${1}/cpp
build_dir=${2}/cpp
gold_dir_0_14_1=$arrow_dir/testing/data/arrow-ipc-stream/integration/0.14.1
gold_dir_0_17_1=$arrow_dir/testing/data/arrow-ipc-stream/integration/0.17.1
+gold_dir_1_0_0_le=$arrow_dir/testing/data/arrow-ipc-stream/integration/1.0.0-littleendian
+gold_dir_1_0_0_be=$arrow_dir/testing/data/arrow-ipc-stream/integration/1.0.0-bigendian
Review comment:
Sidenote: I see that "generated_large_batch" was added to these directories. Is it necessary?
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
Review comment:
By the way, what happens if `data_.offset` is non-zero? Is it supported? If not, should probably return an error.
##########
File path: cpp/src/arrow/ipc/reader.h
##########
@@ -169,6 +172,9 @@ class ARROW_EXPORT RecordBatchFileReader {
/// \brief Return current read statistics
virtual ReadStats stats() const = 0;
+
+ protected:
+ bool swap_endian_;
Review comment:
Same here.
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
+ int64_t length, int64_t extra_size) {
+ auto in_data = reinterpret_cast<const T*>(in_buffer->data());
+ ARROW_ASSIGN_OR_RAISE(auto out_buffer, AllocateBuffer(in_buffer->size()));
+ auto out_data = reinterpret_cast<T*>(out_buffer->mutable_data());
+ for (int64_t i = 0; i < length + extra_size; i++) {
+#if ARROW_LITTLE_ENDIAN
+ out_data[i] = BitUtil::FromBigEndian(in_data[i]);
+#else
+ out_data[i] = BitUtil::FromLittleEndian(in_data[i]);
+#endif
+ }
+ return std::move(out_buffer);
+ }
+
+ template <typename VALUE_TYPE>
+ Status SwapOffset(int index) {
Review comment:
`SwapOffsets` would be better.
##########
File path: cpp/src/arrow/ipc/reader.h
##########
@@ -96,6 +96,9 @@ class ARROW_EXPORT RecordBatchStreamReader : public RecordBatchReader {
/// \brief Return current read statistics
virtual ReadStats stats() const = 0;
+
+ protected:
+ bool swap_endian_;
Review comment:
It doesn't seem necessary to put this in `RecordBatchStreamReader`, you can move it to the implementation class.
##########
File path: cpp/src/arrow/type.h
##########
@@ -1623,13 +1624,26 @@ class ARROW_EXPORT FieldRef {
// ----------------------------------------------------------------------
// Schema
+enum class Endianness {
+ LITTLE = 0,
+ BIG = 1,
+#if ARROW_LITTLE_ENDIAN
+ NATIVE = LITTLE
+#else
+ NATIVE = BIG
Review comment:
Nit, but to avoid issues with third-party macros, I would use `Little`, `Big`, `Native`.
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
+ int64_t length, int64_t extra_size) {
+ auto in_data = reinterpret_cast<const T*>(in_buffer->data());
+ ARROW_ASSIGN_OR_RAISE(auto out_buffer, AllocateBuffer(in_buffer->size()));
+ auto out_data = reinterpret_cast<T*>(out_buffer->mutable_data());
+ for (int64_t i = 0; i < length + extra_size; i++) {
+#if ARROW_LITTLE_ENDIAN
+ out_data[i] = BitUtil::FromBigEndian(in_data[i]);
+#else
+ out_data[i] = BitUtil::FromLittleEndian(in_data[i]);
+#endif
+ }
+ return std::move(out_buffer);
+ }
+
+ template <typename VALUE_TYPE>
+ Status SwapOffset(int index) {
+ if (data_->buffers[index] == nullptr || data_->buffers[index]->size() == 0) {
+ (*out_)->buffers[index] = data_->buffers[index];
+ return Status::OK();
+ }
+ // offset has one more element rather than data->length
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[index],
+ ByteSwapBuffer<VALUE_TYPE>(data_->buffers[index], length_, 1));
+ return Status::OK();
+ }
+
+ Status SwapSmallOffset(int index = 1) { return SwapOffset<int32_t>(index); }
+
+ Status SwapLargeOffset() { return SwapOffset<int64_t>(1); }
+
+ template <typename T>
+ enable_if_t<std::is_base_of<FixedWidthType, T>::value &&
+ !std::is_base_of<FixedSizeBinaryType, T>::value &&
+ !std::is_base_of<DictionaryType, T>::value,
+ Status>
+ Visit(const T& type) {
+ using value_type = typename T::c_type;
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ ByteSwapBuffer<value_type>(data_->buffers[1], length_, 0));
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal128Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp;
+ auto idx = i * 2;
+#if ARROW_LITTLE_ENDIAN
+ tmp = BitUtil::FromBigEndian(data[idx]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#else
+ tmp = BitUtil::FromLittleEndian(data[idx]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#endif
+ }
+ (*out_)->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal256Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp0, tmp1, tmp2;
+ auto idx = i * 4;
+#if ARROW_LITTLE_ENDIAN
+ tmp0 = BitUtil::FromBigEndian(data[idx]);
+ tmp1 = BitUtil::FromBigEndian(data[idx + 1]);
+ tmp2 = BitUtil::FromBigEndian(data[idx + 2]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 3]);
+ new_data[idx + 1] = tmp2;
+ new_data[idx + 2] = tmp1;
+ new_data[idx + 3] = tmp0;
+#else
+ tmp0 = BitUtil::FromLittleEndian(data[idx]);
+ tmp1 = BitUtil::FromLittleEndian(data[idx + 1]);
+ tmp2 = BitUtil::FromLittleEndian(data[idx + 2]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 3]);
+ new_data[idx + 1] = tmp2;
+ new_data[idx + 2] = tmp1;
+ new_data[idx + 3] = tmp0;
+#endif
+ }
+ (*out_)->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const DayTimeIntervalType& type) {
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ ByteSwapBuffer<uint32_t>(data_->buffers[1], length_ * 2, 0));
+ return Status::OK();
+ }
+
+ Status CopyDataBuffer() {
+ if (data_->buffers[1]->data() == nullptr) {
+ return Status::OK();
+ }
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ data_->buffers[1]->CopySlice(0, data_->buffers[1]->size()));
+ return Status::OK();
+ }
+
+ Status Visit(const NullType& type) { return Status::OK(); }
+ Status Visit(const BooleanType& type) { return CopyDataBuffer(); }
+ Status Visit(const Int8Type& type) { return CopyDataBuffer(); }
+ Status Visit(const UInt8Type& type) { return CopyDataBuffer(); }
+ Status Visit(const FixedSizeBinaryType& type) { return CopyDataBuffer(); }
+ Status Visit(const FixedSizeListType& type) { return Status::OK(); }
+ Status Visit(const StructType& type) { return Status::OK(); }
+ Status Visit(const UnionType& type) {
+ (*out_)->buffers[1] = data_->buffers[1];
+ if (type.mode() == UnionMode::DENSE) {
+ RETURN_NOT_OK(SwapSmallOffset(2));
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ enable_if_t<std::is_same<BinaryType, T>::value || std::is_same<StringType, T>::value,
+ Status>
+ Visit(const T& type) {
+ RETURN_NOT_OK(SwapSmallOffset());
+ (*out_)->buffers[2] = data_->buffers[2];
+ return Status::OK();
+ }
+
+ template <typename T>
+ enable_if_t<std::is_same<LargeBinaryType, T>::value ||
+ std::is_same<LargeStringType, T>::value,
+ Status>
+ Visit(const T& type) {
+ RETURN_NOT_OK(SwapLargeOffset());
+ (*out_)->buffers[2] = data_->buffers[2];
+ return Status::OK();
+ }
+
+ Status Visit(const ListType& type) {
+ RETURN_NOT_OK(SwapSmallOffset());
+ return Status::OK();
+ }
+ Status Visit(const LargeListType& type) {
+ RETURN_NOT_OK(SwapLargeOffset());
+ return Status::OK();
+ }
+
+ Status Visit(const MapType& type) {
+ RETURN_NOT_OK(SwapSmallOffset());
+ return Status::OK();
+ }
+
+ Status Visit(const DictionaryType& type) {
+ RETURN_NOT_OK(SwapType(*type.index_type()));
+ (*out_)->dictionary = data_->dictionary;
+ return Status::OK();
+ }
+
+ Status Visit(const ExtensionType& type) {
+ RETURN_NOT_OK(SwapType(*type.storage_type()));
+ (*out_)->dictionary = data_->dictionary;
+ return Status::OK();
+ }
+
+ std::shared_ptr<ArrayData>& data_;
+ int64_t length_;
+ std::shared_ptr<ArrayData>* out_;
+};
+
+Result<std::shared_ptr<ArrayData>> SwapEndianArrayData(
+ std::shared_ptr<ArrayData>& data, const std::shared_ptr<DataType>& type) {
+ std::vector<std::shared_ptr<Buffer>> buffers(data->buffers.size(), nullptr);
+ std::vector<std::shared_ptr<ArrayData>> child_data(data->child_data.size(), nullptr);
+ std::shared_ptr<ArrayData> out =
+ ArrayData::Make(type, data->length, buffers, child_data, data->null_count, 0);
Review comment:
I think it would make the code more regular to create the buffers and child_data inside `ArrayDataEndianSwapper` (since you also need to do it for child fields recursively)
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
Review comment:
Instead of taking the result `ArrayData` as an argument, you can probably create it in this class and return it to the caller.
##########
File path: cpp/src/arrow/array/util.h
##########
@@ -56,6 +56,14 @@ Result<std::shared_ptr<Array>> MakeArrayFromScalar(
namespace internal {
+/// \brief Swap endian of each element in a generic ArrayData
+/// \param[in] data the array contents
+/// \param[in] type the array type
+/// \return the resulting Array instance whose elements were swapped
+ARROW_EXPORT
+Result<std::shared_ptr<ArrayData>> SwapEndianArrayData(
+ std::shared_ptr<ArrayData>& data, const std::shared_ptr<DataType>& type);
+
/// Given a number of ArrayVectors, treat each ArrayVector as the
Review comment:
Hmm... did I miss the tests for `SwapEndianArrayData` or didn't you add any?
I would expect some simple basic tests, at least to exercise nulls, numbers, structs, lists, decimals...
##########
File path: cpp/src/arrow/type.cc
##########
@@ -1352,6 +1369,11 @@ bool Schema::Equals(const Schema& other, bool check_metadata) const {
return true;
}
+ // checks endianness equality
+ if (endianness() != other.endianness()) {
+ return false;
+ }
Review comment:
Can you add equality tests to `type_test.cc`?
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
+ int64_t length, int64_t extra_size) {
+ auto in_data = reinterpret_cast<const T*>(in_buffer->data());
+ ARROW_ASSIGN_OR_RAISE(auto out_buffer, AllocateBuffer(in_buffer->size()));
Review comment:
One other thing: if `sizeof(T) == 1`, then we can simply reuse the existing buffer.
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
+ int64_t length, int64_t extra_size) {
+ auto in_data = reinterpret_cast<const T*>(in_buffer->data());
+ ARROW_ASSIGN_OR_RAISE(auto out_buffer, AllocateBuffer(in_buffer->size()));
+ auto out_data = reinterpret_cast<T*>(out_buffer->mutable_data());
+ for (int64_t i = 0; i < length + extra_size; i++) {
Review comment:
Hmm... are we sure `length + extra_size` and `in_buffer->size()` are consistent?
I would expect either:
* byte-swap `length` entries
* `memset()` the remaining bytes to 0
... or to allocate exactly the required size (i.e. `length * sizeof(T)`)
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
+ int64_t length, int64_t extra_size) {
+ auto in_data = reinterpret_cast<const T*>(in_buffer->data());
+ ARROW_ASSIGN_OR_RAISE(auto out_buffer, AllocateBuffer(in_buffer->size()));
+ auto out_data = reinterpret_cast<T*>(out_buffer->mutable_data());
+ for (int64_t i = 0; i < length + extra_size; i++) {
+#if ARROW_LITTLE_ENDIAN
+ out_data[i] = BitUtil::FromBigEndian(in_data[i]);
+#else
+ out_data[i] = BitUtil::FromLittleEndian(in_data[i]);
+#endif
+ }
+ return std::move(out_buffer);
+ }
+
+ template <typename VALUE_TYPE>
+ Status SwapOffset(int index) {
+ if (data_->buffers[index] == nullptr || data_->buffers[index]->size() == 0) {
+ (*out_)->buffers[index] = data_->buffers[index];
+ return Status::OK();
+ }
+ // offset has one more element rather than data->length
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[index],
+ ByteSwapBuffer<VALUE_TYPE>(data_->buffers[index], length_, 1));
+ return Status::OK();
+ }
+
+ Status SwapSmallOffset(int index = 1) { return SwapOffset<int32_t>(index); }
+
+ Status SwapLargeOffset() { return SwapOffset<int64_t>(1); }
+
+ template <typename T>
+ enable_if_t<std::is_base_of<FixedWidthType, T>::value &&
+ !std::is_base_of<FixedSizeBinaryType, T>::value &&
+ !std::is_base_of<DictionaryType, T>::value,
+ Status>
+ Visit(const T& type) {
+ using value_type = typename T::c_type;
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ ByteSwapBuffer<value_type>(data_->buffers[1], length_, 0));
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal128Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp;
+ auto idx = i * 2;
+#if ARROW_LITTLE_ENDIAN
+ tmp = BitUtil::FromBigEndian(data[idx]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#else
+ tmp = BitUtil::FromLittleEndian(data[idx]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#endif
+ }
+ (*out_)->buffers[1] = std::move(new_buffer);
Review comment:
We should also `memset` the excess bytes to 0. We probably want to factor the buffer allocation out.
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
Review comment:
Since this is a non-exposed class, can you put it in the anonymous namespace?
(you can do the same for `ArrayDataWrapper` at the same time :-))
##########
File path: ci/scripts/integration_arrow.sh
##########
@@ -24,9 +24,16 @@ source_dir=${1}/cpp
build_dir=${2}/cpp
gold_dir_0_14_1=$arrow_dir/testing/data/arrow-ipc-stream/integration/0.14.1
gold_dir_0_17_1=$arrow_dir/testing/data/arrow-ipc-stream/integration/0.17.1
+gold_dir_1_0_0_le=$arrow_dir/testing/data/arrow-ipc-stream/integration/1.0.0-littleendian
+gold_dir_1_0_0_be=$arrow_dir/testing/data/arrow-ipc-stream/integration/1.0.0-bigendian
pip install -e $arrow_dir/dev/archery
archery integration --with-all --run-flight \
--gold-dirs=$gold_dir_0_14_1 \
--gold-dirs=$gold_dir_0_17_1 \
+
+# TODO: support other languages
+archery integration --with-cpp=1 --run-flight \
Review comment:
You mean even the little-endian cases fail on Java?
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
Review comment:
Pass a const reference here to avoid copying the vector.
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
+ int64_t length, int64_t extra_size) {
+ auto in_data = reinterpret_cast<const T*>(in_buffer->data());
+ ARROW_ASSIGN_OR_RAISE(auto out_buffer, AllocateBuffer(in_buffer->size()));
+ auto out_data = reinterpret_cast<T*>(out_buffer->mutable_data());
+ for (int64_t i = 0; i < length + extra_size; i++) {
+#if ARROW_LITTLE_ENDIAN
+ out_data[i] = BitUtil::FromBigEndian(in_data[i]);
+#else
+ out_data[i] = BitUtil::FromLittleEndian(in_data[i]);
+#endif
+ }
+ return std::move(out_buffer);
+ }
+
+ template <typename VALUE_TYPE>
+ Status SwapOffset(int index) {
+ if (data_->buffers[index] == nullptr || data_->buffers[index]->size() == 0) {
+ (*out_)->buffers[index] = data_->buffers[index];
+ return Status::OK();
+ }
+ // offset has one more element rather than data->length
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[index],
+ ByteSwapBuffer<VALUE_TYPE>(data_->buffers[index], length_, 1));
+ return Status::OK();
+ }
+
+ Status SwapSmallOffset(int index = 1) { return SwapOffset<int32_t>(index); }
+
+ Status SwapLargeOffset() { return SwapOffset<int64_t>(1); }
Review comment:
I'm frankly not sure those two mini-methods are useful?
##########
File path: cpp/src/arrow/type.cc
##########
@@ -1352,6 +1369,11 @@ bool Schema::Equals(const Schema& other, bool check_metadata) const {
return true;
}
+ // checks endianness equality
+ if (endianness() != other.endianness()) {
+ return false;
+ }
Review comment:
Also, you'll need to fix fingerprint computation.
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,204 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length)
+ : data_(data), length_(length) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ArrayDataEndianSwapper swapper_child_visitor(data_->child_data[i],
+ data_->child_data[i]->length);
+ RETURN_NOT_OK(VisitTypeInline(*child_field.get()->type(), &swapper_child_visitor));
+ RETURN_NOT_OK(
+ swapper_child_visitor.SwapChildren((*child_field.get()->type()).fields()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename VALUE_TYPE>
+ Status SwapOffset(int index) {
+ if (data_->buffers[index] == nullptr) {
+ return Status::OK();
+ }
+ auto data = reinterpret_cast<const VALUE_TYPE*>(data_->buffers[index]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer,
+ AllocateBuffer(data_->buffers[index]->size() + 1));
+ auto new_data = reinterpret_cast<VALUE_TYPE*>(new_buffer->mutable_data());
+ // offset has one more element rather than data->length
+ int64_t length = length_ + 1;
+ for (int64_t i = 0; i < length; i++) {
+#if ARROW_LITTLE_ENDIAN
+ new_data[i] = BitUtil::FromBigEndian(data[i]);
+#else
+ new_data[i] = BitUtil::FromLittleEndian(data[i]);
+#endif
+ }
+ data_->buffers[index] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status SwapSmallOffset(int index = 1) { return SwapOffset<int32_t>(index); }
+
+ Status SwapLargeOffset() { return SwapOffset<int64_t>(1); }
+
+ template <typename T>
+ Status Visit(const T&) {
+ using value_type = typename T::c_type;
+ auto data = reinterpret_cast<const value_type*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<value_type*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+#if ARROW_LITTLE_ENDIAN
+ new_data[i] = BitUtil::FromBigEndian(data[i]);
+#else
+ new_data[i] = BitUtil::FromLittleEndian(data[i]);
+#endif
+ }
+ data_->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal128Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp;
+ auto idx = i * 2;
+#if ARROW_LITTLE_ENDIAN
+ tmp = BitUtil::FromBigEndian(data[idx]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#else
+ tmp = BitUtil::FromLittleEndian(data[idx]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#endif
+ }
+ data_->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal256Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp0, tmp1, tmp2;
+ auto idx = i * 4;
+#if ARROW_LITTLE_ENDIAN
+ tmp0 = BitUtil::FromBigEndian(data[idx]);
+ tmp1 = BitUtil::FromBigEndian(data[idx + 1]);
+ tmp2 = BitUtil::FromBigEndian(data[idx + 2]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 3]);
+ new_data[idx + 1] = tmp2;
+ new_data[idx + 2] = tmp1;
+ new_data[idx + 3] = tmp0;
+#else
+ tmp0 = BitUtil::FromLittleEndian(data[idx]);
+ tmp1 = BitUtil::FromLittleEndian(data[idx + 1]);
+ tmp2 = BitUtil::FromLittleEndian(data[idx + 2]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 3]);
+ new_data[idx + 1] = tmp2;
+ new_data[idx + 2] = tmp1;
+ new_data[idx + 3] = tmp0;
+#endif
+ }
+ data_->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const DayTimeIntervalType& type) {
+ auto data = reinterpret_cast<const uint32_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint32_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ auto idx = i * 2;
+#if ARROW_LITTLE_ENDIAN
+ new_data[idx] = BitUtil::FromBigEndian(data[idx]);
+ new_data[idx + 1] = BitUtil::FromBigEndian(data[idx + 1]);
+#else
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx]);
+ new_data[idx + 1] = BitUtil::FromLittleEndian(data[idx + 1]);
+#endif
+ }
+ data_->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const NullType& type) { return Status::OK(); }
+ Status Visit(const BooleanType& type) { return Status::OK(); }
+ Status Visit(const Int8Type& type) { return Status::OK(); }
+ Status Visit(const UInt8Type& type) { return Status::OK(); }
+ Status Visit(const FixedSizeBinaryType& type) { return Status::OK(); }
+ Status Visit(const FixedSizeListType& type) { return Status::OK(); }
+ Status Visit(const StructType& type) { return Status::OK(); }
+ Status Visit(const SparseUnionType& type) { return Status::OK(); }
+
+ Status Visit(const StringType& type) {
+ RETURN_NOT_OK(SwapSmallOffset());
+ return Status::OK();
+ }
+ Status Visit(const LargeStringType& type) {
+ RETURN_NOT_OK(SwapLargeOffset());
+ return Status::OK();
+ }
+ Status Visit(const BinaryType& type) {
+ RETURN_NOT_OK(SwapSmallOffset());
+ return Status::OK();
+ }
+ Status Visit(const LargeBinaryType& type) {
+ RETURN_NOT_OK(SwapLargeOffset());
+ return Status::OK();
+ }
+
+ Status Visit(const ListType& type) {
+ RETURN_NOT_OK(SwapSmallOffset());
+ return Status::OK();
+ }
+ Status Visit(const LargeListType& type) {
+ RETURN_NOT_OK(SwapLargeOffset());
+ return Status::OK();
+ }
+
+ Status Visit(const MapType& type) {
Review comment:
`MapType` is a subclass of `ListType`, so you don't need a specific visitor since it's the same code :-)
##########
File path: cpp/src/arrow/type.h
##########
@@ -1641,6 +1655,17 @@ class ARROW_EXPORT Schema : public detail::Fingerprintable,
bool Equals(const Schema& other, bool check_metadata = false) const;
bool Equals(const std::shared_ptr<Schema>& other, bool check_metadata = false) const;
+ /// \brief Replace endianness with platform-native endianness in the schema
+ ///
+ /// \return new Schema
+ std::shared_ptr<Schema> WithNativeEndianness() const;
Review comment:
Wouldn't `WithEndianness(Endianness) const` be more generally useful?
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
+ int64_t length, int64_t extra_size) {
+ auto in_data = reinterpret_cast<const T*>(in_buffer->data());
+ ARROW_ASSIGN_OR_RAISE(auto out_buffer, AllocateBuffer(in_buffer->size()));
+ auto out_data = reinterpret_cast<T*>(out_buffer->mutable_data());
+ for (int64_t i = 0; i < length + extra_size; i++) {
+#if ARROW_LITTLE_ENDIAN
+ out_data[i] = BitUtil::FromBigEndian(in_data[i]);
+#else
+ out_data[i] = BitUtil::FromLittleEndian(in_data[i]);
+#endif
+ }
+ return std::move(out_buffer);
+ }
+
+ template <typename VALUE_TYPE>
+ Status SwapOffset(int index) {
+ if (data_->buffers[index] == nullptr || data_->buffers[index]->size() == 0) {
+ (*out_)->buffers[index] = data_->buffers[index];
+ return Status::OK();
+ }
+ // offset has one more element rather than data->length
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[index],
+ ByteSwapBuffer<VALUE_TYPE>(data_->buffers[index], length_, 1));
+ return Status::OK();
+ }
+
+ Status SwapSmallOffset(int index = 1) { return SwapOffset<int32_t>(index); }
+
+ Status SwapLargeOffset() { return SwapOffset<int64_t>(1); }
+
+ template <typename T>
+ enable_if_t<std::is_base_of<FixedWidthType, T>::value &&
+ !std::is_base_of<FixedSizeBinaryType, T>::value &&
+ !std::is_base_of<DictionaryType, T>::value,
+ Status>
+ Visit(const T& type) {
+ using value_type = typename T::c_type;
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ ByteSwapBuffer<value_type>(data_->buffers[1], length_, 0));
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal128Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp;
+ auto idx = i * 2;
+#if ARROW_LITTLE_ENDIAN
+ tmp = BitUtil::FromBigEndian(data[idx]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#else
+ tmp = BitUtil::FromLittleEndian(data[idx]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#endif
+ }
+ (*out_)->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal256Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp0, tmp1, tmp2;
+ auto idx = i * 4;
+#if ARROW_LITTLE_ENDIAN
+ tmp0 = BitUtil::FromBigEndian(data[idx]);
+ tmp1 = BitUtil::FromBigEndian(data[idx + 1]);
+ tmp2 = BitUtil::FromBigEndian(data[idx + 2]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 3]);
+ new_data[idx + 1] = tmp2;
+ new_data[idx + 2] = tmp1;
+ new_data[idx + 3] = tmp0;
+#else
+ tmp0 = BitUtil::FromLittleEndian(data[idx]);
+ tmp1 = BitUtil::FromLittleEndian(data[idx + 1]);
+ tmp2 = BitUtil::FromLittleEndian(data[idx + 2]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 3]);
+ new_data[idx + 1] = tmp2;
+ new_data[idx + 2] = tmp1;
+ new_data[idx + 3] = tmp0;
+#endif
+ }
+ (*out_)->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const DayTimeIntervalType& type) {
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ ByteSwapBuffer<uint32_t>(data_->buffers[1], length_ * 2, 0));
+ return Status::OK();
+ }
+
+ Status CopyDataBuffer() {
+ if (data_->buffers[1]->data() == nullptr) {
+ return Status::OK();
+ }
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ data_->buffers[1]->CopySlice(0, data_->buffers[1]->size()));
+ return Status::OK();
+ }
+
+ Status Visit(const NullType& type) { return Status::OK(); }
+ Status Visit(const BooleanType& type) { return CopyDataBuffer(); }
+ Status Visit(const Int8Type& type) { return CopyDataBuffer(); }
+ Status Visit(const UInt8Type& type) { return CopyDataBuffer(); }
+ Status Visit(const FixedSizeBinaryType& type) { return CopyDataBuffer(); }
+ Status Visit(const FixedSizeListType& type) { return Status::OK(); }
+ Status Visit(const StructType& type) { return Status::OK(); }
+ Status Visit(const UnionType& type) {
+ (*out_)->buffers[1] = data_->buffers[1];
+ if (type.mode() == UnionMode::DENSE) {
+ RETURN_NOT_OK(SwapSmallOffset(2));
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ enable_if_t<std::is_same<BinaryType, T>::value || std::is_same<StringType, T>::value,
+ Status>
+ Visit(const T& type) {
+ RETURN_NOT_OK(SwapSmallOffset());
Review comment:
You can use `SwapOffsets<T::offset_type>` and merge with the below function.
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
+ int64_t length, int64_t extra_size) {
+ auto in_data = reinterpret_cast<const T*>(in_buffer->data());
+ ARROW_ASSIGN_OR_RAISE(auto out_buffer, AllocateBuffer(in_buffer->size()));
+ auto out_data = reinterpret_cast<T*>(out_buffer->mutable_data());
+ for (int64_t i = 0; i < length + extra_size; i++) {
+#if ARROW_LITTLE_ENDIAN
+ out_data[i] = BitUtil::FromBigEndian(in_data[i]);
+#else
+ out_data[i] = BitUtil::FromLittleEndian(in_data[i]);
+#endif
+ }
+ return std::move(out_buffer);
+ }
+
+ template <typename VALUE_TYPE>
+ Status SwapOffset(int index) {
+ if (data_->buffers[index] == nullptr || data_->buffers[index]->size() == 0) {
+ (*out_)->buffers[index] = data_->buffers[index];
+ return Status::OK();
+ }
+ // offset has one more element rather than data->length
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[index],
+ ByteSwapBuffer<VALUE_TYPE>(data_->buffers[index], length_, 1));
+ return Status::OK();
+ }
+
+ Status SwapSmallOffset(int index = 1) { return SwapOffset<int32_t>(index); }
+
+ Status SwapLargeOffset() { return SwapOffset<int64_t>(1); }
+
+ template <typename T>
+ enable_if_t<std::is_base_of<FixedWidthType, T>::value &&
+ !std::is_base_of<FixedSizeBinaryType, T>::value &&
+ !std::is_base_of<DictionaryType, T>::value,
+ Status>
+ Visit(const T& type) {
+ using value_type = typename T::c_type;
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ ByteSwapBuffer<value_type>(data_->buffers[1], length_, 0));
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal128Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp;
+ auto idx = i * 2;
+#if ARROW_LITTLE_ENDIAN
+ tmp = BitUtil::FromBigEndian(data[idx]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#else
+ tmp = BitUtil::FromLittleEndian(data[idx]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#endif
+ }
+ (*out_)->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal256Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp0, tmp1, tmp2;
+ auto idx = i * 4;
+#if ARROW_LITTLE_ENDIAN
+ tmp0 = BitUtil::FromBigEndian(data[idx]);
+ tmp1 = BitUtil::FromBigEndian(data[idx + 1]);
+ tmp2 = BitUtil::FromBigEndian(data[idx + 2]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 3]);
+ new_data[idx + 1] = tmp2;
+ new_data[idx + 2] = tmp1;
+ new_data[idx + 3] = tmp0;
+#else
+ tmp0 = BitUtil::FromLittleEndian(data[idx]);
+ tmp1 = BitUtil::FromLittleEndian(data[idx + 1]);
+ tmp2 = BitUtil::FromLittleEndian(data[idx + 2]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 3]);
+ new_data[idx + 1] = tmp2;
+ new_data[idx + 2] = tmp1;
+ new_data[idx + 3] = tmp0;
+#endif
+ }
+ (*out_)->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const DayTimeIntervalType& type) {
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ ByteSwapBuffer<uint32_t>(data_->buffers[1], length_ * 2, 0));
+ return Status::OK();
+ }
+
+ Status CopyDataBuffer() {
+ if (data_->buffers[1]->data() == nullptr) {
+ return Status::OK();
+ }
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ data_->buffers[1]->CopySlice(0, data_->buffers[1]->size()));
Review comment:
Why is a copy required? You can just reuse the buffer as-is.
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
+ int64_t length, int64_t extra_size) {
+ auto in_data = reinterpret_cast<const T*>(in_buffer->data());
+ ARROW_ASSIGN_OR_RAISE(auto out_buffer, AllocateBuffer(in_buffer->size()));
+ auto out_data = reinterpret_cast<T*>(out_buffer->mutable_data());
+ for (int64_t i = 0; i < length + extra_size; i++) {
+#if ARROW_LITTLE_ENDIAN
+ out_data[i] = BitUtil::FromBigEndian(in_data[i]);
+#else
+ out_data[i] = BitUtil::FromLittleEndian(in_data[i]);
+#endif
Review comment:
Why not use `BitUtil::ByteSwap`?
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
+ int64_t length, int64_t extra_size) {
+ auto in_data = reinterpret_cast<const T*>(in_buffer->data());
+ ARROW_ASSIGN_OR_RAISE(auto out_buffer, AllocateBuffer(in_buffer->size()));
+ auto out_data = reinterpret_cast<T*>(out_buffer->mutable_data());
+ for (int64_t i = 0; i < length + extra_size; i++) {
+#if ARROW_LITTLE_ENDIAN
+ out_data[i] = BitUtil::FromBigEndian(in_data[i]);
+#else
+ out_data[i] = BitUtil::FromLittleEndian(in_data[i]);
+#endif
+ }
+ return std::move(out_buffer);
+ }
+
+ template <typename VALUE_TYPE>
+ Status SwapOffset(int index) {
+ if (data_->buffers[index] == nullptr || data_->buffers[index]->size() == 0) {
+ (*out_)->buffers[index] = data_->buffers[index];
+ return Status::OK();
+ }
+ // offset has one more element rather than data->length
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[index],
+ ByteSwapBuffer<VALUE_TYPE>(data_->buffers[index], length_, 1));
+ return Status::OK();
+ }
+
+ Status SwapSmallOffset(int index = 1) { return SwapOffset<int32_t>(index); }
+
+ Status SwapLargeOffset() { return SwapOffset<int64_t>(1); }
+
+ template <typename T>
+ enable_if_t<std::is_base_of<FixedWidthType, T>::value &&
+ !std::is_base_of<FixedSizeBinaryType, T>::value &&
+ !std::is_base_of<DictionaryType, T>::value,
+ Status>
+ Visit(const T& type) {
+ using value_type = typename T::c_type;
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ ByteSwapBuffer<value_type>(data_->buffers[1], length_, 0));
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal128Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp;
+ auto idx = i * 2;
+#if ARROW_LITTLE_ENDIAN
+ tmp = BitUtil::FromBigEndian(data[idx]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#else
+ tmp = BitUtil::FromLittleEndian(data[idx]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#endif
+ }
+ (*out_)->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal256Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp0, tmp1, tmp2;
+ auto idx = i * 4;
+#if ARROW_LITTLE_ENDIAN
+ tmp0 = BitUtil::FromBigEndian(data[idx]);
+ tmp1 = BitUtil::FromBigEndian(data[idx + 1]);
+ tmp2 = BitUtil::FromBigEndian(data[idx + 2]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 3]);
+ new_data[idx + 1] = tmp2;
+ new_data[idx + 2] = tmp1;
+ new_data[idx + 3] = tmp0;
+#else
+ tmp0 = BitUtil::FromLittleEndian(data[idx]);
+ tmp1 = BitUtil::FromLittleEndian(data[idx + 1]);
+ tmp2 = BitUtil::FromLittleEndian(data[idx + 2]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 3]);
+ new_data[idx + 1] = tmp2;
+ new_data[idx + 2] = tmp1;
+ new_data[idx + 3] = tmp0;
+#endif
+ }
+ (*out_)->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const DayTimeIntervalType& type) {
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ ByteSwapBuffer<uint32_t>(data_->buffers[1], length_ * 2, 0));
+ return Status::OK();
+ }
+
+ Status CopyDataBuffer() {
+ if (data_->buffers[1]->data() == nullptr) {
+ return Status::OK();
+ }
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ data_->buffers[1]->CopySlice(0, data_->buffers[1]->size()));
+ return Status::OK();
+ }
+
+ Status Visit(const NullType& type) { return Status::OK(); }
+ Status Visit(const BooleanType& type) { return CopyDataBuffer(); }
+ Status Visit(const Int8Type& type) { return CopyDataBuffer(); }
+ Status Visit(const UInt8Type& type) { return CopyDataBuffer(); }
+ Status Visit(const FixedSizeBinaryType& type) { return CopyDataBuffer(); }
+ Status Visit(const FixedSizeListType& type) { return Status::OK(); }
+ Status Visit(const StructType& type) { return Status::OK(); }
+ Status Visit(const UnionType& type) {
+ (*out_)->buffers[1] = data_->buffers[1];
+ if (type.mode() == UnionMode::DENSE) {
+ RETURN_NOT_OK(SwapSmallOffset(2));
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ enable_if_t<std::is_same<BinaryType, T>::value || std::is_same<StringType, T>::value,
+ Status>
+ Visit(const T& type) {
+ RETURN_NOT_OK(SwapSmallOffset());
+ (*out_)->buffers[2] = data_->buffers[2];
+ return Status::OK();
+ }
+
+ template <typename T>
+ enable_if_t<std::is_same<LargeBinaryType, T>::value ||
+ std::is_same<LargeStringType, T>::value,
+ Status>
+ Visit(const T& type) {
+ RETURN_NOT_OK(SwapLargeOffset());
+ (*out_)->buffers[2] = data_->buffers[2];
+ return Status::OK();
+ }
+
+ Status Visit(const ListType& type) {
+ RETURN_NOT_OK(SwapSmallOffset());
+ return Status::OK();
+ }
+ Status Visit(const LargeListType& type) {
+ RETURN_NOT_OK(SwapLargeOffset());
Review comment:
Same here (`SwapOffsets<T::offset_type>`)
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
+ int64_t length, int64_t extra_size) {
+ auto in_data = reinterpret_cast<const T*>(in_buffer->data());
+ ARROW_ASSIGN_OR_RAISE(auto out_buffer, AllocateBuffer(in_buffer->size()));
+ auto out_data = reinterpret_cast<T*>(out_buffer->mutable_data());
+ for (int64_t i = 0; i < length + extra_size; i++) {
+#if ARROW_LITTLE_ENDIAN
+ out_data[i] = BitUtil::FromBigEndian(in_data[i]);
+#else
+ out_data[i] = BitUtil::FromLittleEndian(in_data[i]);
+#endif
+ }
+ return std::move(out_buffer);
+ }
+
+ template <typename VALUE_TYPE>
+ Status SwapOffset(int index) {
+ if (data_->buffers[index] == nullptr || data_->buffers[index]->size() == 0) {
+ (*out_)->buffers[index] = data_->buffers[index];
+ return Status::OK();
+ }
+ // offset has one more element rather than data->length
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[index],
+ ByteSwapBuffer<VALUE_TYPE>(data_->buffers[index], length_, 1));
+ return Status::OK();
+ }
+
+ Status SwapSmallOffset(int index = 1) { return SwapOffset<int32_t>(index); }
+
+ Status SwapLargeOffset() { return SwapOffset<int64_t>(1); }
+
+ template <typename T>
+ enable_if_t<std::is_base_of<FixedWidthType, T>::value &&
+ !std::is_base_of<FixedSizeBinaryType, T>::value &&
+ !std::is_base_of<DictionaryType, T>::value,
+ Status>
+ Visit(const T& type) {
+ using value_type = typename T::c_type;
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ ByteSwapBuffer<value_type>(data_->buffers[1], length_, 0));
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal128Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp;
+ auto idx = i * 2;
+#if ARROW_LITTLE_ENDIAN
+ tmp = BitUtil::FromBigEndian(data[idx]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#else
+ tmp = BitUtil::FromLittleEndian(data[idx]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 1]);
+ new_data[idx + 1] = tmp;
+#endif
+ }
+ (*out_)->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const Decimal256Type& type) {
+ auto data = reinterpret_cast<const uint64_t*>(data_->buffers[1]->data());
+ ARROW_ASSIGN_OR_RAISE(auto new_buffer, AllocateBuffer(data_->buffers[1]->size()));
+ auto new_data = reinterpret_cast<uint64_t*>(new_buffer->mutable_data());
+ int64_t length = length_;
+ for (int64_t i = 0; i < length; i++) {
+ uint64_t tmp0, tmp1, tmp2;
+ auto idx = i * 4;
+#if ARROW_LITTLE_ENDIAN
+ tmp0 = BitUtil::FromBigEndian(data[idx]);
+ tmp1 = BitUtil::FromBigEndian(data[idx + 1]);
+ tmp2 = BitUtil::FromBigEndian(data[idx + 2]);
+ new_data[idx] = BitUtil::FromBigEndian(data[idx + 3]);
+ new_data[idx + 1] = tmp2;
+ new_data[idx + 2] = tmp1;
+ new_data[idx + 3] = tmp0;
+#else
+ tmp0 = BitUtil::FromLittleEndian(data[idx]);
+ tmp1 = BitUtil::FromLittleEndian(data[idx + 1]);
+ tmp2 = BitUtil::FromLittleEndian(data[idx + 2]);
+ new_data[idx] = BitUtil::FromLittleEndian(data[idx + 3]);
+ new_data[idx + 1] = tmp2;
+ new_data[idx + 2] = tmp1;
+ new_data[idx + 3] = tmp0;
+#endif
+ }
+ (*out_)->buffers[1] = std::move(new_buffer);
+ return Status::OK();
+ }
+
+ Status Visit(const DayTimeIntervalType& type) {
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ ByteSwapBuffer<uint32_t>(data_->buffers[1], length_ * 2, 0));
+ return Status::OK();
+ }
+
+ Status CopyDataBuffer() {
+ if (data_->buffers[1]->data() == nullptr) {
+ return Status::OK();
+ }
+ ARROW_ASSIGN_OR_RAISE((*out_)->buffers[1],
+ data_->buffers[1]->CopySlice(0, data_->buffers[1]->size()));
+ return Status::OK();
+ }
+
+ Status Visit(const NullType& type) { return Status::OK(); }
+ Status Visit(const BooleanType& type) { return CopyDataBuffer(); }
+ Status Visit(const Int8Type& type) { return CopyDataBuffer(); }
+ Status Visit(const UInt8Type& type) { return CopyDataBuffer(); }
+ Status Visit(const FixedSizeBinaryType& type) { return CopyDataBuffer(); }
+ Status Visit(const FixedSizeListType& type) { return Status::OK(); }
+ Status Visit(const StructType& type) { return Status::OK(); }
+ Status Visit(const UnionType& type) {
+ (*out_)->buffers[1] = data_->buffers[1];
+ if (type.mode() == UnionMode::DENSE) {
+ RETURN_NOT_OK(SwapSmallOffset(2));
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ enable_if_t<std::is_same<BinaryType, T>::value || std::is_same<StringType, T>::value,
+ Status>
+ Visit(const T& type) {
+ RETURN_NOT_OK(SwapSmallOffset());
+ (*out_)->buffers[2] = data_->buffers[2];
+ return Status::OK();
+ }
+
+ template <typename T>
+ enable_if_t<std::is_same<LargeBinaryType, T>::value ||
+ std::is_same<LargeStringType, T>::value,
+ Status>
+ Visit(const T& type) {
+ RETURN_NOT_OK(SwapLargeOffset());
+ (*out_)->buffers[2] = data_->buffers[2];
+ return Status::OK();
+ }
+
+ Status Visit(const ListType& type) {
+ RETURN_NOT_OK(SwapSmallOffset());
+ return Status::OK();
+ }
+ Status Visit(const LargeListType& type) {
+ RETURN_NOT_OK(SwapLargeOffset());
+ return Status::OK();
+ }
+
+ Status Visit(const MapType& type) {
+ RETURN_NOT_OK(SwapSmallOffset());
+ return Status::OK();
+ }
+
+ Status Visit(const DictionaryType& type) {
+ RETURN_NOT_OK(SwapType(*type.index_type()));
+ (*out_)->dictionary = data_->dictionary;
+ return Status::OK();
+ }
+
+ Status Visit(const ExtensionType& type) {
+ RETURN_NOT_OK(SwapType(*type.storage_type()));
+ (*out_)->dictionary = data_->dictionary;
+ return Status::OK();
+ }
+
+ std::shared_ptr<ArrayData>& data_;
Review comment:
const reference here
##########
File path: cpp/src/arrow/array/util.cc
##########
@@ -74,6 +75,220 @@ class ArrayDataWrapper {
std::shared_ptr<Array>* out_;
};
+class ArrayDataEndianSwapper {
+ public:
+ ArrayDataEndianSwapper(std::shared_ptr<ArrayData>& data, int64_t length,
+ std::shared_ptr<ArrayData>* out)
+ : data_(data), length_(length), out_(out) {}
+
+ Status SwapType(const DataType& type) {
+ RETURN_NOT_OK(VisitTypeInline(type, this));
+ RETURN_NOT_OK(SwapChildren(type.fields()));
+ return Status::OK();
+ }
+
+ Status SwapChildren(std::vector<std::shared_ptr<Field>> child_fields) {
+ int i = 0;
+ for (const auto& child_field : child_fields) {
+ ARROW_ASSIGN_OR_RAISE(
+ (*out_)->child_data[i],
+ SwapEndianArrayData(data_->child_data[i], child_field.get()->type()));
+ i++;
+ }
+ return Status::OK();
+ }
+
+ template <typename T>
+ Result<std::shared_ptr<Buffer>> ByteSwapBuffer(std::shared_ptr<Buffer>& in_buffer,
+ int64_t length, int64_t extra_size) {
Review comment:
Why `extra_size`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org