Jefffrey commented on code in PR #5158:
URL: https://github.com/apache/arrow-rs/pull/5158#discussion_r1412691279


##########
parquet/src/file/writer.rs:
##########
@@ -323,14 +323,27 @@ impl<W: Write + Send> SerializedFileWriter<W> {
             None => Some(self.kv_metadatas.clone()),
         };
 
+        // We only include ColumnOrder for leaf nodes.
+        // Currently only supported ColumnOrder is TypeDefinedOrder so we set 
this
+        // for all leaf nodes.

Review Comment:
   In future there may be new supported order, see here: 
https://github.com/apache/parquet-format/pull/221



##########
parquet/src/file/writer.rs:
##########
@@ -323,14 +323,27 @@ impl<W: Write + Send> SerializedFileWriter<W> {
             None => Some(self.kv_metadatas.clone()),
         };
 
+        // We only include ColumnOrder for leaf nodes.
+        // Currently only supported ColumnOrder is TypeDefinedOrder so we set 
this
+        // for all leaf nodes.
+        // Even if the column has an undefined sort order, such as INTERVAL, 
this
+        // is still technically the defined TYPEORDER so it should still be 
set.
+        let column_orders = (0..self.schema_descr().num_columns())
+            .map(|_| parquet::ColumnOrder::TYPEORDER(parquet::TypeDefinedOrder 
{}))
+            .collect();
+        // This field is optional, perhaps in cases where no min/max fields 
are set
+        // in any Statistics or ColumnIndex object in the whole file.
+        // But for simplicity we always set this field.
+        let column_orders = Some(column_orders);

Review Comment:
   For reference, looks like parquet-mr does something similar
   
   
https://github.com/apache/parquet-mr/blob/97e6ba8e8343a7274e530a83496a7933cc5ffc01/parquet-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java#L244-L260



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