From cac0162ae4659aa0891738c086ce64efe7b60004 Mon Sep 17 00:00:00 2001 From: mwish Date: Wed, 27 Nov 2024 18:42:00 +0800 Subject: [PATCH 1/3] GH-44866: [C++] Minor enhance the doc and impl of DictionaryArray::Transpose --- cpp/src/arrow/array/array_dict.cc | 10 +++++----- cpp/src/arrow/array/array_dict.h | 4 +++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/array/array_dict.cc b/cpp/src/arrow/array/array_dict.cc index 55e086af30bc2..aa533f2fdde71 100644 --- a/cpp/src/arrow/array/array_dict.cc +++ b/cpp/src/arrow/array/array_dict.cc @@ -270,7 +270,7 @@ struct CompactTransposeMapVisitor { AllocateBuffer(dict_length * sizeof(int32_t), pool)); auto* output_map_raw = output_map->mutable_data_as(); int32_t current_index = 0; - for (CType i = 0; i < dict_len; i++) { + for (CType i = 0; i < dict_len; ++i) { if (dict_used[i]) { dict_indices_builder.UnsafeAppend(i); output_map_raw[i] = current_index; @@ -309,7 +309,7 @@ Result> CompactTransposeMap( CompactTransposeMapVisitor visitor{data, pool, nullptr, nullptr}; RETURN_NOT_OK(VisitTypeInline(*dict_type.index_type(), &visitor)); - out_compact_dictionary = visitor.out_compact_dictionary; + out_compact_dictionary = std::move(visitor.out_compact_dictionary); return std::move(visitor.output_map); } } // namespace @@ -320,7 +320,7 @@ Result> DictionaryArray::Transpose( ARROW_ASSIGN_OR_RAISE(auto transposed, TransposeDictIndices(data_, data_->type, type, dictionary->data(), transpose_map, pool)); - return MakeArray(std::move(transposed)); + return MakeArray(transposed); } Result> DictionaryArray::Compact(MemoryPool* pool) const { @@ -520,8 +520,8 @@ struct RecursiveUnifier { Result> DictionaryUnifier::Make( std::shared_ptr value_type, MemoryPool* pool) { - MakeUnifier maker(pool, value_type); - RETURN_NOT_OK(VisitTypeInline(*value_type, &maker)); + MakeUnifier maker(pool, std::move(value_type)); + RETURN_NOT_OK(VisitTypeInline(*maker.value_type, &maker)); return std::move(maker.result); } diff --git a/cpp/src/arrow/array/array_dict.h b/cpp/src/arrow/array/array_dict.h index bf376b51f8c94..9736e7ab0f69c 100644 --- a/cpp/src/arrow/array/array_dict.h +++ b/cpp/src/arrow/array/array_dict.h @@ -90,7 +90,9 @@ class ARROW_EXPORT DictionaryArray : public Array { /// \param[in] type the new type object /// \param[in] dictionary the new dictionary /// \param[in] transpose_map transposition array of this array's indices - /// into the target array's indices + /// into the target array's indices. The value of transpose_map should + /// be in the range [0, this->length()). + /// /// \param[in] pool a pool to allocate the array data from Result> Transpose( const std::shared_ptr& type, const std::shared_ptr& dictionary, From 7271e885430e8723bfcf78c0ebea20a42f3d54fc Mon Sep 17 00:00:00 2001 From: mwish Date: Thu, 28 Nov 2024 10:41:22 +0800 Subject: [PATCH 2/3] more hints --- cpp/src/arrow/array/array_dict.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/array/array_dict.h b/cpp/src/arrow/array/array_dict.h index 9736e7ab0f69c..0e256ec815213 100644 --- a/cpp/src/arrow/array/array_dict.h +++ b/cpp/src/arrow/array/array_dict.h @@ -91,7 +91,9 @@ class ARROW_EXPORT DictionaryArray : public Array { /// \param[in] dictionary the new dictionary /// \param[in] transpose_map transposition array of this array's indices /// into the target array's indices. The value of transpose_map should - /// be in the range [0, this->length()). + /// be in the range [0, this->length()). And the dictionary array's + /// indices in the target array's indices should be in the range + /// of [0, dictionary->length()). /// /// \param[in] pool a pool to allocate the array data from Result> Transpose( From 63f5d680caa0be7279e5d8c837333992f3ad98ae Mon Sep 17 00:00:00 2001 From: mwish Date: Fri, 29 Nov 2024 16:49:55 +0800 Subject: [PATCH 3/3] minor fix --- cpp/src/arrow/array/array_dict.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/array/array_dict.h b/cpp/src/arrow/array/array_dict.h index 0e256ec815213..24cdd3b59525c 100644 --- a/cpp/src/arrow/array/array_dict.h +++ b/cpp/src/arrow/array/array_dict.h @@ -91,9 +91,8 @@ class ARROW_EXPORT DictionaryArray : public Array { /// \param[in] dictionary the new dictionary /// \param[in] transpose_map transposition array of this array's indices /// into the target array's indices. The value of transpose_map should - /// be in the range [0, this->length()). And the dictionary array's - /// indices in the target array's indices should be in the range - /// of [0, dictionary->length()). + /// be in the range [0, dictionary->length()). And the dictionary indices + /// at transpose_map[i] should not be out of bounds. /// /// \param[in] pool a pool to allocate the array data from Result> Transpose(