dmitry-chirkov-dremio commented on code in PR #53161:
URL: https://github.com/apache/arrow/pull/53161#discussion_r4369357414
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -2230,21 +2237,95 @@ struct SerializeFunctor<
}
};
-template <>
-struct SerializeFunctor<Int32Type, ::arrow::Time32Type> {
- Status Serialize(const ::arrow::Time32Array& array, ArrowWriteContext*,
int32_t* out) {
- const int32_t* input = array.raw_values();
- const auto& type = static_cast<const ::arrow::Time32Type&>(*array.type());
- if (type.unit() == ::arrow::TimeUnit::SECOND) {
- for (int64_t i = 0; i < array.length(); i++) {
- out[i] = input[i] * 1000;
+template <typename ParquetType>
+template <typename ArrowType>
+Status TypedColumnWriterImpl<ParquetType>::WriteArrowTimes(
+ 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;
+ const auto& values = checked_cast<const ArrayType&>(array);
+ const auto& source_type = checked_cast<const
::arrow::TimeType&>(*array.type());
+ const auto& logical_type = this->descr()->logical_type();
+ auto target_unit = source_type.unit() == ::arrow::TimeUnit::SECOND
+ ? ::arrow::TimeUnit::MILLI
+ : source_type.unit();
+ if (logical_type->is_time()) {
+ switch (checked_cast<const TimeLogicalType&>(*logical_type).time_unit()) {
+ case LogicalType::TimeUnit::MILLIS:
+ target_unit = ::arrow::TimeUnit::MILLI;
+ break;
+ case LogicalType::TimeUnit::MICROS:
+ target_unit = ::arrow::TimeUnit::MICRO;
+ break;
+ case LogicalType::TimeUnit::NANOS:
+ target_unit = ::arrow::TimeUnit::NANO;
+ break;
+ default:
+ return Status::Invalid("Unsupported Parquet time unit");
+ }
+ } else if constexpr (sizeof(T) != sizeof(typename ArrowType::c_type)) {
+ ARROW_UNSUPPORTED();
+ }
+ if (source_type.unit() == target_unit) {
+ return WriteArrowZeroCopy(def_levels, rep_levels, num_levels, array, ctx,
+ maybe_parent_nulls);
+ }
+
+ const auto target_type = target_unit == ::arrow::TimeUnit::MILLI
+ ? ::arrow::time32(target_unit)
+ : ::arrow::time64(target_unit);
+ const auto [operation, factor] =
+ ::arrow::util::GetTimestampConversion(source_type.unit(), target_unit);
+ const bool allow_truncation = ctx->properties->coerce_timestamps_enabled() &&
+
ctx->properties->truncated_timestamps_allowed();
+ T* out = nullptr;
+ RETURN_NOT_OK(ctx->GetScratchData<T>(array.length(), &out));
+ int64_t level_index = 0;
+ for (int64_t i = 0; i < array.length(); ++i) {
Review Comment:
`chunk_array` includes the remaining tail, so this loop converts the same
values again for every content-defined chunk.
With `time64[ns]`, version 2.4 and 64–128-byte chunks, I measured about
20/78/304 ms for 16K/32K/64K rows in a Release build. Equivalent microsecond
input takes about 2/4/8 ms. A temporary experiment limiting each slice to the
current chunk also brings the 64K case down to about 8 ms.
I'm not familiar enough with this specific code to anticipate how hot this
path is but could be an easy win.
--
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]
