wgtmac commented on code in PR #3526:
URL: https://github.com/apache/parquet-java/pull/3526#discussion_r3564253902
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/CodecFactory.java:
##########
@@ -271,15 +291,124 @@ public BytesDecompressor
getDecompressor(CompressionCodecName codecName) {
}
protected BytesCompressor createCompressor(CompressionCodecName codecName) {
- CompressionCodec codec = getCodec(codecName);
- return codec == null ? NO_OP_COMPRESSOR : new
HeapBytesCompressor(codecName, codec);
+ return compressorForCodec(codecName, getCodec(codecName));
}
protected BytesDecompressor createDecompressor(CompressionCodecName
codecName) {
CompressionCodec codec = getCodec(codecName);
return codec == null ? NO_OP_DECOMPRESSOR : new
HeapBytesDecompressor(codec);
}
+ protected BytesCompressor createCompressorAtLevel(CompressionCodecName
codecName, int level) {
+ return compressorForCodec(codecName, getCodecAtLevel(codecName, level));
+ }
+
+ private BytesCompressor compressorForCodec(CompressionCodecName codecName,
CompressionCodec codec) {
+ return codec == null ? NO_OP_COMPRESSOR : new
HeapBytesCompressor(codecName, codec);
+ }
+
+ static void validateZstdLevel(int level) {
+ if (level < 1 || level > 22) {
Review Comment:
IIRC, ZSTD supports negative levels to be even faster?
##########
parquet-hadoop/src/test/java/org/apache/parquet/hadoop/TestDirectCodecFactory.java:
##########
@@ -285,4 +290,158 @@ public void cachingKeysZstd() {
Assert.assertEquals(codec_2_1, codec_2_2);
Assert.assertNotEquals(codec_2_1, codec_5_1);
}
+
+ @Test
+ public void levelAwareCompressor_sameLevel_returnsCachedInstance() {
Review Comment:
These test case names are not consistent with this repo.
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetWriter.java:
##########
@@ -569,6 +575,32 @@ public SELF withCompressionCodec(CompressionCodecName
codecName) {
return self();
}
+ /**
+ * Override the compression codec for a specific column.
+ *
+ * @param columnPath dot-string path of the column (e.g. {@code "my_col"})
+ * @param codecName the codec to use for that column
+ * @return this builder for method chaining.
+ */
+ public SELF withCompressionCodec(String columnPath, CompressionCodecName
codecName) {
Review Comment:
Should we provide a convenience method to set codec name and level in a
single shot?
##########
parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java:
##########
@@ -388,7 +415,9 @@ public String toString() {
+ "Page row count limit to " + getPageRowCountLimit() + '\n'
+ "Writing page checksums is: " + (getPageWriteChecksumEnabled() ?
"on" : "off") + '\n'
+ "Statistics enabled: " + statisticsEnabled + '\n'
- + "Size statistics enabled: " + sizeStatisticsEnabled;
+ + "Size statistics enabled: " + sizeStatisticsEnabled + '\n'
+ + "Per-column codecs: " + columnCodecs + '\n'
Review Comment:
Does this produce too much noise if not enabled? Perhaps we can shorten them
into a single line or even omit them when disabled.
--
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]