Part of #93.
Enum contents are one of the easiest things to get wrong across an ALTER EXTENSION UPDATE: the install script states the full label list in CREATE TYPE … AS ENUM (…), while an update script has to say ALTER TYPE … ADD VALUE — two different statements expressing the same change, one label at a time, and adding a label to one file without the other is easy to miss in review.
A text/parser-level check is the wrong mechanism for this. Making a static checker understand that a changed CREATE TYPE … AS ENUM list corresponds to a set of ALTER TYPE … ADD VALUE statements means teaching it that two unrelated statement forms mean the same thing — exactly the sort of special-casing that makes such a tool grow without bound. Enum label support was deliberately dropped from #92 for this reason.
The right mechanism is a test asserting each enum's contents. The suite already runs identical expected output against a fresh install, an updated database, and a pg_upgraded one, so an enum whose install and update scripts disagree fails on its own, in whichever mode diverges, naming the enum. No new comparison machinery, and it covers paths a static check never sees.
What this issue is
Relying on those tests only works if they actually exist for every enum, and nothing currently enforces that. So: a check that every enum the extension defines has a test asserting its contents.
The design constraint that makes this cheap: mandate something trivially greppable on both sides. Enum definitions are already trivial to extract from the install source —
$ grep -oE 'CREATE TYPE [a-z_.]+ AS ENUM' sql/cat_tools.sql.in | sed 's/CREATE TYPE //;s/ AS ENUM//'
cat_tools.constraint_type
cat_tools.procedure_type
cat_tools.relation_type
cat_tools.relation_relkind
cat_tools.routine_prokind
cat_tools.routine_type
cat_tools.routine_proargmode
cat_tools.routine_argument_mode
cat_tools.routine_provolatile
cat_tools.routine_volatility
cat_tools.routine_proparallel
cat_tools.routine_parallel_safety
cat_tools.object_type
— 13 of them. The other half is picking a convention in the test files that is equally greppable, so the check is a set difference between two greps rather than anything that has to parse SQL. That convention is the substance of this issue: it should be obvious to write, hard to write wrongly, and impossible to satisfy accidentally.
Current coverage is partial
test/helpers/enum_mapping.sql already does roundtrip testing for paired enums (a "char"-based one and its human-readable counterpart), preceded by a size check — so some enums are covered, via that helper. test/sql/enum.sql tests cat_tools' own enum_range/enum_range_srf functions rather than the label contents of the extension's enum types.
Worth establishing before designing the convention: which of the 13 currently have their contents asserted, and whether the existing helper's size check is strong enough to count (a size check catches an added label but not a renamed or reordered one).
Notes
- A size check alone is weaker than an explicit label list — it catches additions but not substitutions.
- Label order matters for enums (
enumsortorder), and ALTER TYPE … ADD VALUE takes optional BEFORE/AFTER, so a fresh install and an updated one can agree on the label set while disagreeing on order. Decide whether the assertion covers order.
- If an explicit comparison is ever wanted beyond per-enum tests, the better shape is comparing a freshly-created database against an updated one, not comparing script text.
Part of #93.
Enum contents are one of the easiest things to get wrong across an
ALTER EXTENSION UPDATE: the install script states the full label list inCREATE TYPE … AS ENUM (…), while an update script has to sayALTER TYPE … ADD VALUE— two different statements expressing the same change, one label at a time, and adding a label to one file without the other is easy to miss in review.A text/parser-level check is the wrong mechanism for this. Making a static checker understand that a changed
CREATE TYPE … AS ENUMlist corresponds to a set ofALTER TYPE … ADD VALUEstatements means teaching it that two unrelated statement forms mean the same thing — exactly the sort of special-casing that makes such a tool grow without bound. Enum label support was deliberately dropped from #92 for this reason.The right mechanism is a test asserting each enum's contents. The suite already runs identical expected output against a fresh install, an updated database, and a pg_upgraded one, so an enum whose install and update scripts disagree fails on its own, in whichever mode diverges, naming the enum. No new comparison machinery, and it covers paths a static check never sees.
What this issue is
Relying on those tests only works if they actually exist for every enum, and nothing currently enforces that. So: a check that every enum the extension defines has a test asserting its contents.
The design constraint that makes this cheap: mandate something trivially greppable on both sides. Enum definitions are already trivial to extract from the install source —
— 13 of them. The other half is picking a convention in the test files that is equally greppable, so the check is a set difference between two greps rather than anything that has to parse SQL. That convention is the substance of this issue: it should be obvious to write, hard to write wrongly, and impossible to satisfy accidentally.
Current coverage is partial
test/helpers/enum_mapping.sqlalready does roundtrip testing for paired enums (a"char"-based one and its human-readable counterpart), preceded by a size check — so some enums are covered, via that helper.test/sql/enum.sqltests cat_tools' ownenum_range/enum_range_srffunctions rather than the label contents of the extension's enum types.Worth establishing before designing the convention: which of the 13 currently have their contents asserted, and whether the existing helper's size check is strong enough to count (a size check catches an added label but not a renamed or reordered one).
Notes
enumsortorder), andALTER TYPE … ADD VALUEtakes optionalBEFORE/AFTER, so a fresh install and an updated one can agree on the label set while disagreeing on order. Decide whether the assertion covers order.