diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 318fb89..bb84188 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -14,29 +14,36 @@ jobs: build: runs-on: ubuntu-latest + # Matrix validates the MSRV-ish floor (1.88.0) AND the toolchain used by + # the Dockerfile (1.95). Keep the upper bound in sync with `Dockerfile`. + strategy: + fail-fast: false + matrix: + rust: ["1.88.0", "1.95.0"] + steps: - uses: actions/checkout@v4 - + - name: Install Rust uses: dtolnay/rust-toolchain@stable with: - toolchain: 1.88.0 + toolchain: ${{ matrix.rust }} components: rustfmt, clippy - + - name: Check format - run: + run: cd ${{ github.workspace }}/app ; cargo fmt -- --check - name: Check clippy - run: + run: cd ${{ github.workspace }}/app ; cargo clippy --all-targets --all-features - name: Build - run: + run: cd ${{ github.workspace }}/app ; cargo build --verbose - name: Run tests - run: + run: cd ${{ github.workspace }}/app ; cargo test --verbose @@ -48,7 +55,13 @@ jobs: steps: - uses: actions/checkout@v4 - + + - name: Read crate version + id: crate + run: | + version=$(grep -m1 '^version' app/Cargo.toml | cut -d '"' -f2) + echo "version=${version}" >> "$GITHUB_OUTPUT" + - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 @@ -79,6 +92,8 @@ jobs: push: false tags: pgc:test labels: ${{ steps.meta.outputs.labels }} + build-args: | + PGC_VERSION=${{ steps.crate.outputs.version }} cache-from: type=gha cache-to: type=gha,mode=max load: true @@ -98,6 +113,8 @@ jobs: push: true tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} + build-args: | + PGC_VERSION=${{ steps.crate.outputs.version }} cache-from: type=gha cache-to: type=gha,mode=max continue-on-error: true diff --git a/CHANGELOG b/CHANGELOG index 5a6b4cb..70f4ff8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,76 @@ +2026-04-23 v1.0.17 + + Bug fixes: + - PostgreSQL version agnostic handling of attstattarget + attribute. + - Replaced double `.unwrap()` in the dump serialization + path with proper error propagation so a `serde_json` + failure surfaces as a readable error instead of a + panic. + - `compare_types` no longer emits a stray + `-- Multirange … is created automatically` comment for + new range types. PostgreSQL auto-creates the + companion multirange row when `CREATE TYPE … AS RANGE` + runs, so the comparer now skips new-in-`to` + multiranges in the CREATE branch (symmetric to the + existing skip in the DROP branch). The skip is + deliberately scoped to the new-type branch only — an + existing multirange whose owner, comment, or ACL + differs between dumps still reaches + `get_alter_script` and emits the corresponding + `COMMENT ON TYPE` / `ALTER TYPE … OWNER TO`. This + was the leading cause of diffs looking "empty" on + schemas that added a new range type. + + Reliability: + - Added `FILL_SIBLING_BRANCH_COUNT` constant with a + compile-time arity guard against the outer `try_join!` + in `Dump::fill`, plus a runtime warning when + `--max-connections` / `MAX_CONNECTIONS` is below the + number of parallel dump branches (pool starvation + would otherwise go unnoticed). + - `Config::load` now warns when `FROM_*` and `TO_*` + resolve to the same host+port+database+schema, which + almost always means a typo in the config file and + silently produces an empty diff. + + Tooling: + - CI build job runs against both Rust 1.88.0 (floor) + and 1.95.0 (matches the Dockerfile toolchain), so the + shipped Docker image toolchain is CI-validated. + - Dockerfile `version` / `org.opencontainers.image.version` + labels are now filled from `ARG PGC_VERSION` wired + through GitHub Actions from `app/Cargo.toml` — no + more hard-coded version skew on release. + + UX / Docs: + - Fixed copy-pasted `--server` help text on `--port` + and `--scheme`; documented the `SIMILAR TO` matching + behaviour of `--scheme` (e.g. `public|app`) and + expanded `--grants-mode` to describe ignore / + addonly / full explicitly. + - Added explanatory comments at every + `recreated_tables.insert()` site documenting the + coupling with `compare_grants` default-ACL handling + for dropped-and-recreated tables. + + Tests: + - New buffer-ordering tests pin the emission order of + the comparer's post-script buffers: + sequence_post_script → type_post_script → + enum_post_script → trigger_post_script. + - Replaced `.find().unwrap()` assertions in the + clear-script tests with a helper that panics with + the full script, so ordering-test failures are + actionable. + - New multirange regression tests covering both sides + of the fix: + `compare_types_multirange_not_created_independently` + (symptom fix) and + `compare_types_multirange_comment_change_still_emits_alter` + (guards against over-broad skipping — a metadata + ALTER on an existing multirange must still emit). + 2026-04-20 v1.0.16 Performance: diff --git a/Dockerfile b/Dockerfile index d0525fa..d52998b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build stage -FROM rust:1.93-slim AS builder +FROM rust:1.95-slim AS builder # Install system dependencies needed for building RUN apt-get update && apt-get install -y \ @@ -65,11 +65,15 @@ VOLUME ["/home/pgc/data"] CMD ["pgc", "--help"] # Metadata +# PGC_VERSION is supplied at build time from app/Cargo.toml so the image +# labels stay in sync with the crate version. Override locally with +# `docker build --build-arg PGC_VERSION=$(grep '^version' app/Cargo.toml | head -1 | cut -d '"' -f2) .` +ARG PGC_VERSION=unknown LABEL maintainer="nettrash" \ description="PostgreSQL Database Comparer (PGC) - A tool for comparing PostgreSQL database schemas" \ - version="1.0.16" \ + version="${PGC_VERSION}" \ org.opencontainers.image.title="pgc" \ org.opencontainers.image.description="PostgreSQL Database Comparer" \ - org.opencontainers.image.version="1.0.16" \ + org.opencontainers.image.version="${PGC_VERSION}" \ org.opencontainers.image.source="https://github.com/nettrash/pgc" \ org.opencontainers.image.licenses="MIT" \ No newline at end of file diff --git a/app/Cargo.lock b/app/Cargo.lock index 8cec452..e999ca7 100644 --- a/app/Cargo.lock +++ b/app/Cargo.lock @@ -1146,7 +1146,7 @@ checksum = "e3148f5046208a5d56bcfc03053e3ca6334e51da8dfb19b6cdc8b306fae3283e" [[package]] name = "pgc" -version = "1.0.16" +version = "1.0.17" dependencies = [ "chrono", "clap", diff --git a/app/Cargo.toml b/app/Cargo.toml index f94001d..ad7c2af 100644 --- a/app/Cargo.toml +++ b/app/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pgc" -version = "1.0.16" +version = "1.0.17" edition = "2024" license = "MIT" diff --git a/app/src/comparer/core.rs b/app/src/comparer/core.rs index f411dcd..d227d3b 100644 --- a/app/src/comparer/core.rs +++ b/app/src/comparer/core.rs @@ -798,6 +798,16 @@ impl Comparer { } } } else { + // New-in-`to` multirange: PostgreSQL auto-creates the + // multirange row when its range type is created, so there is + // nothing to emit here (and `get_script` for 'm' would only + // return a no-op comment). Deliberately narrower than the + // drop loop's blanket 'm' skip: multiranges that exist in + // BOTH dumps but have metadata drift (owner / comment / ACL) + // still reach the ALTER branch above and emit those deltas. + if (to_type.typtype as u8 as char) == 'm' { + continue; + } self.script.push_str( format!("/* Type: {}.{} */\n", to_type.schema, to_type.typname).as_str(), ); @@ -1578,6 +1588,10 @@ impl Comparer { let to_table = &self.to.tables[tidx]; if table.partition_key != to_table.partition_key { recreated_parents.insert(Self::table_key(&table.schema, &table.name)); + // Mark for grants: recreated tables auto-inherit default + // privileges when the new row is created, so compare_grants + // must treat the from-side ACL as "default", not the old + // explicit ACL. See the `is_recreated` branch there. self.recreated_tables .insert(Self::table_key(&table.schema, &table.name)); } @@ -1682,6 +1696,10 @@ impl Comparer { ); if parent_recreated { + // Mark for grants: this partition child is being + // re-created (its parent was dropped+recreated), so it + // inherits default privileges. compare_grants keys off + // `recreated_tables` to swap in the default ACL. self.recreated_tables .insert(Self::table_key(&table.schema, &table.name)); self.script @@ -1696,6 +1714,10 @@ impl Comparer { // and the table is a partitioned parent, record it so its // children will be recreated instead of altered. if alter_script.to_lowercase().contains("drop table if exists") { + // Mark for grants: same reasoning as the branch + // above — the dropped+recreated table inherits + // default privileges, so compare_grants must use + // the default ACL as the effective `from_acl`. self.recreated_tables .insert(Self::table_key(&table.schema, &table.name)); if table.partition_key.is_some() { diff --git a/app/src/comparer/core_tests.rs b/app/src/comparer/core_tests.rs index 32518b2..df0fa99 100644 --- a/app/src/comparer/core_tests.rs +++ b/app/src/comparer/core_tests.rs @@ -6147,3 +6147,229 @@ async fn compare_types_multirange_not_dropped_independently() { "Multirange type must NOT be dropped independently, got: {script}" ); } + +/// Symmetric to the drop-side test above: multirange types are auto-CREATED +/// by PostgreSQL when the range type is created. The comparer must NOT emit +/// any per-multirange output in the main script — the `CREATE TYPE … AS +/// RANGE` for the range is enough. Previously the CREATE loop skipped only +/// enums, so a new range also produced a stray `-- Multirange …` comment +/// that made new-range diffs look noisy and was the leading explanation for +/// "the diff looks empty" reports on fresh schema_b dumps. +#[tokio::test] +async fn compare_types_multirange_not_created_independently() { + let from_dump = Dump::new(DumpConfig::default()); + let mut to_dump = Dump::new(DumpConfig::default()); + + // New range type only in TO. + let mut range_type = make_domain_type("test_schema", "int_range", 800); + range_type.typtype = 'r' as i8; + range_type.range_subtype = Some("integer".to_string()); + range_type.hash(); + + // Its auto-generated multirange, also only in TO. + let mut mr_type = make_domain_type("test_schema", "int_range_multirange", 801); + mr_type.typtype = 'm' as i8; + mr_type.hash(); + + to_dump.types.push(range_type); + to_dump.types.push(mr_type); + + let mut comparer = Comparer::new(from_dump, to_dump, true, false, true, GrantsMode::Ignore); + comparer.compare().await.unwrap(); + let script = comparer.get_script(); + + assert!( + script.contains("create type test_schema.int_range as range"), + "Range type must be created, got: {script}" + ); + let has_mr_comment = script + .contains("Multirange type test_schema.int_range_multirange is created automatically"); + assert!( + !has_mr_comment, + "Multirange must not emit a stand-alone comment block, got: {script}" + ); + let has_mr_create = script.contains("create type test_schema.int_range_multirange"); + assert!( + !has_mr_create, + "Multirange must not be CREATED independently, got: {script}" + ); +} + +/// A multirange that exists in BOTH dumps but whose owner or comment has +/// changed must still emit an ALTER (COMMENT ON TYPE / ALTER TYPE OWNER). +/// Regression guard against over-broad `'m'` skipping: the skip lives in the +/// new-in-`to` branch only, so metadata drift on existing multiranges still +/// propagates via `get_alter_script`'s comment/owner diff tail. +#[tokio::test] +async fn compare_types_multirange_comment_change_still_emits_alter() { + let mut from_dump = Dump::new(DumpConfig::default()); + let mut to_dump = Dump::new(DumpConfig::default()); + + let mut from_mr = make_domain_type("test_schema", "my_range_multirange", 900); + from_mr.typtype = 'm' as i8; + from_mr.comment = None; + from_mr.hash(); + + let mut to_mr = make_domain_type("test_schema", "my_range_multirange", 900); + to_mr.typtype = 'm' as i8; + to_mr.comment = Some("updated description".to_string()); + to_mr.hash(); + + from_dump.types.push(from_mr); + to_dump.types.push(to_mr); + + let mut comparer = Comparer::new(from_dump, to_dump, true, false, true, GrantsMode::Ignore); + comparer.compare().await.unwrap(); + let script = comparer.get_script(); + + assert!( + script + .contains("comment on type test_schema.my_range_multirange is 'updated description';"), + "Metadata ALTER on existing multirange must still emit, got: {script}" + ); +} + +// --- Post-buffer ordering (Comparer::compare concatenates several ordered +// script buffers: main → sequence_post → type_post → enum_post → +// trigger_post. These tests pin the emission order so dependency-aware +// rearrangements don't regress silently). --- + +#[tokio::test] +async fn buffer_ordering_type_drop_before_enum_drop() { + // Both buffers are populated when the FROM dump carries a domain type + // AND an enum that are both absent in the TO dump. type_post must come + // before enum_post in the final script (so enums outlive types that may + // reference them). + let mut from_dump = Dump::new(DumpConfig::default()); + let to_dump = Dump::new(DumpConfig::default()); + + from_dump + .types + .push(make_domain_type("test_schema", "dropped_domain", 701)); + from_dump.types.push(make_enum_type( + "test_schema", + "dropped_enum", + 702, + vec!["a", "b"], + )); + + let mut comparer = Comparer::new(from_dump, to_dump, true, false, true, GrantsMode::Ignore); + comparer.compare().await.unwrap(); + let script = comparer.get_script(); + + let type_drop_pos = script + .find("drop type if exists test_schema.dropped_domain cascade;") + .unwrap_or_else(|| panic!("domain drop missing in:\n{script}")); + let enum_drop_pos = script + .find("drop type if exists test_schema.dropped_enum cascade;") + .unwrap_or_else(|| panic!("enum drop missing in:\n{script}")); + assert!( + type_drop_pos < enum_drop_pos, + "type_post_script must precede enum_post_script, got:\n{script}" + ); +} + +#[tokio::test] +async fn buffer_ordering_enum_drop_before_trigger_create() { + // FROM has an enum to drop (populates enum_post_script). + // TO has a brand-new table with a trigger (populates trigger_post_script + // for the CREATE TRIGGER). enum_post must come before trigger_post so + // that triggers referencing newly-created routines/types run after all + // type-dependency cleanup. + let mut from_dump = Dump::new(DumpConfig::default()); + let mut to_dump = Dump::new(DumpConfig::default()); + + from_dump.types.push(make_enum_type( + "test_schema", + "legacy_status", + 703, + vec!["ok", "err"], + )); + + let mut new_table = Table::new( + "public".to_string(), + "events".to_string(), + "public".to_string(), + "events".to_string(), + "postgres".to_string(), + None, + vec![int_column("public", "events", "id", 1)], + vec![], + vec![], + vec![TableTrigger { + oid: Oid(9999), + name: "trg_events_audit".to_string(), + definition: + "create trigger trg_events_audit before insert on public.events for each row execute function audit()" + .to_string(), + enabled: "O".to_string(), + comment: None, + }], + None, + ); + new_table.hash(); + to_dump.tables.push(new_table); + + let mut comparer = Comparer::new(from_dump, to_dump, true, false, true, GrantsMode::Ignore); + comparer.compare().await.unwrap(); + let script = comparer.get_script(); + + let enum_drop_pos = script + .find("drop type if exists test_schema.legacy_status cascade;") + .unwrap_or_else(|| panic!("enum drop missing in:\n{script}")); + let trigger_create_pos = script + .find("create trigger trg_events_audit") + .unwrap_or_else(|| panic!("CREATE TRIGGER missing in:\n{script}")); + + assert!( + enum_drop_pos < trigger_create_pos, + "enum_post_script must precede trigger_post_script, got:\n{script}" + ); +} + +#[tokio::test] +async fn buffer_ordering_sequence_drop_before_type_drop() { + // FROM has an unowned sequence and a domain type, both absent in TO. + // sequence_post_script is emitted before type_post_script so that + // sequences with default-value dependencies on types are dropped first. + let mut from_dump = Dump::new(DumpConfig::default()); + let to_dump = Dump::new(DumpConfig::default()); + + let seq = crate::dump::sequence::Sequence::new( + "test_schema".to_string(), + "dropped_seq".to_string(), + "postgres".to_string(), + "bigint".to_string(), + Some(1), + Some(1), + Some(9223372036854775807), + Some(1), + false, + Some(1), + Some(1), + None, + None, + None, + ); + from_dump.sequences.push(seq); + from_dump + .types + .push(make_domain_type("test_schema", "dropped_domain", 704)); + + let mut comparer = Comparer::new(from_dump, to_dump, true, false, true, GrantsMode::Ignore); + comparer.compare().await.unwrap(); + let script = comparer.get_script(); + + let seq_drop_pos = script + .find("drop sequence if exists \"test_schema\".\"dropped_seq\"") + .or_else(|| script.find("drop sequence if exists test_schema.dropped_seq")) + .unwrap_or_else(|| panic!("sequence drop missing in:\n{script}")); + let type_drop_pos = script + .find("drop type if exists test_schema.dropped_domain cascade;") + .unwrap_or_else(|| panic!("type drop missing in:\n{script}")); + + assert!( + seq_drop_pos < type_drop_pos, + "sequence_post_script must precede type_post_script, got:\n{script}" + ); +} diff --git a/app/src/config/core.rs b/app/src/config/core.rs index bfe5330..e996622 100644 --- a/app/src/config/core.rs +++ b/app/src/config/core.rs @@ -161,6 +161,26 @@ impl Config { ssl: to_ssl, file: to_dump, }; + + // A FROM and TO that point at exactly the same database+schema almost + // always indicates a typo in the config file — comparing a DB to + // itself produces an empty diff, which silently looks like a + // successful run. Warn loudly so the user notices. + if !from.host.is_empty() + && !to.host.is_empty() + && from.host.eq_ignore_ascii_case(&to.host) + && from.port == to.port + && from.database.eq_ignore_ascii_case(&to.database) + && from.scheme == to.scheme + { + eprintln!( + "Warning: FROM and TO point at the same target \ + (host={}, port={}, database={}, scheme={}). The comparison \ + will produce an empty diff. Double-check the config file.", + from.host, from.port, from.database, from.scheme + ); + } + Ok(Config { from, to, diff --git a/app/src/dump/core.rs b/app/src/dump/core.rs index 9c99f40..9a0562b 100644 --- a/app/src/dump/core.rs +++ b/app/src/dump/core.rs @@ -30,6 +30,45 @@ use std::io::{Error, Read}; use zip::ZipWriter; use zip::write::SimpleFileOptions; +/// Number of top-level sibling futures passed to `tokio::try_join!` inside +/// [`Dump::fill`]. Each branch holds at least one PostgreSQL connection for +/// the duration of its query; if `max_connections` is lower than this value, +/// branches queue for connections and wall-clock time suffers — or, combined +/// with nested `try_join!`s in branches like `tables`, deadlock becomes +/// possible. +/// +/// This constant is statically asserted to equal the actual arity of the +/// [`fill_try_join!`] invocation in [`Dump::fill`]; adding or removing a +/// branch without updating this value is a compile error. +pub(crate) const FILL_SIBLING_BRANCH_COUNT: u32 = 12; + +/// Invoke `tokio::try_join!` on the given sibling futures AND statically +/// assert that the branch count matches [`FILL_SIBLING_BRANCH_COUNT`]. +/// +/// The count is derived from the macro invocation itself (one `()` emitted +/// per `$fut`), so it is always the real arity of the join. If someone +/// adds or removes a branch without touching the constant — or touches the +/// constant without matching the branch list — the `const assert!` fails +/// the build with the message below. +macro_rules! fill_try_join { + ($($fut:expr),* $(,)?) => {{ + const _FILL_ARITY: u32 = { + // One `()` is emitted per `$fut`; slice length = branch count. + // `stringify!` is const-safe and lets us reference `$fut` inside + // the repetition (otherwise the metavariable is unused). + let branches: &[()] = &[$({ let _ = stringify!($fut); }),*]; + branches.len() as u32 + }; + const _: () = assert!( + _FILL_ARITY == FILL_SIBLING_BRANCH_COUNT, + "FILL_SIBLING_BRANCH_COUNT is out of sync with the fill_try_join! arity \ + in Dump::fill. Update the constant to match the new branch count \ + (or remove the extra branch)." + ); + tokio::try_join!($($fut),*) + }}; +} + // This file defines the Dump struct and its serialization/deserialization logic. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Dump { @@ -155,6 +194,16 @@ impl Dump { // Retrieve the dump from the configuration. pub async fn process(&mut self, max_connections: u32) -> Result<(), Error> { + if max_connections < FILL_SIBLING_BRANCH_COUNT { + eprintln!( + "Warning: max_connections ({}) is below the number of parallel \ + dump branches ({}). Branches will queue for connections and \ + the dump may be significantly slower. Consider raising \ + MAX_CONNECTIONS to at least {}.", + max_connections, FILL_SIBLING_BRANCH_COUNT, FILL_SIBLING_BRANCH_COUNT + ); + } + let pool = PgPoolOptions::new() .max_connections(max_connections) .connect(self.configuration.get_connection_string().as_str()) @@ -173,14 +222,8 @@ impl Dump { pool.close().await; // Serialize the dump to a file. - let serialized = serde_json::to_string(&self); - if serialized.is_err() { - return Err(Error::other(format!( - "Failed to serialize dump: {}.", - serialized.err().unwrap() - ))); - } - let serialized_data = serialized.unwrap(); + let serialized_data = serde_json::to_string(&self) + .map_err(|e| Error::other(format!("Failed to serialize dump: {e}.")))?; let serialized_bytes = serialized_data.as_bytes(); let file = File::create(&self.configuration.file)?; @@ -270,6 +313,11 @@ impl Dump { )) }; + // Branch count is statically checked against FILL_SIBLING_BRANCH_COUNT + // by `fill_try_join!`; see the macro definition at the top of this + // file. The pool-size warning in `Dump::process` keys off that same + // constant, so the warning cannot silently drift out of sync with + // the actual parallelism here. let ( types_enums, extensions, @@ -283,7 +331,7 @@ impl Dump { event_triggers, schema_extras, global_extras, - ) = tokio::try_join!( + ) = fill_try_join!( types_enums_fut, extensions_fut, sequences_fut, @@ -3480,15 +3528,18 @@ mod tests { let script = dump.generate_clear_script(false, false, false); - let view_pos = script.find("drop view if exists public.v1;").unwrap(); - let table_pos = script.find("drop table if exists public.tbl1;").unwrap(); - let routine_pos = script - .find("drop function if exists public.fn1 ();") - .unwrap(); - let seq_pos = script.find("drop sequence if exists public.seq1;").unwrap(); - let type_pos = script.find("drop type if exists public.my_type;").unwrap(); - let ext_pos = script.find("drop extension if exists pg_trgm;").unwrap(); - let schema_pos = script.find("drop schema if exists public;").unwrap(); + let find = |needle: &str| { + script + .find(needle) + .unwrap_or_else(|| panic!("missing `{needle}` in clear script:\n{script}")) + }; + let view_pos = find("drop view if exists public.v1;"); + let table_pos = find("drop table if exists public.tbl1;"); + let routine_pos = find("drop function if exists public.fn1 ();"); + let seq_pos = find("drop sequence if exists public.seq1;"); + let type_pos = find("drop type if exists public.my_type;"); + let ext_pos = find("drop extension if exists pg_trgm;"); + let schema_pos = find("drop schema if exists public;"); assert!(view_pos < table_pos, "views before tables"); assert!(table_pos < routine_pos, "tables before routines"); @@ -3662,9 +3713,14 @@ mod tests { dump.views.push(make_view_with_deps("s", "c", vec!["s.b"])); let script = dump.generate_clear_script(false, false, false); - let pos_c = script.find("drop view if exists s.c;").unwrap(); - let pos_b = script.find("drop view if exists s.b;").unwrap(); - let pos_a = script.find("drop view if exists s.a;").unwrap(); + let find = |needle: &str| { + script + .find(needle) + .unwrap_or_else(|| panic!("missing `{needle}` in clear script:\n{script}")) + }; + let pos_c = find("drop view if exists s.c;"); + let pos_b = find("drop view if exists s.b;"); + let pos_a = find("drop view if exists s.a;"); assert!(pos_c < pos_b, "c before b"); assert!(pos_b < pos_a, "b before a"); } @@ -3678,9 +3734,14 @@ mod tests { dump.views.push(make_view("s", "mu")); let script = dump.generate_clear_script(false, false, false); - let pos_a = script.find("drop view if exists s.alpha;").unwrap(); - let pos_m = script.find("drop view if exists s.mu;").unwrap(); - let pos_z = script.find("drop view if exists s.zeta;").unwrap(); + let find = |needle: &str| { + script + .find(needle) + .unwrap_or_else(|| panic!("missing `{needle}` in clear script:\n{script}")) + }; + let pos_a = find("drop view if exists s.alpha;"); + let pos_m = find("drop view if exists s.mu;"); + let pos_z = find("drop view if exists s.zeta;"); assert!(pos_a < pos_m, "alpha before mu"); assert!(pos_m < pos_z, "mu before zeta"); } @@ -3694,11 +3755,14 @@ mod tests { dump.views.push(make_view("s", "regular_a")); let script = dump.generate_clear_script(false, false, false); - let mat_pos = script - .find("drop materialized view if exists s.mat_a;") - .unwrap(); - let reg_a_pos = script.find("drop view if exists s.regular_a;").unwrap(); - let reg_b_pos = script.find("drop view if exists s.regular_b;").unwrap(); + let find = |needle: &str| { + script + .find(needle) + .unwrap_or_else(|| panic!("missing `{needle}` in clear script:\n{script}")) + }; + let mat_pos = find("drop materialized view if exists s.mat_a;"); + let reg_a_pos = find("drop view if exists s.regular_a;"); + let reg_b_pos = find("drop view if exists s.regular_b;"); assert!(mat_pos < reg_a_pos, "materialized before regular_a"); assert!(mat_pos < reg_b_pos, "materialized before regular_b"); assert!( diff --git a/app/src/dump/table.rs b/app/src/dump/table.rs index 3adee5a..71fd5e8 100644 --- a/app/src/dump/table.rs +++ b/app/src/dump/table.rs @@ -322,18 +322,8 @@ impl Table { Ok(()) } - /// Fetch columns for every table in the accessible schemas in one query. - async fn fetch_columns_bulk( - pool: &PgPool, - schema_filter: &str, - caps: PgCatalogCaps, - ) -> Result>, Error> { - let compression_col = if caps.has_attcompression { - ",\n a.attcompression::text as col_compression" - } else { - "" - }; - let query = format!( + fn build_columns_query(compression_col: &str, schema_filter: &str) -> String { + format!( "SELECT c.table_catalog, quote_ident(c.table_schema) as table_schema, @@ -386,7 +376,7 @@ impl Table { c.generation_expression, a.attgenerated::text as attgenerated, a.attstorage::text as col_storage, - a.attstattarget as col_stattarget, + a.attstattarget::int4 as col_stattarget, c.is_updatable, pd.description as column_comment{compression_col}, coalesce( @@ -434,7 +424,21 @@ impl Table { AND pd.objsubid = a.attnum WHERE c.table_schema IN {schema_filter} ORDER BY c.table_schema, c.table_name, c.ordinal_position" - ); + ) + } + + /// Fetch columns for every table in the accessible schemas in one query. + async fn fetch_columns_bulk( + pool: &PgPool, + schema_filter: &str, + caps: PgCatalogCaps, + ) -> Result>, Error> { + let compression_col = if caps.has_attcompression { + ",\n a.attcompression::text as col_compression" + } else { + "" + }; + let query = Self::build_columns_query(compression_col, schema_filter); let rows = sqlx::query(&query).fetch_all(pool).await?; let mut columns_by_key: HashMap<(String, String), Vec> = HashMap::new(); @@ -519,8 +523,8 @@ impl Table { None }, statistics_target: { - let st: Option = row.get("col_stattarget"); - st.filter(|&v| v >= 0).map(|v| v as i32) + let st: Option = row.get("col_stattarget"); + st.filter(|&v| v >= 0) }, acl: row.get::, _>("col_acl"), serial_type: None, @@ -4498,4 +4502,14 @@ mod tests { "expected NO INHERIT on named NOT NULL constraint: {script}" ); } + + #[test] + fn fetch_columns_query_casts_attstattarget_to_int4() { + let query = Table::build_columns_query("", "('public')"); + + assert!( + query.contains("a.attstattarget::int4"), + "expected ::int4 cast for attstattarget" + ); + } } diff --git a/app/src/main.rs b/app/src/main.rs index ed9b031..af27551 100644 --- a/app/src/main.rs +++ b/app/src/main.rs @@ -26,27 +26,29 @@ struct Args { #[arg(long)] command: Option, - /// Hostname for the command + /// PostgreSQL server hostname #[arg(long, default_value = "localhost")] server: Option, - /// Hostname for the command + /// PostgreSQL server port #[arg(long, default_value = "5432")] port: Option, - /// User name for the command + /// PostgreSQL user name #[arg(long, default_value = "")] user: Option, - /// Password of user for the command + /// PostgreSQL user password #[arg(long, default_value = "")] password: Option, - /// Database name for the command + /// PostgreSQL database name #[arg(long, default_value = "postgres")] database: Option, - /// Schema name for the command + /// PostgreSQL schema name — matched against pg_namespace.nspname using + /// SQL SIMILAR TO, so patterns like `public|app` select multiple schemas + /// and `app_.*` matches every schema with an `app_` prefix. #[arg(long, default_value = "public")] scheme: Option, @@ -82,7 +84,11 @@ struct Args { #[arg(long, default_value_t = true, num_args = 0..=1, default_missing_value = "true", value_parser = clap::builder::BoolishValueParser::new(), action = clap::ArgAction::Set)] use_comments: bool, - /// Grants handling mode: ignore, addonly, full + /// Grants handling mode: + /// ignore - do not emit any GRANT/REVOKE statements (default); + /// addonly - emit only GRANTs that are present in `to` but missing in `from`; + /// full - emit GRANTs and also REVOKEs for privileges present in `from` + /// but absent in `to` (true synchronisation). #[arg(long, value_parser = parse_grants_mode, default_value = "ignore")] grants_mode: GrantsMode,