This is an automated email from the ASF dual-hosted git repository.
apitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 4ea567c9d4 GH-35437: [C++] Remove obsolete TODO about DictionaryArray
const& return types (#48956)
4ea567c9d4 is described below
commit 4ea567c9d4b4f222b47511e742118e19453e4149
Author: Hyukjin Kwon <[email protected]>
AuthorDate: Wed Jan 28 18:34:17 2026 +0900
GH-35437: [C++] Remove obsolete TODO about DictionaryArray const& return
types (#48956)
### Rationale for this change
The TODO comment in `vector_array_sort.cc` asking whether
`DictionaryArray::dictionary()` and `DictionaryArray::indices()` should return
`const&` has been obsolete.
It was added in commit 6ceb12f700a when dictionary array sorting was
implemented. At that time, these methods returned `std::shared_ptr<Array>` by
value, causing unnecessary copies.
The issue was fixed in commit 95a8bfb319b which changed both methods to
return `const std::shared_ptr<Array>&`, removing the copies. However, the TODO
comment was left unremoved.
### What changes are included in this PR?
Removed the outdated TODO comment that referenced GH-35437.
### Are these changes tested?
I did not test.
### Are there any user-facing changes?
No.
* GitHub Issue: #35437
Authored-by: Hyukjin Kwon <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/compute/kernels/vector_array_sort.cc | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/cpp/src/arrow/compute/kernels/vector_array_sort.cc
b/cpp/src/arrow/compute/kernels/vector_array_sort.cc
index 950de47733..0c27808dd1 100644
--- a/cpp/src/arrow/compute/kernels/vector_array_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_array_sort.cc
@@ -183,10 +183,8 @@ class ArrayCompareSorter<DictionaryType> {
const ArraySortOptions& options,
ExecContext* ctx) {
const auto& dict_array = checked_cast<const DictionaryArray&>(array);
- // TODO: These methods should probably return a const&? They seem capable.
- // https://github.com/apache/arrow/issues/35437
- auto dict_values = dict_array.dictionary();
- auto dict_indices = dict_array.indices();
+ const auto& dict_values = dict_array.dictionary();
+ const auto& dict_indices = dict_array.indices();
// Algorithm:
// 1) Use the Rank function to get an exactly-equivalent-order array
