wjones127 commented on code in PR #14658:
URL: https://github.com/apache/arrow/pull/14658#discussion_r1024345220
##########
cpp/src/arrow/compute/exec/asof_join_node_test.cc:
##########
@@ -83,6 +85,7 @@ Result<BatchesWithSchema> MakeBatchesFromNumString(
batches.schema = schema;
int n_fields = schema->num_fields();
for (auto num_batch : num_batches.batches) {
+ Datum two(ConstantArrayGenerator::Int32(num_batch.length, 2));
Review Comment:
Why not a scalar here? (and remove `#include "arrow/testing/generator.h"`
above)
```suggestion
Datum two(Int32Scalar(2));
```
##########
cpp/src/arrow/compute/exec/asof_join_node.cc:
##########
@@ -664,12 +666,25 @@ class CompositeReferenceTable {
}
template <class Type, class Builder = typename TypeTraits<Type>::BuilderType>
- enable_if_fixed_width_type<Type, Status> static BuilderAppend(
+ enable_if_boolean<Type, Status> static BuilderAppend(
Builder& builder, const std::shared_ptr<ArrayData>& source, row_index_t
row) {
if (source->IsNull(row)) {
builder.UnsafeAppendNull();
return Status::OK();
}
+ builder.UnsafeAppend(bit_util::GetBit(source->template
GetValues<uint8_t>(1), row));
+ return Status::OK();
+ }
+
+ template <class Type, class Builder = typename TypeTraits<Type>::BuilderType>
+ enable_if_t<is_fixed_width_type<Type>::value &&
!is_boolean_type<Type>::value,
+ Status> static BuilderAppend(Builder& builder,
+ const std::shared_ptr<ArrayData>&
source,
+ row_index_t row) {
+ if (source->IsNull(row)) {
+ builder.UnsafeAppendNull();
+ return Status::OK();
+ }
Review Comment:
One pattern we've been moving to in the code base is using constexpr
conditions:
```suggestion
template <class Type, class Builder = typename
TypeTraits<Type>::BuilderType>
enable_if_fixed_width_type<Type, Status> static BuilderAppend(Builder&
builder,
const std::shared_ptr<ArrayData>&
source,
row_index_t row) {
if (source->IsNull(row)) {
builder.UnsafeAppendNull();
return Status::OK();
}
if constexpr (is_boolean_type<Type>::value) {
builder.UnsafeAppend(bit_util::GetBit(source->template
GetValues<uint8_t>(1), row));
} else {
using CType = typename TypeTraits<Type>::CType;
builder.UnsafeAppend(source->template GetValues<CType>(1)[row]);
}
```
(Note this suggestion is incomplete, since GitHub limits which lines I can
diff.)
--
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]
