divjotarora commented on code in PR #50916:
URL: https://github.com/apache/arrow/pull/50916#discussion_r3928589150


##########
cpp/src/parquet/arrow/reader_internal.cc:
##########
@@ -856,46 +855,80 @@ Status TransferHalfFloat(RecordReader* reader, 
MemoryPool* pool,
   return Status::OK();
 }
 
-// Read a TIMESTAMP-annotated FLBA(12) column as a 64-bit Arrow timestamp. 
Values that do
-// not fit in the 64 bit range either error or clamp to min/max int64, 
depending on
-// configuration.
+// Decode a little-endian 96-bit FLBA(12) TIMESTAMP value into a 64-bit Arrow 
timestamp.
+// Values that do not fit in the int64 range either error or clamp to 
INT64_MIN/INT64_MAX,
+// depending on clamp_on_overflow.
+Status FlbaTimestampToInt64(const uint8_t* bytes, bool clamp_on_overflow, 
int64_t* out) {
+  const uint64_t low = bit_util::FromLittleEndian(SafeLoadAs<uint64_t>(bytes));
+  const uint32_t high = bit_util::FromLittleEndian(SafeLoadAs<uint32_t>(bytes 
+ 8));
+  const int32_t high_signed = static_cast<int32_t>(high);
+  const int64_t low_signed = static_cast<int64_t>(low);
+  const int32_t sign_extension = (low_signed < 0) ? -1 : 0;
+  // Fits in int64 iff the high part is a pure sign-extension of the low part.
+  if (high_signed != sign_extension) {
+    if (!clamp_on_overflow) {
+      return Status::Invalid(
+          "FLBA(12) TIMESTAMP value does not fit in a 64-bit Arrow timestamp");
+    }
+    *out = high_signed < 0 ? INT64_MIN : INT64_MAX;
+  } else {
+    *out = low_signed;
+  }
+  return Status::OK();
+}
+
+Result<::arrow::TimeUnit::type> 
ArrowTimeUnitFromParquet(LogicalType::TimeUnit::unit unit) {
+  switch (unit) {
+    case LogicalType::TimeUnit::MILLIS:
+      return ::arrow::TimeUnit::MILLI;
+    case LogicalType::TimeUnit::MICROS:
+      return ::arrow::TimeUnit::MICRO;
+    case LogicalType::TimeUnit::NANOS:
+      return ::arrow::TimeUnit::NANO;
+    default:
+      return Status::Invalid("Unrecognized Parquet TIMESTAMP time unit");
+  }
+}
+
+// Read a TIMESTAMP-annotated FLBA(12) column as a 64-bit Arrow timestamp.
 Status TransferFlbaTimestamp(RecordReader* reader, MemoryPool* pool,
                              const std::shared_ptr<Field>& field, Datum* out,
                              bool clamp_on_overflow) {
-  static const auto binary_type = ::arrow::fixed_size_binary(12);
-  std::shared_ptr<ChunkedArray> chunked_array;
-  RETURN_NOT_OK(
-      TransferBinary(reader, pool, field->WithType(binary_type), 
&chunked_array));
+  auto binary_reader = dynamic_cast<BinaryRecordReader*>(reader);
+  DCHECK(binary_reader);
+  ::arrow::ArrayVector chunks = binary_reader->GetBuilderChunks();
 
-  ::arrow::TimestampBuilder builder(field->type(), pool);
-  RETURN_NOT_OK(builder.Reserve(chunked_array->length()));
-  for (const auto& chunk : chunked_array->chunks()) {
+  for (auto& chunk : chunks) {
     const auto& values = checked_cast<const 
::arrow::FixedSizeBinaryArray&>(*chunk);
-    for (int64_t i = 0; i < values.length(); ++i) {
-      if (values.IsNull(i)) {
-        builder.UnsafeAppendNull();
-        continue;
-      }
-      const uint8_t* bytes = values.GetValue(i);
-      const uint64_t low = 
bit_util::FromLittleEndian(SafeLoadAs<uint64_t>(bytes));
-      const uint32_t high = 
bit_util::FromLittleEndian(SafeLoadAs<uint32_t>(bytes + 8));
-      const int64_t low_signed = static_cast<int64_t>(low);
-      // Fits in int64 iff the high part is a pure sign-extension of the low 
part.
-      if (static_cast<int32_t>(high) != (low_signed < 0 ? -1 : 0)) {
-        if (!clamp_on_overflow) {
-          return Status::Invalid(
-              "FLBA(12) TIMESTAMP value does not fit in a 64-bit Arrow 
timestamp");
+    const int64_t length = values.length();
+    ARROW_ASSIGN_OR_RAISE(auto data,
+                          ::arrow::AllocateBuffer(length * sizeof(int64_t), 
pool));
+    auto out_ptr = reinterpret_cast<int64_t*>(data->mutable_data());
+
+    const int64_t null_count = values.null_count();
+    if (null_count > 0) {

Review Comment:
   This split was taken from the decimal code. It seems `VisitArraySpanInline` 
should be usable here, will change it



-- 
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]

Reply via email to