Core: Prevent dropping column which is referenced by active partition… #10352
Conversation
| @@ -533,6 +537,34 @@ private static Schema applyChanges( | |||
| } | |||
| } | |||
|
|
|||
| Map<Integer, List<Integer>> specToDeletes = Maps.newHashMap(); | |||
There was a problem hiding this comment.
This is a mapping from a spec to the requested fields to delete, where the fields are referenced as part of that spec.
It's a reverse mapping which makes surfacing the error details for the case where it's an active partition spec easier. Probably needs a better name though
There was a problem hiding this comment.
Maybe put this in a separate checkNotDeletingColumnsInActiveSpecs method.
|
|
amogh-jahagirdar
force-pushed
the
prevent-dropping-column-active-spec
branch
from
e0e0223
to
184d434
Compare
| for (int fieldIdToDelete : deletes) { | ||
| for (PartitionSpec spec : base.specs()) { | ||
| if (spec.schema().findField(fieldIdToDelete) != null) { | ||
| List<Integer> deletesForSpec = | ||
| specToDeletes.computeIfAbsent(spec.specId(), k -> Lists.newArrayList()); | ||
| deletesForSpec.add(fieldIdToDelete); | ||
| specToDeletes.put(spec.specId(), deletesForSpec); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we only need to do this logic if the current snapshot is not null, so probably want to re-organize the logic around that.
| .findAny(); | ||
| Preconditions.checkArgument( | ||
| !manifestReferencingActivePartition.isPresent(), | ||
| "Cannot delete field %s as it is used by an active partition spec %s", |
There was a problem hiding this comment.
Probably use the field name in the error message instead of the ID so it's more useful to a user
amogh-jahagirdar
force-pushed
the
prevent-dropping-column-active-spec
branch
from
184d434
to
3e454d2
Compare
amogh-jahagirdar
force-pushed
the
prevent-dropping-column-active-spec
branch
from
3e454d2
to
c62b649
Compare
Fixes #10234