fqaiser94 commented on code in PR #9860:
URL: https://github.com/apache/iceberg/pull/9860#discussion_r1519703373
##########
core/src/main/java/org/apache/iceberg/BaseReplacePartitions.java:
##########
@@ -87,31 +87,40 @@ public BaseReplacePartitions toBranch(String branch) {
@Override
public void validate(TableMetadata currentMetadata, Snapshot parent) {
- if (validateConflictingData) {
- if (dataSpec().isUnpartitioned()) {
- validateAddedDataFiles(
- currentMetadata, startingSnapshotId, Expressions.alwaysTrue(),
parent);
- } else {
- validateAddedDataFiles(currentMetadata, startingSnapshotId,
replacedPartitions, parent);
- }
- }
-
- if (validateConflictingDeletes) {
- if (dataSpec().isUnpartitioned()) {
- validateDeletedDataFiles(
- currentMetadata, startingSnapshotId, Expressions.alwaysTrue(),
parent);
- validateNoNewDeleteFiles(
- currentMetadata, startingSnapshotId, Expressions.alwaysTrue(),
parent);
- } else {
- validateDeletedDataFiles(currentMetadata, startingSnapshotId,
replacedPartitions, parent);
- validateNoNewDeleteFiles(currentMetadata, startingSnapshotId,
replacedPartitions, parent);
- }
- }
+ dataSpecs()
+ .forEach(
+ dataSpec -> {
+ if (validateConflictingData) {
+ if (dataSpec.isUnpartitioned()) {
+ validateAddedDataFiles(
+ currentMetadata, startingSnapshotId,
Expressions.alwaysTrue(), parent);
+ } else {
+ validateAddedDataFiles(
+ currentMetadata, startingSnapshotId, replacedPartitions,
parent);
+ }
+ }
+
+ if (validateConflictingDeletes) {
+ if (dataSpec.isUnpartitioned()) {
+ validateDeletedDataFiles(
+ currentMetadata, startingSnapshotId,
Expressions.alwaysTrue(), parent);
+ validateNoNewDeleteFiles(
+ currentMetadata, startingSnapshotId,
Expressions.alwaysTrue(), parent);
+ } else {
+ validateDeletedDataFiles(
+ currentMetadata, startingSnapshotId, replacedPartitions,
parent);
+ validateNoNewDeleteFiles(
+ currentMetadata, startingSnapshotId, replacedPartitions,
parent);
+ }
+ }
+ });
}
@Override
public List<ManifestFile> apply(TableMetadata base, Snapshot snapshot) {
- if (dataSpec().fields().size() <= 0) {
+ // TODO: I don't understand this, why delete all data? what if only part
of the table is
Review Comment:
Your explanation makes sense but I'm still a little lost when looking at the
code.
Specifically, I don't understand how this [line of
code](https://github.com/apache/iceberg/blob/131265d4ec79ccd114219b2989dd68694b473240/core/src/main/java/org/apache/iceberg/BaseReplacePartitions.java#L114)
is correct?
It's presumably checking `dataSpec().fields().size() <= 0` to figure out if
the current schema of the table is unpartitioned.
However, to my understanding, `dataSpec()` returns the spec of the files
added, which is not necessarily the same as the current schema of the table.
I feel like the correct way to do this check would be to do something along
the lines of `base.spec().isPartitioned()`?
Would love to understand this more as this class in general is the only part
of this PR I'm unsure about how to go about updating.
--
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]
