Skip to content

Reject v3-only types and initial defaults in schema updates on pre-v3 tables - #4160

Open
Ankitraj-builds wants to merge 1 commit into
apache:mainfrom
Ankitraj-builds:fix/reject-v3-schema-on-pre-v3
Open

Ankitraj-builds wants to merge 1 commit into
apache:mainfrom
Ankitraj-builds:fix/reject-v3-schema-on-pre-v3

Conversation

@Ankitraj-builds

Copy link
Copy Markdown
Contributor

Part of #4156

Rationale for this change

The format version check for v3-only types only ran in create_table. On a v1 or v2 table, update_schema().add_column(...) accepted timestamp_ns and unknown columns and a non-null default_value, and committed them.

This runs check_format_version_compatibility when a schema is added to the table metadata, and extends it to reject a non-null initial-default before v3, matching Java's Schema.checkCompatibility. Dropping a v3-only column still works, so an invalid table can be repaired (rule 2 of the issue).

This PR only covers schema changes from rule 1. Properties, encryption and the write-side rules are left for follow-up PRs.

Are these changes tested?

Yes. Two new tests in tests/catalog/test_catalog_behaviors.py, run for format versions 1 and 2 on every catalog:

  • test_update_schema_rejects_v3_only_types: adding timestamp_ns or unknown raises, and the table schema is unchanged.
  • test_update_schema_rejects_initial_default_before_v3: adding a column with a non-null default raises.

All existing tests in tests/table, tests/test_schema.py and tests/catalog/test_catalog_behaviors.py pass.

Are there any user-facing changes?

Yes. Schema updates on v1/v2 tables that add a v3-only type or a non-null initial default now raise ValueError instead of being committed.

@rambleraptor

Copy link
Copy Markdown
Collaborator

A side effect of most new validations is that they'll cause existing tests to fail in unexpected ways.

Could you take a look at the integration test failures?

This branch has not been deployed

No deployments
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.

2 participants