cat_tools 0.2.3: fix backwards relkind mapping#36
Open
jnasbyupgrade wants to merge 1 commit into
Open
Conversation
relation__kind() and relation__relkind() (shipped through 0.2.2) mapped
pg_class.relkind values 'c', 'f', and 'm' backwards. Per pg_class.h the
correct mapping is 'c' = composite type, 'f' = foreign table,
'm' = materialized view; the code had 'c' -> materialized view,
'f' -> composite type, 'm' -> foreign table (and the inverse).
Changes:
- Correct the c/f/m arms in both mapping functions in cat_tools.sql.in.
- Add upgrade script sql/cat_tools--0.2.2--0.2.3.sql.in that CREATE OR
REPLACEs both functions so installed 0.2.2 gets the fix via
ALTER EXTENSION cat_tools UPDATE.
- Bump to 0.2.3 (control default_version, META).
- Rewrite the relation_type test: the old `kinds` view positionally zipped
enum_range('relation_type') against enum_range('relation_relkind'), which
merely re-encoded the enum declaration order and could never detect a
backwards mapping. Replace with hard-coded canonical (relkind, kind) pairs.
- Update CI version assertions from 0.2.2 to 0.2.3.
- HISTORY.asc 0.2.3 entry.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
relation__kind()(relkind -> type) andrelation__relkind()(type -> relkind), as shipped through 0.2.2, map thepg_class.relkindcodesc,f, andmbackwards.Per
pg_class.hthe correct mapping is:c= composite typef= foreign tablem= materialized viewThe code instead had
c-> materialized view,f-> composite type,m-> foreign table (and the inverse inrelation__relkind()). The other relkinds (r,i,S,t,v) were already correct.The fix
c/f/marms in both mapping functions insql/cat_tools.sql.in(enum order and all other arms untouched).sql/cat_tools--0.2.2--0.2.3.sql.inthatCREATE OR REPLACEs both functions with the corrected bodies, so an installed 0.2.2 picks up the fix viaALTER EXTENSION cat_tools UPDATE.cat_tools.controldefault_version,META), regenerate the trackedsql/cat_tools--0.2.3.sql.in.Test change
test/sql/relation_type.sqlpreviously built itskindsview by positionally zippingenum_range('relation_type')againstenum_range('relation_relkind'). Because the enum declaration order itself encoded the buggy pairing, that test was tautological — it could never catch a backwards mapping. It is rewritten to use hard-coded canonical(relkind, kind)VALUES pairs (only the relkinds present in the master enums), so the existing assertions now genuinely verify the mapping. No relkind-drift machinery added (out of scope).Upgrade path (verified locally, PG17 and PG12)
CI version assertions updated 0.2.2 -> 0.2.3; the existing extension-update-test now exercises 0.2.0/0.2.1 -> 0.2.2 -> 0.2.3, covering the new upgrade script.
This is a minimal bugfix release — no other behavior changes.
Known CI dependency: binary pg_upgrade legs will fail until rebased
The Binary pg_upgrade matrix legs that cross a catalog-column removal boundary (
→12,→13,→18) currently fail on this PR, and this is an expected, pre-existing cross-cutting dependency — not a defect in the relkind change:#18pg_upgrade-compatibility fix (omittingrelhasoids/relhaspkeyandattcacheofffrom_cat_tools.pg_class_v/_cat_tools.pg_attribute_v) was applied (commit 31204e1) only to the generatedcat_tools--0.2.2.sql.in/ upgrade scripts, and was never applied to the base sourcesql/cat_tools.sql.in.cat_tools--0.2.3.sql.ininherits the still-buggy base source, so a fresh0.2.3install builds views referencingrelhasoids(removed in PG12) andattcacheoff(removed in PG18). Binarypg_upgradeto those versions then fails duringpg_restore.fix-28-install-upgrade-scripts). Once that lands, 0.2.3 must be rebased onto it and the pg_upgrade legs will go green. This PR deliberately does not touchomit_column/pg_class_v/pg_attribute_vto avoid colliding with that work.All non-pg_upgrade CI legs pass: per-version tests (PG 10–18), extension-update-test (PG10), and review checks.
🤖 Generated with Claude Code