wgtmac commented on code in PR #1241:
URL: https://github.com/apache/parquet-mr/pull/1241#discussion_r1438279868


##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetWriter.java:
##########
@@ -403,15 +405,29 @@ public ParquetWriter(Path file, Configuration conf, 
WriteSupport<T> writeSupport
 
     this.codecFactory = codecFactory;
     CompressionCodecFactory.BytesInputCompressor compressor = 
codecFactory.getCompressor(compressionCodecName);
+
+    final String extraMetadataConfPrefix = 
ParquetOutputFormat.EXTRA_WRITE_METADATA + ".";
+    final Map<String, String> extraMetadata = new 
HashMap<>(writeContext.getExtraMetaData());
+
+    conf.iterator().forEachRemaining(entry -> {

Review Comment:
   To support parquet writer without hadoop dependency, it would be good to 
implement this logic to `ParquetConfiguration` in two more overload 
constructors as well: 
https://github.com/apache/parquet-mr/blob/master/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetWriter.java#L341-L379



##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetOutputFormat.java:
##########
@@ -140,6 +140,8 @@ public static enum JobSummaryLevel {
   public static final String ENABLE_DICTIONARY = "parquet.enable.dictionary";
   public static final String VALIDATION = "parquet.validation";
   public static final String WRITER_VERSION = "parquet.writer.version";
+

Review Comment:
   Remove the blank line



##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetWriter.java:
##########
@@ -403,15 +405,29 @@ public ParquetWriter(Path file, Configuration conf, 
WriteSupport<T> writeSupport
 
     this.codecFactory = codecFactory;
     CompressionCodecFactory.BytesInputCompressor compressor = 
codecFactory.getCompressor(compressionCodecName);
+
+    final String extraMetadataConfPrefix = 
ParquetOutputFormat.EXTRA_WRITE_METADATA + ".";
+    final Map<String, String> extraMetadata = new 
HashMap<>(writeContext.getExtraMetaData());
+
+    conf.iterator().forEachRemaining(entry -> {
+      if (entry.getKey().startsWith(extraMetadataConfPrefix)) {

Review Comment:
   The caveat of this approach is that it wastes the entire loop if no extra 
metadata is provided, which is a common case.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to