-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PARQUET-1126: Write unencrypted Parquet files without Hadoop #1376
Conversation
… Hadoop in the classpath. Relates to PARQUET-1126.
|
||
Configuration hadoopConf = ConfigurationUtil.createHadoopConfiguration(fileParquetConfig); | ||
URI path = tempFilePath == null ? null : tempFilePath.toUri(); | ||
return createEncryptionProperties( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will trigger a second loading of the factory class. Can we replace ll40-41 with the l52 code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ggershinsky , Thank you for taking a look at the PR. This is a good point. I corrected it in the latest commit.
@ggershinsky , Thank you for the approval. Is anything else necessary to merge this? |
I just merged this. Thanks @dlvenable and @ggershinsky! |
* Internal changes to allow writing unencrypted Parquet without needing Hadoop in the classpath. Relates to PARQUET-1126. * Avoid loading encryption factory twice.
If you want to write an unencrypted Parquet file without Hadoop, the existing code will use Hadoop to try to get encryption properties.
parquet-java/parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetWriter.java
Lines 388 to 393 in fbe13d8
However, if you have these
null
, we really didn't need to go through Hadoop. Also, it calls a helper method inParquetOutputFormat
. This class inherits from Hadoop'sFileOutputFormat
. So calling this method at all, requires Hadoop classes. To resolve this, I moved this helper into a package-protectedEncryptionPropertiesHelper
class.Closes #1497