[GitHub] [drill] vdiravka commented on a change in pull request #2351: DRILL1282: Move parquet to use v2 format as default

[GitBox <gi...@apache.org>]

vdiravka commented on a change in pull request #2351:
URL: https://github.com/apache/drill/pull/2351#discussion_r738450978



##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFormatPlugin.java
##########
@@ -167,6 +167,10 @@ public RecordWriter getRecordWriter(FragmentContext context, ParquetWriter write
     options.put(ExecConstants.PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS,
         context.getOptions().getOption(ExecConstants.PARQUET_WRITER_USE_PRIMITIVE_TYPES_FOR_DECIMALS).bool_val.toString());
 
+    options.put(
+      ExecConstants.PARQUET_WRITER_FORMAT_VERSION,
+      context.getOptions().getOption(ExecConstants.PARQUET_WRITER_FORMAT_VERSION).string_val);

Review comment:
       ```suggestion
   context.getOptions().getOption(ExecConstants.PARQUET_WRITER_FORMAT_VERSION).getValue());
   ```
   

##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
##########
@@ -375,6 +375,12 @@ private ExecConstants() {
   public static final OptionValidator PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS_VALIDATOR = new EnumeratedStringValidator(PARQUET_WRITER_LOGICAL_TYPE_FOR_DECIMALS,
       new OptionDescription("Parquet writer logical type for decimal; supported types \'fixed_len_byte_array\' and \'binary\'"),
       "fixed_len_byte_array", "binary");
+  public static final String PARQUET_WRITER_FORMAT_VERSION = "store.parquet.writer.format_version";
+  public static final OptionValidator PARQUET_WRITER_FORMAT_VERSION_VALIDATOR = new EnumeratedStringValidator(
+    PARQUET_WRITER_FORMAT_VERSION,
+    new OptionDescription("Parquet format version used for storing Parquet output.  Allowed values: PARQUET_1_0, PARQUET_2_0"),
+    "PARQUET_1_0", "PARQUET_2_0"

Review comment:
       Optionally you can create String array of parquet versions (possibly in `ParquetFormatPlugin`) and use it for description and `EnumeratedStringValidator` values. Also (minor) consider adding short description.
   
   ```
   public static final String[] parquetVersions= {"PARQUET_1_0", "PARQUET_2_0"};
   ```
   ```
   public static final OptionValidator PARQUET_WRITER_FORMAT_VERSION_VALIDATOR = new EnumeratedStringValidator(
       PARQUET_WRITER_FORMAT_VERSION,
       new OptionDescription("Parquet format version used for storing Parquet output.  Allowed values:" + Arrays.toString(parquetVersions)), "Parquet format version",
       parquetVersions);
   ```




-- 
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: dev-unsubscribe@drill.apache.org

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