Skip to content

Compare constraint definitions in upgrade schema snapshots#262

Merged
pinodeca merged 2 commits into
microsoft:mainfrom
snvtac:snvtac/241-schema-snapshot-constraint-defs
Jun 23, 2026
Merged

Compare constraint definitions in upgrade schema snapshots#262
pinodeca merged 2 commits into
microsoft:mainfrom
snvtac:snvtac/241-schema-snapshot-constraint-defs

Conversation

@snvtac

@snvtac snvtac commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #241.

Summary

  • switch the upgrade-test schema snapshot constraint section to read from pg_constraint
  • include pg_get_constraintdef(con.oid) so CHECK bodies and full constraint definitions are compared
  • include con.convalidated so validated vs NOT VALID drift is caught
  • keep deterministic ordering and the existing seven-column snapshot shape

Testing

  • bash -n scripts/test-upgrade.sh
  • cargo fmt -p pg_durable -- --check
  • git diff --check

Not run: ./scripts/test-upgrade.sh because this local environment does not have pgrx PostgreSQL 17 installed (Error: pgrx PostgreSQL 17 not installed).

@pinodeca pinodeca added the test-pg18 Run CI tests against PostgreSQL 18 label Jun 23, 2026
NOT NULL column constraints are first-class pg_constraint rows on PG18+
but not on PG17 (they live in pg_attribute.attnotnull), which would make
the constraint snapshot PG-version-dependent. Per-column nullability is
already captured by the 'column' section via is_nullable, so filter out
contype = 'n' to keep the snapshot deterministic across PG versions while
preserving the CHECK body / convalidated comparison from issue microsoft#241.

@pinodeca pinodeca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Reviewed with Opus 4.8

Opus pointed out that the new approach checks NOT NULL constraints on PG 18 and suggested I run the CI for PG 18 (I did and it passed) to check for spurious failures due to auto-naming of NOT NULL constraints.

Nullability is already covered by column constraints, so checking NOT NULL constraints has limited value. Hence Opus suggested excluding those constraints and adding a comment about it in the test-upgrade.sh script.

I did so and pushed a commit on the PR branch in the author's fork.

@snvtac , thanks for setting "Maintainers are allowed to edit this pull request."

@pinodeca pinodeca merged commit 4cafc7e into microsoft:main Jun 23, 2026
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test-pg18 Run CI tests against PostgreSQL 18

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test-upgrade.sh: schema snapshot should compare CHECK bodies (pg_get_constraintdef) and convalidated

2 participants