Resolve comments

facebookincubator · Jan 16, 2024 · 29afcfa · 29afcfa
1 parent 3fed97f
commit 29afcfa
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 14 deletions.
diff --git a/velox/dwio/common/TimestampDecoder.h b/velox/dwio/common/TimestampDecoder.h
@@ -31,7 +31,7 @@ class TimestampDecoder : public DirectDecoder<false> {
       uint32_t numBytes,
       bool bigEndian = false)
       : DirectDecoder<false>{std::move(input), useVInts, numBytes, bigEndian},
-        _precision(precision) {}
+        precision_(precision) {}
 
   template <bool hasNulls, typename Visitor>
   void readWithVisitor(
@@ -62,15 +62,12 @@ class TimestampDecoder : public DirectDecoder<false> {
       }
       if constexpr (std::is_same_v<typename Visitor::DataType, int128_t>) {
         auto units = IntDecoder<false>::template readInt<int64_t>();
-        Timestamp timestap;
-        if (_precision == TIMESTAMP_PRECISION::MILLIS) {
-          timestap = facebook::velox::util::fromUTCMillisParquet(units);
-        } else {
-          timestap = facebook::velox::util::fromUTCMicrosParquet(units);
-        }
+        Timestamp timestamp = precision_ == TIMESTAMP_PRECISION::MILLIS
+            ? util::fromUTCMillis(units)
+            : util::fromUTCMicros(units);
 
         toSkip =
-            visitor.process(*reinterpret_cast<int128_t*>(&timestap), atEnd);
+            visitor.process(*reinterpret_cast<int128_t*>(&timestamp), atEnd);
       } else {
         toSkip = visitor.process(
             IntDecoder<false>::template readInt<int64_t>(), atEnd);
@@ -88,6 +85,6 @@ class TimestampDecoder : public DirectDecoder<false> {
   }
 
  private:
-  TIMESTAMP_PRECISION _precision;
+  TIMESTAMP_PRECISION precision_;
 };
 } // namespace facebook::velox::dwio::common
diff --git a/velox/dwio/parquet/reader/PageReader.cpp b/velox/dwio/parquet/reader/PageReader.cpp
@@ -380,12 +380,12 @@ void PageReader::prepareDictionary(const PageHeader& pageHeader) {
           if (logicalType.TIMESTAMP.unit.__isset.MICROS) {
             for (auto i = dictionary_.numValues - 1; i >= 0; --i) {
               memcpy(&units, parquetValues + i * typeSize, typeSize);
-              values[i] = facebook::velox::util::fromUTCMicrosParquet(units);
+              values[i] = util::fromUTCMicros(units);
             }
           } else if (logicalType.TIMESTAMP.unit.__isset.MILLIS) {
             for (auto i = dictionary_.numValues - 1; i >= 0; --i) {
               memcpy(&units, parquetValues + i * typeSize, typeSize);
-              values[i] = facebook::velox::util::fromUTCMillisParquet(units);
+              values[i] = util::fromUTCMillis(units);
             }
           } else {
             VELOX_NYI("Unsupported timestamp unit");

diff --git a/velox/dwio/parquet/reader/ParquetReader.cpp b/velox/dwio/parquet/reader/ParquetReader.cpp
@@ -395,7 +395,7 @@ std::shared_ptr<const ParquetTypeWithId> ReaderBase::getParquetColumnInfo(
     if (veloxType->kind() == TypeKind::TIMESTAMP &&
         schemaElement.type == thrift::Type::INT64 &&
         !logicalType_.has_value()) {
-      // Construct logical type from deprecated converted type of Parquet
+      // Construct logical type from deprecated converted type of Parquet.
       thrift::TimestampType timestamp;
       timestamp.__set_isAdjustedToUTC(true);
       thrift::TimeUnit unit;

diff --git a/velox/type/TimestampConversion.h b/velox/type/TimestampConversion.h
@@ -134,11 +134,11 @@ inline Timestamp fromTimestampString(const StringView& str) {
 
 Timestamp fromDatetime(int64_t daysSinceEpoch, int64_t microsSinceMidnight);
 
-inline Timestamp fromUTCMillisParquet(int64_t millis) {
+inline Timestamp fromUTCMillis(int64_t millis) {
   return Timestamp(millis / 1000, (millis % 1000) * 1000000);
 }
 
-inline Timestamp fromUTCMicrosParquet(int64_t micros) {
+inline Timestamp fromUTCMicros(int64_t micros) {
   return Timestamp(micros / 1000000, (micros % 1000000) * 1000);
 }