wgtmac commented on code in PR #45688:
URL: https://github.com/apache/arrow/pull/45688#discussion_r1984336590
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -1209,28 +1209,31 @@ Status ConvertDictionaryToDense(const ::arrow::Array&
array, MemoryPool* pool,
return Status::OK();
}
-template <typename DType>
-class TypedColumnWriterImpl : public ColumnWriterImpl, public
TypedColumnWriter<DType> {
+template <typename ParquetType>
Review Comment:
This makes sense and already used by some functions in this file.
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -1870,67 +1908,40 @@ struct SerializeFunctor {
}
};
-template <typename ParquetType, typename ArrowType>
-Status WriteArrowSerialize(const ::arrow::Array& array, int64_t num_levels,
- const int16_t* def_levels, const int16_t*
rep_levels,
- ArrowWriteContext* ctx,
TypedColumnWriter<ParquetType>* writer,
- bool maybe_parent_nulls) {
- using ParquetCType = typename ParquetType::c_type;
+template <typename ParquetType>
+template <typename ArrowType>
+Status TypedColumnWriterImpl<ParquetType>::WriteArrowSerialize(
+ const int16_t* def_levels, const int16_t* rep_levels, int64_t num_levels,
+ const ::arrow::Array& array, ArrowWriteContext* ctx, bool
maybe_parent_nulls) {
using ArrayType = typename ::arrow::TypeTraits<ArrowType>::ArrayType;
+ using ParquetCType = typename ParquetType::c_type;
ParquetCType* buffer = nullptr;
PARQUET_THROW_NOT_OK(ctx->GetScratchData<ParquetCType>(array.length(),
&buffer));
SerializeFunctor<ParquetType, ArrowType> functor;
RETURN_NOT_OK(functor.Serialize(checked_cast<const ArrayType&>(array), ctx,
buffer));
bool no_nulls =
- writer->descr()->schema_node()->is_required() || (array.null_count() ==
0);
+ this->descr()->schema_node()->is_required() || (array.null_count() == 0);
if (!maybe_parent_nulls && no_nulls) {
- PARQUET_CATCH_NOT_OK(writer->WriteBatch(num_levels, def_levels,
rep_levels, buffer));
+ PARQUET_CATCH_NOT_OK(WriteBatch(num_levels, def_levels, rep_levels,
buffer));
} else {
- PARQUET_CATCH_NOT_OK(writer->WriteBatchSpaced(num_levels, def_levels,
rep_levels,
- array.null_bitmap_data(),
- array.offset(), buffer));
+ PARQUET_CATCH_NOT_OK(WriteBatchSpaced(num_levels, def_levels, rep_levels,
+ array.null_bitmap_data(),
array.offset(),
+ buffer));
}
return Status::OK();
}
-template <typename ParquetType>
-Status WriteArrowZeroCopy(const ::arrow::Array& array, int64_t num_levels,
- const int16_t* def_levels, const int16_t* rep_levels,
- ArrowWriteContext* ctx,
TypedColumnWriter<ParquetType>* writer,
- bool maybe_parent_nulls) {
- using T = typename ParquetType::c_type;
- const auto& data = static_cast<const ::arrow::PrimitiveArray&>(array);
- const T* values = nullptr;
- // The values buffer may be null if the array is empty (ARROW-2744)
- if (data.values() != nullptr) {
- values = reinterpret_cast<const T*>(data.values()->data()) + data.offset();
- } else {
- DCHECK_EQ(data.length(), 0);
- }
- bool no_nulls =
- writer->descr()->schema_node()->is_required() || (array.null_count() ==
0);
+#define WRITE_SERIALIZE_CASE(ArrowEnum, ArrowType)
\
Review Comment:
Is it possible to remove `ArrowType` as well by deducing it from `typename
::arrow::TypeIdTraits<::arrow::Type::ArrowEnum:>::Type`?
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -1870,67 +1908,40 @@ struct SerializeFunctor {
}
};
-template <typename ParquetType, typename ArrowType>
-Status WriteArrowSerialize(const ::arrow::Array& array, int64_t num_levels,
- const int16_t* def_levels, const int16_t*
rep_levels,
- ArrowWriteContext* ctx,
TypedColumnWriter<ParquetType>* writer,
- bool maybe_parent_nulls) {
- using ParquetCType = typename ParquetType::c_type;
+template <typename ParquetType>
+template <typename ArrowType>
Review Comment:
Why splitting them into two lines?
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
