felipecrv commented on code in PR #43445:
URL: https://github.com/apache/arrow/pull/43445#discussion_r1697368151
##########
cpp/src/arrow/util/binary_view_util.h:
##########
@@ -34,22 +35,27 @@ inline BinaryViewType::c_type ToInlineBinaryView(const
void* data, int32_t size)
}
inline BinaryViewType::c_type ToInlineBinaryView(std::string_view v) {
+ assert(v.size() <= BinaryViewType::kInlineSize);
return ToInlineBinaryView(v.data(), static_cast<int32_t>(v.size()));
}
-inline BinaryViewType::c_type ToBinaryView(const void* data, int32_t size,
- int32_t buffer_index, int32_t
offset) {
- if (size <= BinaryViewType::kInlineSize) {
- return ToInlineBinaryView(data, size);
- }
-
+inline BinaryViewType::c_type ToLargeBinaryView(const void* data, int32_t size,
Review Comment:
nit: call this `ToNonInlineBinaryView` to not confuse people that might
think a `LARGE_BINARY_VIEW` type exists. [I misread it like this at first :)]
##########
cpp/src/arrow/util/binary_view_util.h:
##########
@@ -26,6 +26,7 @@
namespace arrow::util {
inline BinaryViewType::c_type ToInlineBinaryView(const void* data, int32_t
size) {
+ assert(size <= BinaryViewType::kInlineSize);
Review Comment:
Which I think we be assumed when this is inlined from the `if` below.
##########
cpp/src/arrow/builder_benchmark.cc:
##########
@@ -458,6 +495,8 @@ BENCHMARK(BuildBinaryArray);
BENCHMARK(BuildChunkedBinaryArray);
BENCHMARK(BuildFixedSizeBinaryArray);
BENCHMARK(BuildDecimalArray);
+BENCHMARK(BuildSmallBinaryViewArray);
+BENCHMARK(BuildLargeBinaryViewArray);
Review Comment:
and the renames above ofc
##########
cpp/src/arrow/builder_benchmark.cc:
##########
@@ -458,6 +495,8 @@ BENCHMARK(BuildBinaryArray);
BENCHMARK(BuildChunkedBinaryArray);
BENCHMARK(BuildFixedSizeBinaryArray);
BENCHMARK(BuildDecimalArray);
+BENCHMARK(BuildSmallBinaryViewArray);
+BENCHMARK(BuildLargeBinaryViewArray);
Review Comment:
```suggestion
BENCHMARK(BuildInlineBinaryViewArray);
BENCHMARK(BuildNonInlineinaryViewArray);
```
--
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]
