Re: [PR] GH-41476: [C++][Python][Parquet] Add time_is_adjusted_to_utc to parquet prope… [arrow]

["llama90 (via GitHub)" <gi...@apache.org>]

llama90 commented on code in PR #43268:
URL: https://github.com/apache/arrow/pull/43268#discussion_r1679248190


##########
cpp/src/arrow/dataset/file_parquet_test.cc:
##########
@@ -229,6 +236,57 @@ TEST_F(TestParquetFileFormat, WriteRecordBatchReaderCustomOptions) {
                     *actual_schema);
 }
 
+TEST_F(TestParquetFileFormat, WriteRecordBatchReaderTimeIsAdjustedToUTCFalse) {
+  std::vector<parquet::schema::NodePtr> parquet_fields;
+
+  // // Time32 millis, Time64 micros, Time64 nanos

Review Comment:
   typo too
   
   ```suggestion
     // Time32 millis, Time64 micros, Time64 nanos
   ```



##########
cpp/src/parquet/arrow/arrow_schema_test.cc:
##########
@@ -1183,6 +1183,76 @@ TEST_F(TestConvertArrowSchema, ParquetNestedComplianceEnabledNotNullable) {
   ASSERT_NO_FATAL_FAILURE(CheckFlatSchema(parquet_fields));
 }
 
+TEST_F(TestConvertArrowSchema, ParquetTimeIsAdjustedToUTC) {
+  std::vector<NodePtr> parquet_fields;
+  std::vector<std::shared_ptr<Field>> arrow_fields;
+
+  // // Time32 millis, Time64 micros, Time64 nanos
+
+  auto milli_logical = LogicalType::Time(true, LogicalType::TimeUnit::MILLIS);
+  auto milli_parquet = PrimitiveNode::Make("millis", Repetition::OPTIONAL, milli_logical,
+                                           ParquetType::INT32, -1);
+  parquet_fields.push_back(milli_parquet);
+  auto micro_logical = LogicalType::Time(true, LogicalType::TimeUnit::MICROS);
+  auto micro_parquet = PrimitiveNode::Make("micros", Repetition::OPTIONAL, micro_logical,
+                                           ParquetType::INT64, -1);
+  parquet_fields.push_back(milli_parquet);
+  auto nano_logical = LogicalType::Time(true, LogicalType::TimeUnit::NANOS);
+  auto nano_parquet = PrimitiveNode::Make("nanos", Repetition::OPTIONAL, nano_logical,
+                                          ParquetType::INT64, -1);
+  parquet_fields.push_back(nano_parquet);
+
+  auto milli_arrow = ::arrow::field("millis", ::arrow::time32(::arrow::TimeUnit::MILLI));
+  arrow_fields.push_back(milli_arrow);
+  auto micro_arrow = ::arrow::field("micros", ::arrow::time64(::arrow::TimeUnit::MICRO));
+  arrow_fields.push_back(milli_arrow);
+  auto nano_arrow = ::arrow::field("nanos", ::arrow::time64(::arrow::TimeUnit::NANO));
+  arrow_fields.push_back(nano_arrow);
+
+  ArrowWriterProperties::Builder builder;
+  builder.set_time_is_adjusted_to_utc();
+  auto arrow_properties = builder.build();
+
+  ASSERT_OK(ConvertSchema(arrow_fields, arrow_properties));
+
+  ASSERT_NO_FATAL_FAILURE(CheckFlatSchema(parquet_fields));
+}
+
+TEST_F(TestConvertArrowSchema, ParquetTimeIsAdjustedToUTCFalse) {
+  std::vector<NodePtr> parquet_fields;
+  std::vector<std::shared_ptr<Field>> arrow_fields;
+
+  // // Time32 millis, Time64 micros, Time64 nanos
+

Review Comment:
   ```suggestion
     // Time32 millis, Time64 micros, Time64 nanos
   ```



##########
cpp/src/parquet/arrow/arrow_schema_test.cc:
##########
@@ -1183,6 +1183,76 @@ TEST_F(TestConvertArrowSchema, ParquetNestedComplianceEnabledNotNullable) {
   ASSERT_NO_FATAL_FAILURE(CheckFlatSchema(parquet_fields));
 }
 
+TEST_F(TestConvertArrowSchema, ParquetTimeIsAdjustedToUTC) {
+  std::vector<NodePtr> parquet_fields;
+  std::vector<std::shared_ptr<Field>> arrow_fields;
+
+  // // Time32 millis, Time64 micros, Time64 nanos
+

Review Comment:
   ```suggestion
     // Time32 millis, Time64 micros, Time64 nanos
   ```



##########
cpp/src/arrow/dataset/file_parquet_test.cc:
##########
@@ -229,6 +236,57 @@ TEST_F(TestParquetFileFormat, WriteRecordBatchReaderCustomOptions) {
                     *actual_schema);
 }
 
+TEST_F(TestParquetFileFormat, WriteRecordBatchReaderTimeIsAdjustedToUTCFalse) {

Review Comment:
   Regarding the `WriteRecordBatchReaderTimeIsAdjustedToUTC` function, could we consider removing some of the duplication?



##########
python/pyarrow/_parquet.pyx:
##########
@@ -2074,6 +2075,13 @@ cdef shared_ptr[ArrowWriterProperties] _create_arrow_writer_properties(
         raise ValueError("Unsupported Writer Engine Version: {0}"
                          .format(writer_engine_version))
 
+    # time_is_adjusted_to_utc
+

Review Comment:
   ```suggestion
       # time_is_adjusted_to_utc
   ```



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org