Remove reg* pseudotype columns; add object info functions (reconciles new_features) - #17
Open
jnasbyupgrade wants to merge 19 commits into
Open
Remove reg* pseudotype columns; add object info functions (reconciles new_features)#17jnasbyupgrade wants to merge 19 commits into
jnasbyupgrade wants to merge 19 commits into
Conversation
e9c24de Fix pg_regress on versions > 12 (#5) c0af00f Improvements to HISTORY.asc 6e8f2a7 Allow use of sudo when installing an extension 705f1ec Don't run clean as part of make test 370fa8e Create test/sql during setup git-subtree-dir: pgxntool git-subtree-split: e9c24de986ddc85bbd1fb3149076888d075ce100
Ditch the old travis setup. Pulls in some pgxntool changes as well.
# Conflicts: # .gitignore # pgxntool/HISTORY.asc # pgxntool/base.mk # pgxntool/setup.sh
Vendor Postgres-Extensions/linter as a git submodule at .vendor/linter,
following the same pattern already adopted in cat_tools: a thin
self-initializing lint.mk hand-off (so `make lint` works right after a
plain `git clone`, no --recurse-submodules needed), LINT_TARGETS scoped
to sql/object_reference.sql and test/ (excluding the frozen, never
hand-edited versioned install files under sql/, e.g.
object_reference--0.1.0.sql/--stable.sql), and a CI job that runs
`make lint` directly -- the same entry point a developer uses locally
-- so the self-init logic is actually exercised, not just the rule
checking.
The `include lint.mk` is guarded on .git being present: a tarball build
(PGXN distribution, `git archive` with no .git) has no submodule to
initialize, and Make resolves every `include` before running any
target regardless of which one was requested, so an unguarded rule
would break `make`/`make install` entirely for a tarball build, not
just `make lint`.
Fixes the real pre-existing style findings this first run turned up
(52 total): most were commented-out SQL marked as prose comments
instead of using the linter's `EXCLUDED CODE` disabled-code convention
(missing " * " prefixes flagged as comment-line-prefix/comment-opening
violations); one COPY data block's `secondary` column intentionally
mirrors pg_catalog's own type display name ("integer" for int4) rather
than following prefer-short-type, so it's suppressed via a scoped
disable-block region instead of being "fixed" into incorrect test data.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…able pgxn install --unstable cat_tools resolves to the newest release actually published to the PGXN package index, which is still 0.2.1 (2017) -- it fails standalone on modern PostgreSQL with "column oid specified more than once" at CREATE EXTENSION. cat_tools 0.3.0 fixes this but hasn't been uploaded to PGXN yet, only tagged in git, so build it from that tag directly until PGXN has it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cat_tools 0.3.0 is the first version of cat_tools this extension's test suite has ever actually run against (pgxn install --unstable previously resolved to a version that failed CREATE EXTENSION outright, so none of this was exercised before now): - cat_tools.function__arg_types_text() is deprecated in 0.3.0 in favor of cat_tools.routine__parse_arg_types_text() (same signature, same body, just renamed) and now emits a WARNING on every call. Switch to the non-deprecated name directly rather than carrying the warning forward. - cat_tools 0.3.0's object_type enum gained two new members, "partitioned table" and "partitioned index". object_reference has no per-object-type special-casing (object identity is tracked generically via classid/objid), so these are already handled by the existing code the same way "table" and "index" are -- they just weren't yet reflected in object_reference's own classification of which object types are covered by a test. Add them to the untested() set so object_reference.untested_srf() ∪ tested types ∪ unsupported() still accounts for every member of the enum; a follow-up can add dedicated create/drop test coverage for them. - Regenerate sql/object_reference--stable.sql (auto-generated from sql/object_reference.sql) and test/expected/zzz_build.out (line numbers in test/temp_load.not_sql shift by one now that a source comment line changed) to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…back # Conflicts: # .github/workflows/ci.yml
The linter PR's comment-style edits (block comments -> "EXCLUDED CODE") land in sql/object_reference.sql, which test/build's raw-load sanity check (zzz_build) re-parses independently of pg_regress's own SQL loading and reports line numbers for. Regenerated via `make results` after confirming zero raw "not ok" TAP assertions anywhere in test/results/*.out (the earlier copy of this file, committed before the linter-PR merge, was captured while a leftover extension install from prior manual testing was still on disk in the sandbox, which produced extra NOTICEs that don't appear in a real clean run -- this version comes from a fully clean rebuild). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Verified directly: pg_get_object_address('partitioned table', ...) throws
"unrecognized object type" -- PostgreSQL's real object-address API only
knows the base "table"/"index" object types cat_tools's "partitioned table"/
"partitioned index" are derived from. object_reference actively calls
pg_get_object_address() on every object_type it tracks (not just in the
disabled sanity CHECK), so these two types would break identity tracking
outright rather than merely lacking test coverage -- the earlier "untested"
classification undersold the actual constraint.
Matches the classification (and the exact reasoning/wording) already landed
independently on the separate, longer-running new_features branch (PR #2),
which reaches unsupported() via the same cat_tools 0.3.0 enum growth. Update
test/sql/all.sql's own sanity-check of the unsupported set to match.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reconciles the substantive feature delta from new_features (PR #2) onto the current 'stable' baseline (post PR #5/#16: pgxn-tools testing, cat_tools 0.3.0, and the linter): - _object_reference._object_oid: drop the per-catalog regclass/regconfig/ regdictionary/regnamespace/regoperator/regprocedure/regtype columns and their unique indexes plus the count_nulls-backed null_count trigger that enforced "exactly one is set". classid is now plain oid and object_oid (also NOT NULL) is the sole identifier column, so there's nothing left to arbitrate between. - _object_reference._object_v / _object_v__for_update: drop the reg* columns from the column list to match. - _object_reference._object_oid__add: replace the dynamic, format()-built INSERT that picked a reg* column based on cat_tools.object__reg_type() with a plain INSERT into object_oid. - Drop the count_nulls search_path DO block (dead now that the trigger using it is gone) and the count_nulls dependency throughout (control, Makefile, test setup). - Add object_reference.object__describe()/object__identity(), thin wrappers around pg_describe_object()/pg_identify_object(); and object__cleanup(), which best-effort deletes an object record (ignoring foreign_key_violation if it's still referenced elsewhere). Wire object__cleanup() up to a new AFTER DELETE trigger on object_group__object so removing an object from its last group automatically attempts cleanup. - _object_v__for_update (the getsert core): refuse to track objects living in a pg_temp*/pg_toast_temp* schema, since a tracked reference would outlive the temporary object it points to. - test/sql/object_group.sql: switch the two scratch tables from TEMP to regular tables (object__getsert now rejects temp objects) and add coverage for the new automatic-cleanup trigger. - test/sql/base.sql: replace the count_nulls-relocation test (relocation was already unsupported and the whole extension no longer depends on count_nulls) with coverage for object_oid, object__describe(), object__identity(), and temp-object rejection. sql/object_reference--0.1.0.sql (the frozen historical release) and the META files are untouched. default_version stays 'stable'; sql/object_reference--stable.sql is regenerated to match sql/object_reference.sql. make lint and make test (including the dump/restore test) pass on both PostgreSQL 12 and 17. Supersedes PR #2 (new_features) and, for the update/upgrade test infrastructure built on top of it, sets up the rebuild of PR #3. Co-Authored-By: Claude Sonnet 5 <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:
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
1 participant
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.
Summary
This is the first PR of a 3-PR stack. It reconciles the real, substantive
feature delta from
new_features(#2) onto the currentstable-versionbaseline (branched fresh from
master, which already has #12's real 0.1.0restore, and merges in #5/#16's pgxn-tools testing + cat_tools 0.3.0 fix +
SQL linter — both still open at the time of this PR, but their content was
needed for this branch to have working CI/lint to validate itself against).
reg*pseudotype columns (regclass/regconfig/regdictionary/regnamespace/regoperator/regprocedure/regtype) from_object_reference._object_oidin favor of a single plainoidcolumn(
object_oid), removing the count_nulls-backednull_counttrigger thatused to arbitrate between them and simplifying
_object_oid__add's insertaccordingly.
classidis now plainoidtoo.count_nullsdependency entirely (the search_pathworkaround DO block,
object_reference.control'srequires, the Makefileinstall target, and test setup) — the only thing that needed it was the
trigger removed above.
object_reference.object__describe()/object__identity()(thinwrappers around
pg_describe_object()/pg_identify_object()), andobject__cleanup()(best-effort delete, ignoringforeign_key_violation),wired up to a new
AFTER DELETEtrigger onobject_group__objectsoremoving an object from its last group auto-attempts cleanup.
object__getsert's core (_object_v__for_update) now refuses to trackobjects living in a
pg_temp*/pg_toast_temp*schema.object_group.sql's scratch tables switch fromTEMPtoregular tables (temp objects are now rejected) plus new coverage for the
cleanup trigger;
base.sqlswaps the now-moot count_nulls-relocation testfor coverage of
object_oid,object__describe(),object__identity(),and temp-object rejection.
sql/object_reference--0.1.0.sql(the frozen historical release) and theMETA files are untouched — this repo's convention is that frozen versioned
SQL files are never hand-edited after release.
default_versionstays'stable';sql/object_reference--stable.sqlis regenerated fromsql/object_reference.sqlviamake.This PR supersedes #2 (
new_features) — please close that once thismerges. It also obsoletes #3's update/upgrade test infrastructure (built
against the old
0.2.0/reg*-pseudotype model); rebuilding that against thisbranch is a separate follow-up, not included here.
Test plan
make lint— clean (0 findings) againstsql/object_reference.sqland
test/make test— all 7 regression tests + the dump/restore test pass onPostgreSQL 17
make test— all 7 regression tests + the dump/restore test pass onPostgreSQL 12 (the repo's floor per CI's matrix)
test/expected/*.outregenerated viamake results(never hand-edited)🤖 Generated with Claude Code