Add pg_dump schema extraction and the catalog drift fingerprint - #4
Merged
Conversation
collect_schema(config, server_version_num) returns the schema DDL as a string together with a catalog fingerprint taken before the dump, so the after-collection recheck in task 10 covers drift during the dump itself. The client check runs first. An older pg_dump does not refuse a newer server outright in every case; it can emit a dump that is quietly wrong, and discovering that during a migration is expensive. probe_pg_dump_major opens no connection, and a test asserts nothing else runs when the check fails. build_pg_dump_args passes --schema-only --no-owner --no-privileges plus the -n and -N schema filters. Roles and grants are the customer's access control, not schema shape a migration needs. A test joins the whole argv and asserts none of the URL, host, user, password, or database appears in it. Scope filtering for the fingerprint happens in Python rather than in SQL. Building the WHERE clause from --schema-include would mean assembling SQL inline, which the house rule forbids so that --check-safety can see every statement. Filtering the returned rows has a second benefit: a schema the operator excluded cannot abort the run by changing underneath it. The canonical form separates fields and rows with ASCII unit and record separators. Neither can occur in a PostgreSQL identifier, so no pair of relation names can forge a field boundary and collide into one digest. There is a test for that rather than a comment claiming it. User schemas are selected with left(nspname, 3) <> 'pg_'. One comparison covers pg_catalog, pg_toast, pg_temp_N and pg_toast_temp_N, and it needs no backslash escaping inside a Python string. No relkind filter: a narrower one would only create blind spots, and TOAST relations are already excluded by the schema test. --schema-include validation moved to config time, so the operator finds out before we connect. PostgreSQL reserves the pg_ prefix, so pg_myschema cannot be a real user schema either. --schema-exclude is not validated; excluding something already out of scope is harmless. UnsupportedClientVersion is separate from UnsupportedServerVersion. The remedy differs: upgrade the local client, not the server. 125 tests pass. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
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.
Task 4 of the MVP plan. First real consumer of the
run_command/run_pg_dumphelpers from task 3.
collect_schema(config, server_version_num)returns the schema DDL as a stringtogether with a catalog fingerprint taken before the dump, so the after-collection
recheck in task 10 covers drift during the dump itself.
The client check runs first
An older
pg_dumpdoes not refuse a newer server outright in every case — it canemit a dump that is quietly wrong, and discovering that midway through a migration is
expensive.
probe_pg_dump_major()opens no connection, and a test asserts nothingelse executes when the check fails.
UnsupportedClientVersionis deliberately separate fromUnsupportedServerVersion:the remedy differs, since the operator upgrades the local client, not the server.
Dump arguments
--schema-only --no-owner --no-privileges, plus-n/-Nfor the schema filters.Roles and grants are the customer's access control, not schema shape a migration
needs. A test joins the whole argv and asserts none of the URL, host, user, password,
or database appears anywhere in it.
Fingerprint
SHA-256 over sorted
(nspname, relname, relkind)frompg_classjoined topg_namespace. Three details worth review:Scope filtering happens in Python, not in SQL. Building the
WHEREclause from--schema-includewould mean assembling SQL inline, which the house rule forbidsprecisely so
--check-safetycan see every statement the tool can issue.SQL_SCHEMA_FINGERPRINTstays a fixed module constant and the rows are filtered afterthey come back. That has a second benefit: a schema the operator excluded cannot abort
the run by changing underneath it. Tests cover both directions — out-of-scope churn is
ignored, in-scope churn is still detected.
Fields and rows are separated by ASCII unit and record separators (
\x1f,\x1e).Neither can occur in a PostgreSQL identifier, so
(public, ab)and(publica, b)cannot collide into the same digest. That is a test, not a comment claiming it.
User schemas are selected with
left(nspname, 3) <> 'pg_'rather than aLIKEpattern. One comparison covers
pg_catalog,pg_toast,pg_temp_Nandpg_toast_temp_N, and it needs no backslash escaping inside a Python string. There isno
relkindfilter — TOAST relations are already excluded by the schema test, and anarrower filter would only create blind spots.
Deviations from the plan
collect_schematakes the server version and runs three child processes, not two.The checklist said "one dump and one fingerprint query". It also checks the client
itself rather than trusting the orchestrator to remember, which adds a
pg_dump --versionprobe. Failing before we produce anything is worth the extraprocess.
--schema-includevalidation moved to config time, inbuild_postgres_config, sothe operator finds out before we connect. PostgreSQL reserves the
pg_prefix, sopg_myschemais rejected too — it cannot be a real user schema.--schema-excludeisnot validated: excluding something already out of scope is harmless.
Verification
python3 dbprofiler.py --check-safetyexits 0; the newSQL_SCHEMA_FINGERPRINTclears the relation allowlist, and a test asserts
audit_sql()returns nothingfor it.
pg_classforpg_statisticin that exact queryshape is caught by the audit with its reason.
ruff checkclean. Source re-verified as 3.9-parseable.