vinooganesh commented on code in PR #3397:
URL: https://github.com/apache/parquet-java/pull/3397#discussion_r3997787303
##########
parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java:
##########
@@ -585,6 +624,77 @@ public Builder withExtendedByteStreamSplitEncoding(boolean
enable) {
return this;
}
+ /**
+ * Set the full ALP configuration for FLOAT and DOUBLE columns.
+ *
+ * @param config the ALP configuration
+ * @return this builder for method chaining.
+ */
+ public Builder withAlp(AlpConfig config) {
Review Comment:
Yes, I think so, and it is in a8c584f64 as `withoutAlp(columnPath)`. The
reason it was missing is that ColumnProperty treated a null per-column value as
unset and fell back to the default, so it now checks containsKey instead and an
explicit null overrides; every other per-column property either cannot hold a
null or already defaults to one, so nothing else changes behaviour.
Two things had to come with it. The schema check was treating "column named
in the ALP property" as "ALP enabled here", so disabling ALP on an int column
failed with a message claiming ALP was enabled on it, and toString rendered a
disabled column as null again, which undoes your other nit. Both are fixed and
covered.
--
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]
