vinooganesh commented on code in PR #3397:
URL: https://github.com/apache/parquet-java/pull/3397#discussion_r4019015378
##########
parquet-format-structures/src/main/thrift/parquet.thrift:
##########
@@ -636,6 +636,21 @@ enum Encoding {
Support for INT32, INT64 and FIXED_LEN_BYTE_ARRAY added in 2.11.
*/
BYTE_STREAM_SPLIT = 9;
+
Review Comment:
Done as #3791, which merged yesterday, and I've merged master into this
branch. `parquet.thrift` here is now byte-identical to upstream 2.14.0, so the
local `ALP = 10` block and the "LOCAL ADDITION" notice are gone and the entry
comes straight from parquet-format.
##########
parquet-hadoop/src/test/java/org/apache/parquet/format/converter/TestParquetMetadataConverter.java:
##########
@@ -471,6 +471,10 @@ public void testLogicalToConvertedTypeConversion() {
public void testEnumEquivalence() {
ParquetMetadataConverter parquetMetadataConverter = new
ParquetMetadataConverter();
for (org.apache.parquet.column.Encoding encoding :
org.apache.parquet.column.Encoding.values()) {
+ // Skip ALP encoding as it's not yet in the parquet-format specification
Review Comment:
Agreed, and that's now done. The one wrinkle was that #3791 had to add a
temporary ALP skip back into `testEnumEquivalence` (the bump fails on master
otherwise), and merging master quietly brought it back here. It's removed again
in 7b48dd189, so ALP round trips in both directions like every other encoding.
--
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]
