[GitHub] [parquet-mr] ggershinsky commented on a change in pull request #861: PARQUET-1970: Make minor releases source compatible

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

ggershinsky commented on a change in pull request #861:
URL: https://github.com/apache/parquet-mr/pull/861#discussion_r569412617



##########
File path: parquet-common/src/main/java/org/apache/parquet/io/OutputFile.java
##########
@@ -31,5 +31,7 @@
 
   long defaultBlockSize();
 
-  String getPath();
+  default String getPath() {
+    throw new UnsupportedOperationException();

Review comment:
       We're consuming this function in one of the encryption options:
   https://github.com/apache/parquet-mr/blob/master/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetWriter.java#L284
   If there is no encryption, or encryption is used with a crypto factory that doesn't need the file path - this exception will break the existing callers. A simple way to resolve this is by catching the exception here, and passing a null file path. This allows for these callers to run ok. The crypto factories that do need the file path sometimes (like the one we have in parquet-mr) should check `path==null` in situations where the file path is used, and throw an exception there. What do you think?




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

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