Skip to content

Conversation

@gabeiglio
Copy link
Contributor

@gabeiglio gabeiglio commented Sep 2, 2025

Closes #2166

Rationale for this change

We should block when an user wants to drop a column if that column is being referenced by either a active partition spec or sort order field.

Are these changes tested?

Yes, I added unit tests for every incompatible schema change in partitions and sort orders. Also added two new integration tests in test_catalog to test for this scenario

Are there any user-facing changes?

No

@gabeiglio gabeiglio force-pushed the partition-spec-validation branch from d4928bd to 81732d4 Compare September 2, 2025 07:05
@gabeiglio gabeiglio marked this pull request as ready for review September 2, 2025 07:06
Copy link
Contributor

@Anton-Tarazi Anton-Tarazi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, left a minor comment :)

…or sort order fields reference schema fields
@gabeiglio gabeiglio force-pushed the partition-spec-validation branch from 81732d4 to 7e47009 Compare December 7, 2025 12:49
@gabeiglio
Copy link
Contributor Author

@Anton-Tarazi thanks for the review (this PR went off my radar) I have rebased and applied recommended changes

@gabeiglio
Copy link
Contributor Author

cc: @Fokko @geruh This is somewhat adjacent to the recent PR especially this comment


for field in self.fields:
source_field = schema_fields.get(field.source_id)
if allow_missing_fields and source_field:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while parent_id:
parent_type = schema.find_type(parent_id)
if not parent_type.is_struct:
raise ValidationError("Invalid partition field parent: %s", parent_type)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should we also use f string here to align with others

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pyiceberg allows dropping the sort order column and causes table corruption on AWS Glue Catalog

4 participants