#12081: "Add deleteFileThreshold parameter to SizeBasedDataRewriter, update logic, and include tests" #12133
+27
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Title of this pr looks incorrect? This is a new feature and not a fix. The PR also would require some tests to prove the new parameter is working as expected. |
jangalasriramd7
changed the title
|
I have changed the PR title. |
core/src/main/java/org/apache/iceberg/actions/SizeBasedDataRewriter.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/iceberg/actions/SizeBasedDataRewriter.java
Outdated
Show resolved
Hide resolved
|
@RussellSpitzer Addressed your comments:
|
nastra
reviewed
Jan 30, 2025
core/src/main/java/org/apache/iceberg/actions/SizeBasedDataRewriter.java
Show resolved
Hide resolved
nastra
reviewed
Jan 30, 2025
| @@ -67,6 +68,8 @@ public Set<String> validOptions() { | |||
| public void init(Map<String, String> options) { | |||
| super.init(options); | |||
| this.deleteFileThreshold = deleteFileThreshold(options); | |||
| this.deleteRatioThreshold = PropertyUtil.propertyAsDouble( | |||
There was a problem hiding this comment.
we also need to validate that 0.0 > threshold <= 1.0. That would also require some additional unit tests that check for valid/invalid parameters
There was a problem hiding this comment.
also this.deleteRatioThreshold needs to be used in tooHighDeleteRatio()
nastra
reviewed
Jan 30, 2025
core/src/main/java/org/apache/iceberg/actions/SizeBasedDataRewriter.java
Show resolved
Hide resolved
nastra
reviewed
Jan 30, 2025
core/src/test/java/org/apache/iceberg/actions/TestSizeBasedRewriter.java
Show resolved
Hide resolved
|
Could you please help me out in resolving the error. This build must be run with JDK 11 or 17 or 21 but was executed with JDK 23
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refines the SizeBasedDataRewriter class in the Apache Iceberg project, addressing issues related to the constructor initialization and logic for delete file threshold handling. It makes the class more robust by improving its flexibility and functionality in the context of delete-based data file rewriting.
Constructor Updates:
Refactored constructors to properly initialize the deleteRatioThreshold and deleteFileThreshold parameters.
Added an additional constructor that allows specifying the deleteRatioThreshold value explicitly while maintaining backward compatibility with the default constructor.