Skip to content

Commit 8aeaf96

Browse files
wesmwesm
committed
Code review
Change-Id: I5180e072b4451ec575c6b9157679fa31dfba5b74
1 parent ab534d1 commit 8aeaf96

File tree

8 files changed

+99
-111
lines changed

8 files changed

+99
-111
lines changed

‎cpp/src/arrow/array.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ std::shared_ptr<ArrayData> ArrayData::Make(const std::shared_ptr<DataType>& type
7777
}
7878

7979
int64_t ArrayData::GetNullCount() const {
80-
if (ARROW_PREDICT_FALSE(this->null_count < 0)) {
80+
if (ARROW_PREDICT_FALSE(this->null_count == kUnknownNullCount)) {
8181
if (this->buffers[0]) {
8282
this->null_count = this->length - CountSetBits(this->buffers[0]->data(),
8383
this->offset, this->length);

‎cpp/src/arrow/compute/kernel.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,8 @@ class ARROW_EXPORT UnaryKernel : public OpKernel {
189189
/// contracts.
190190
virtual Status Call(FunctionContext* ctx, const Datum& input, Datum* out) = 0;
191191

192+
/// \brief EXPERIMENTAL The output data type of the kernel
193+
/// \return the output type
192194
virtual std::shared_ptr<DataType> out_type() const = 0;
193195
};
194196

‎cpp/src/arrow/compute/kernels/cast.cc‎

Lines changed: 55 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -618,8 +618,8 @@ struct CastFunctor<Date32Type, Date64Type> {
618618

619619
class CastKernelBase : public UnaryKernel {
620620
public:
621-
explicit CastKernelBase(const std::shared_ptr<DataType>& out_type)
622-
: out_type_(out_type) {}
621+
explicit CastKernelBase(std::shared_ptr<DataType> out_type)
622+
: out_type_(std::move(out_type)) {}
623623

624624
std::shared_ptr<DataType> out_type() const override { return out_type_; }
625625

@@ -649,8 +649,8 @@ Status InvokeWithAllocation(FunctionContext* ctx, UnaryKernel* func, const Datum
649649
class ListCastKernel : public CastKernelBase {
650650
public:
651651
ListCastKernel(std::unique_ptr<UnaryKernel> child_caster,
652-
const std::shared_ptr<DataType>& out_type)
653-
: CastKernelBase(out_type), child_caster_(std::move(child_caster)) {}
652+
std::shared_ptr<DataType> out_type)
653+
: CastKernelBase(std::move(out_type)), child_caster_(std::move(child_caster)) {}
654654

655655
Status Call(FunctionContext* ctx, const Datum& input, Datum* out) override {
656656
DCHECK_EQ(Datum::ARRAY, input.kind());
@@ -1069,8 +1069,8 @@ class ZeroCopyCast : public CastKernelBase {
10691069
class CastKernel : public CastKernelBase {
10701070
public:
10711071
CastKernel(const CastOptions& options, const CastFunction& func,
1072-
const std::shared_ptr<DataType>& out_type)
1073-
: CastKernelBase(out_type), options_(options), func_(func) {}
1072+
std::shared_ptr<DataType> out_type)
1073+
: CastKernelBase(std::move(out_type)), options_(options), func_(func) {}
10741074

10751075
Status Call(FunctionContext* ctx, const Datum& input, Datum* out) override {
10761076
DCHECK_EQ(input.kind(), Datum::ARRAY);
@@ -1100,22 +1100,44 @@ class CastKernel : public CastKernelBase {
11001100
}; \
11011101
break;
11021102

1103-
#define GET_CAST_FUNCTION(CASE_GENERATOR, InType) \
1104-
static std::unique_ptr<UnaryKernel> Get##InType##CastFunc( \
1105-
const std::shared_ptr<DataType>& out_type, const CastOptions& options) { \
1106-
CastFunction func; \
1107-
switch (out_type->id()) { \
1108-
CASE_GENERATOR(CAST_CASE); \
1109-
default: \
1110-
break; \
1111-
} \
1112-
if (func != nullptr) { \
1113-
return std::unique_ptr<UnaryKernel>(new CastKernel(options, func, out_type)); \
1114-
} \
1115-
return nullptr; \
1103+
#define GET_CAST_FUNCTION(CASE_GENERATOR, InType) \
1104+
static std::unique_ptr<UnaryKernel> Get##InType##CastFunc( \
1105+
std::shared_ptr<DataType> out_type, const CastOptions& options) { \
1106+
CastFunction func; \
1107+
switch (out_type->id()) { \
1108+
CASE_GENERATOR(CAST_CASE); \
1109+
default: \
1110+
break; \
1111+
} \
1112+
if (func != nullptr) { \
1113+
return std::unique_ptr<UnaryKernel>( \
1114+
new CastKernel(options, func, std::move(out_type))); \
1115+
} \
1116+
return nullptr; \
11161117
}
11171118

1118-
#include "cast-codegen-internal.h" // NOLINT
1119+
#include "generated/cast-codegen-internal.h" // NOLINT
1120+
1121+
GET_CAST_FUNCTION(NULL_CASES, NullType)
1122+
GET_CAST_FUNCTION(BOOLEAN_CASES, BooleanType)
1123+
GET_CAST_FUNCTION(UINT8_CASES, UInt8Type)
1124+
GET_CAST_FUNCTION(INT8_CASES, Int8Type)
1125+
GET_CAST_FUNCTION(UINT16_CASES, UInt16Type)
1126+
GET_CAST_FUNCTION(INT16_CASES, Int16Type)
1127+
GET_CAST_FUNCTION(UINT32_CASES, UInt32Type)
1128+
GET_CAST_FUNCTION(INT32_CASES, Int32Type)
1129+
GET_CAST_FUNCTION(UINT64_CASES, UInt64Type)
1130+
GET_CAST_FUNCTION(INT64_CASES, Int64Type)
1131+
GET_CAST_FUNCTION(FLOAT_CASES, FloatType)
1132+
GET_CAST_FUNCTION(DOUBLE_CASES, DoubleType)
1133+
GET_CAST_FUNCTION(DATE32_CASES, Date32Type)
1134+
GET_CAST_FUNCTION(DATE64_CASES, Date64Type)
1135+
GET_CAST_FUNCTION(TIME32_CASES, Time32Type)
1136+
GET_CAST_FUNCTION(TIME64_CASES, Time64Type)
1137+
GET_CAST_FUNCTION(TIMESTAMP_CASES, TimestampType)
1138+
GET_CAST_FUNCTION(BINARY_CASES, BinaryType)
1139+
GET_CAST_FUNCTION(STRING_CASES, StringType)
1140+
GET_CAST_FUNCTION(DICTIONARY_CASES, DictionaryType)
11191141

11201142
#define CAST_FUNCTION_CASE(InType) \
11211143
case InType::type_id: \
@@ -1124,7 +1146,7 @@ class CastKernel : public CastKernelBase {
11241146

11251147
namespace {
11261148

1127-
Status GetListCastFunc(const DataType& in_type, const std::shared_ptr<DataType>& out_type,
1149+
Status GetListCastFunc(const DataType& in_type, std::shared_ptr<DataType> out_type,
11281150
const CastOptions& options, std::unique_ptr<UnaryKernel>* kernel) {
11291151
if (out_type->id() != Type::LIST) {
11301152
// Kernel will be null
@@ -1135,8 +1157,8 @@ Status GetListCastFunc(const DataType& in_type, const std::shared_ptr<DataType>&
11351157
checked_cast<const ListType&>(*out_type).value_type();
11361158
std::unique_ptr<UnaryKernel> child_caster;
11371159
RETURN_NOT_OK(GetCastFunction(in_value_type, out_value_type, options, &child_caster));
1138-
*kernel =
1139-
std::unique_ptr<UnaryKernel>(new ListCastKernel(std::move(child_caster), out_type));
1160+
*kernel = std::unique_ptr<UnaryKernel>(
1161+
new ListCastKernel(std::move(child_caster), std::move(out_type)));
11401162
return Status::OK();
11411163
}
11421164

@@ -1162,15 +1184,15 @@ inline bool IsZeroCopyCast(Type::type in_type, Type::type out_type) {
11621184
return false;
11631185
}
11641186

1165-
Status GetCastFunction(const DataType& in_type, const std::shared_ptr<DataType>& out_type,
1187+
Status GetCastFunction(const DataType& in_type, std::shared_ptr<DataType> out_type,
11661188
const CastOptions& options, std::unique_ptr<UnaryKernel>* kernel) {
11671189
if (in_type.Equals(out_type)) {
1168-
*kernel = std::unique_ptr<UnaryKernel>(new IdentityCast(out_type));
1190+
*kernel = std::unique_ptr<UnaryKernel>(new IdentityCast(std::move(out_type)));
11691191
return Status::OK();
11701192
}
11711193

11721194
if (IsZeroCopyCast(in_type.id(), out_type->id())) {
1173-
*kernel = std::unique_ptr<UnaryKernel>(new ZeroCopyCast(out_type));
1195+
*kernel = std::unique_ptr<UnaryKernel>(new ZeroCopyCast(std::move(out_type)));
11741196
return Status::OK();
11751197
}
11761198

@@ -1196,7 +1218,7 @@ Status GetCastFunction(const DataType& in_type, const std::shared_ptr<DataType>&
11961218
CAST_FUNCTION_CASE(StringType);
11971219
CAST_FUNCTION_CASE(DictionaryType);
11981220
case Type::LIST:
1199-
RETURN_NOT_OK(GetListCastFunc(in_type, out_type, options, kernel));
1221+
RETURN_NOT_OK(GetListCastFunc(in_type, std::move(out_type), options, kernel));
12001222
break;
12011223
default:
12021224
break;
@@ -1208,22 +1230,20 @@ Status GetCastFunction(const DataType& in_type, const std::shared_ptr<DataType>&
12081230
return Status::OK();
12091231
}
12101232

1211-
Status Cast(FunctionContext* ctx, const Datum& value,
1212-
const std::shared_ptr<DataType>& out_type, const CastOptions& options,
1213-
Datum* out) {
1233+
Status Cast(FunctionContext* ctx, const Datum& value, std::shared_ptr<DataType> out_type,
1234+
const CastOptions& options, Datum* out) {
12141235
const DataType& in_type = *value.type();
12151236

12161237
// Dynamic dispatch to obtain right cast function
12171238
std::unique_ptr<UnaryKernel> func;
1218-
RETURN_NOT_OK(GetCastFunction(in_type, out_type, options, &func));
1239+
RETURN_NOT_OK(GetCastFunction(in_type, std::move(out_type), options, &func));
12191240
return InvokeWithAllocation(ctx, func.get(), value, out);
12201241
}
12211242

1222-
Status Cast(FunctionContext* ctx, const Array& array,
1223-
const std::shared_ptr<DataType>& out_type, const CastOptions& options,
1224-
std::shared_ptr<Array>* out) {
1243+
Status Cast(FunctionContext* ctx, const Array& array, std::shared_ptr<DataType> out_type,
1244+
const CastOptions& options, std::shared_ptr<Array>* out) {
12251245
Datum datum_out;
1226-
RETURN_NOT_OK(Cast(ctx, Datum(array.data()), out_type, options, &datum_out));
1246+
RETURN_NOT_OK(Cast(ctx, Datum(array.data()), std::move(out_type), options, &datum_out));
12271247
DCHECK_EQ(Datum::ARRAY, datum_out.kind());
12281248
*out = MakeArray(datum_out.array());
12291249
return Status::OK();

‎cpp/src/arrow/compute/kernels/cast.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ struct ARROW_EXPORT CastOptions {
6262
/// \since 0.7.0
6363
/// \note API not yet finalized
6464
ARROW_EXPORT
65-
Status GetCastFunction(const DataType& in_type, const std::shared_ptr<DataType>& to_type,
65+
Status GetCastFunction(const DataType& in_type, std::shared_ptr<DataType> to_type,
6666
const CastOptions& options, std::unique_ptr<UnaryKernel>* kernel);
6767

6868
/// \brief Cast from one array type to another
@@ -76,7 +76,7 @@ Status GetCastFunction(const DataType& in_type, const std::shared_ptr<DataType>&
7676
/// \note API not yet finalized
7777
ARROW_EXPORT
7878
Status Cast(FunctionContext* context, const Array& value,
79-
const std::shared_ptr<DataType>& to_type, const CastOptions& options,
79+
std::shared_ptr<DataType> to_type, const CastOptions& options,
8080
std::shared_ptr<Array>* out);
8181

8282
/// \brief Cast from one value to another
@@ -90,8 +90,7 @@ Status Cast(FunctionContext* context, const Array& value,
9090
/// \note API not yet finalized
9191
ARROW_EXPORT
9292
Status Cast(FunctionContext* context, const Datum& value,
93-
const std::shared_ptr<DataType>& to_type, const CastOptions& options,
94-
Datum* out);
93+
std::shared_ptr<DataType> to_type, const CastOptions& options, Datum* out);
9594

9695
} // namespace compute
9796
} // namespace arrow

0 commit comments

Comments
 (0)