pitrou commented on a change in pull request #7898:
URL: https://github.com/apache/arrow/pull/7898#discussion_r467947160
##########
File path: cpp/src/arrow/array/builder_dict.h
##########
@@ -409,6 +409,10 @@ class DictionaryBuilder : public
internal::DictionaryBuilderBase<AdaptiveIntBuil
using BASE = internal::DictionaryBuilderBase<AdaptiveIntBuilder, T>;
using BASE::BASE;
+ Status ExpandIndexByteWidth(uint8_t new_index_byte_width) {
+ return BASE::indices_builder_.ExpandIntSize(new_index_byte_width);
Review comment:
Wouldn't it be better to add a new constructor signature, such as:
```c++
DictionaryBuilderBase(const std::shared_ptr<DataType>& start_index_type,
const std::shared_ptr<DataType>& value_type,
MemoryPool* pool = default_memory_pool())
```
?
##########
File path: cpp/src/arrow/array/builder_adaptive.h
##########
@@ -122,9 +122,10 @@ class ARROW_EXPORT AdaptiveUIntBuilder : public
internal::AdaptiveIntBuilderBase
std::shared_ptr<DataType> type() const override;
+ Status ExpandIntSize(uint8_t new_int_size);
Review comment:
Hmm... I don't think it is required to make this public. Is it?
##########
File path: cpp/src/arrow/array/array_dict_test.cc
##########
@@ -22,6 +22,8 @@
#include <string>
#include <vector>
+#include <iostream>
Review comment:
Is this necessary? If so, please put this include with the standard
library includes above.
----------------------------------------------------------------
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:
[email protected]
