jerolba commented on code in PR #1035:
URL: https://github.com/apache/parquet-mr/pull/1035#discussion_r1129276294
##
parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java:
##
@@ -477,15 +477,15 @@ public Builder withMaxBloomFilterBytes(int
maxBloomFilterBytes) {
* @return this builder for method chaining
*/
public Builder withBloomFilterNDV(String columnPath, long ndv) {
- Preconditions.checkArgument(ndv > 0, "Invalid NDV for column \"%s\":
%d", columnPath, ndv);
+ Preconditions.checkArgument(ndv > 0, "Invalid NDV for column \"%s\":
%s", columnPath, ndv);
this.bloomFilterNDVs.withValue(columnPath, ndv);
// Setting an NDV for a column implies writing a bloom filter
this.bloomFilterEnabled.withValue(columnPath, true);
return this;
}
public Builder withBloomFilterFPP(String columnPath, double fpp) {
- Preconditions.checkArgument(fpp > 0.0 && fpp < 1.0, "Invalid FPP for
column \"%s\": %d", columnPath, fpp);
+ Preconditions.checkArgument(fpp > 0.0 && fpp < 1.0, "Invalid FPP for
column \"%s\": %s", columnPath, fpp);
Review Comment:
The changed code was incorrect and in case of failure in the check, the code
throws a `java.util.IllegalFormatConversionException`
The existing [code behind checkArgument
method](https://github.com/jerolba/parquet-mr/blob/5078cc629074046b5699544c33256ca980ab2761/parquet-common/src/main/java/org/apache/parquet/Preconditions.java#L243)
converts all parameters to String before applying the String.format.
If String.format finds an incorrect type, throws
`IllegalFormatConversionException`.
For example try this:
```java
String output = String.format("Integer value: %d", "10");
```
Then, existing Preconditions implementation forces to use Strings messages
with %s.
I reviewed all calls to `checkArgument` to fix the incorrect format.
--
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: dev-unsubscr...@parquet.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
