Implement performing row-level delta to branches #5595
Conversation
amogh-jahagirdar
changed the title
amogh-jahagirdar
force-pushed
the
branch-rld
branch
from
61d90ed to
6880349
Compare
|
Er. It may make more sense to come back to row level delta after implementing branch writing for the other operations. There's more validation logic to modify and meaningful testing of row level delta is difficult without implementing branch writes for the other operations. |
| /** | ||
| * Validates that no delete files matching a filter have been added to the table since a starting | ||
| * snapshot. | ||
| * ToDo: Remove after branch writing implementation complete |
There was a problem hiding this comment.
We can remove the existing validate after we implement the different write operations because we will always have an ending snapshot. This is fine because validations are all protected/private.
| AssertHelpers.assertThrows( | ||
| "Should fail to commit when validating from non-ancestor snapshot", | ||
| ValidationException.class, | ||
| "Cannot commit, missing data files", |
There was a problem hiding this comment.
need to fix the message
|
Looks like @namrathamyske also has a PR for this https://github.com/apache/iceberg/pull/5234/files/ , the approach is fundamentally the same but I was thinking it makes more sense to iterate on one operation at a time rather than doing all the merging snapshot producers at once. We can have copies of the validate methods which accept an ending snapshot. That way we can validate the code and tests for a single operation at a time which seems easier for review IMO, but open to discussion |
|
Actually the approach in @namrathamyske works because for the other operations the branch write wouldn't be supported. It's just passing through the end snapshot. So in that case, that PR makes sense, we can just continue discussion on there then. |
1 participant
No description provided.