diff --git a/CHANGELOG b/CHANGELOG index 2e670bc..163fa8b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,23 @@ +v1.0.19 + + Bug fixes: + - Refactored comment-related query construction into dedicated + helper methods: + `Dump::build_sequences_standalone_query` and + `Table::build_indexes_bulk_query`. + - Kept `pg_description` joins scoped to relation comments by + filtering on `pg_description.classoid = 'pg_class'::regclass` + and `pg_description.objsubid = 0` to avoid catalog + collisions. + - Normalized SQL formatting in the new helper query strings. + + Tests: + - Added regression tests proving both query builders + include the `pg_class` classoid filter: + `build_sequences_standalone_query_filters_by_pg_class` + and + `build_indexes_bulk_query_filters_by_pg_class`. + 2026-04-25 v1.0.18 Bug fixes: diff --git a/app/src/dump/core.rs b/app/src/dump/core.rs index 06abb90..3375f5f 100644 --- a/app/src/dump/core.rs +++ b/app/src/dump/core.rs @@ -889,48 +889,7 @@ impl Dump { pool: &PgPool, schema_filter: &str, ) -> Result, Error> { - let query = format!( - " - select - quote_ident(seq.schemaname) as schemaname, - quote_ident(seq.sequencename) as sequencename, - quote_ident(seq.sequenceowner) as sequenceowner, - seq.data_type::varchar as sequencedatatype, - seq.start_value, - seq.min_value, - seq.max_value, - seq.increment_by, - seq.cycle, - seq.cache_size, - seq.last_value, - quote_ident(owner_ns.nspname) as owned_by_schema, - quote_ident(owner_table.relname) as owned_by_table, - quote_ident(owner_attr.attname) as owned_by_column, - dep.deptype::text as dependency_type, - seq_desc.description as seq_comment, - seq_class.relacl::text[] as seq_acl, - seq_class.relpersistence::text as seq_persistence - from - pg_sequences seq - left join pg_namespace seq_ns on seq_ns.nspname = seq.schemaname - left join pg_class seq_class on seq_class.relname = seq.sequencename - and seq_class.relnamespace = seq_ns.oid - left join pg_description seq_desc on seq_desc.objoid = seq_class.oid and seq_desc.objsubid = 0 - left join pg_depend dep on dep.objid = seq_class.oid - and dep.deptype in ('a', 'i') - left join pg_class owner_table on owner_table.oid = dep.refobjid - left join pg_namespace owner_ns on owner_ns.oid = owner_table.relnamespace - left join pg_attribute owner_attr on owner_attr.attrelid = dep.refobjid - and owner_attr.attnum = dep.refobjsubid - where - seq.schemaname in {} - and not exists ( - select 1 from pg_depend ext_dep - where ext_dep.objid = seq_class.oid - and ext_dep.deptype = 'e' - )", - schema_filter - ); + let query = Self::build_sequences_standalone_query(schema_filter); let rows = sqlx::query(query.as_str()) .fetch_all(pool) @@ -985,6 +944,52 @@ impl Dump { Ok(sequences) } + fn build_sequences_standalone_query(schema_filter: &str) -> String { + format!( + "select + quote_ident(seq.schemaname) as schemaname, + quote_ident(seq.sequencename) as sequencename, + quote_ident(seq.sequenceowner) as sequenceowner, + seq.data_type::varchar as sequencedatatype, + seq.start_value, + seq.min_value, + seq.max_value, + seq.increment_by, + seq.cycle, + seq.cache_size, + seq.last_value, + quote_ident(owner_ns.nspname) as owned_by_schema, + quote_ident(owner_table.relname) as owned_by_table, + quote_ident(owner_attr.attname) as owned_by_column, + dep.deptype::text as dependency_type, + seq_desc.description as seq_comment, + seq_class.relacl::text[] as seq_acl, + seq_class.relpersistence::text as seq_persistence + from + pg_sequences seq + left join pg_namespace seq_ns on seq_ns.nspname = seq.schemaname + left join pg_class seq_class on seq_class.relname = seq.sequencename + and seq_class.relnamespace = seq_ns.oid + left join pg_description seq_desc on seq_desc.objoid = seq_class.oid + and seq_desc.classoid = 'pg_class'::regclass + and seq_desc.objsubid = 0 + left join pg_depend dep on dep.objid = seq_class.oid + and dep.deptype in ('a', 'i') + left join pg_class owner_table on owner_table.oid = dep.refobjid + left join pg_namespace owner_ns on owner_ns.oid = owner_table.relnamespace + left join pg_attribute owner_attr on owner_attr.attrelid = dep.refobjid + and owner_attr.attnum = dep.refobjsubid + where + seq.schemaname in {} + and not exists ( + select 1 from pg_depend ext_dep + where ext_dep.objid = seq_class.oid + and ext_dep.deptype = 'e' + )", + schema_filter + ) + } + async fn fetch_routines_standalone( pool: &PgPool, schema_filter: &str, @@ -1225,55 +1230,7 @@ impl Dump { // Probe catalog capabilities once for the entire dump run. let caps = PgCatalogCaps::detect(pool, pg_version).await; - let query = format!( - " - select - quote_ident(t.schemaname) as schemaname, - quote_ident(t.tablename) as tablename, - quote_ident(t.tableowner) as tableowner, - t.schemaname as raw_schema_name, - t.tablename as raw_table_name, - t.tablespace, - t.hasindexes, - t.hastriggers, - t.hasrules, - t.rowsecurity, - d.description as table_comment, - c.relacl::text[] as table_acl, - am.amname as access_method, - c.relpersistence as relpersistence, - c.reloptions as reloptions, - c.relreplident as relreplident, - c.relforcerowsecurity as relforcerowsecurity, - case when c.reloftype <> 0 then c.reloftype::regtype::text else null end as typed_table_type, - array( - select quote_ident(pn.nspname) || '.' || quote_ident(pc.relname) - from pg_inherits pi2 - join pg_class pc on pc.oid = pi2.inhparent - join pg_namespace pn on pn.oid = pc.relnamespace - where pi2.inhrelid = c.oid - and not exists ( - select 1 from pg_partitioned_table pt where pt.partrelid = pi2.inhparent - ) - order by pi2.inhseqno - ) as inherits_from - from pg_tables t - left join pg_class c on c.relname = t.tablename - and c.relkind in ('r','p') - and c.relnamespace = (select oid from pg_namespace where nspname = t.schemaname) - left join pg_am am on am.oid = c.relam - left join pg_description d on d.objoid = c.oid and d.objsubid = 0 - where - t.schemaname not in ('pg_catalog', 'information_schema') - and t.schemaname in {} - and t.tablename not like 'pg_%' - and not exists ( - select 1 from pg_depend ext_dep - where ext_dep.objid = c.oid - and ext_dep.deptype = 'e' - );", - schema_filter - ); + let query = Self::build_tables_standalone_query(schema_filter); let rows = sqlx::query(query.as_str()) .fetch_all(pool) @@ -1370,89 +1327,68 @@ impl Dump { Ok(shell_tables) } + fn build_tables_standalone_query(schema_filter: &str) -> String { + format!( + " + select + quote_ident(t.schemaname) as schemaname, + quote_ident(t.tablename) as tablename, + quote_ident(t.tableowner) as tableowner, + t.schemaname as raw_schema_name, + t.tablename as raw_table_name, + t.tablespace, + t.hasindexes, + t.hastriggers, + t.hasrules, + t.rowsecurity, + d.description as table_comment, + c.relacl::text[] as table_acl, + am.amname as access_method, + c.relpersistence as relpersistence, + c.reloptions as reloptions, + c.relreplident as relreplident, + c.relforcerowsecurity as relforcerowsecurity, + case when c.reloftype <> 0 then c.reloftype::regtype::text else null end as typed_table_type, + array( + select quote_ident(pn.nspname) || '.' || quote_ident(pc.relname) + from pg_inherits pi2 + join pg_class pc on pc.oid = pi2.inhparent + join pg_namespace pn on pn.oid = pc.relnamespace + where pi2.inhrelid = c.oid + and not exists ( + select 1 from pg_partitioned_table pt where pt.partrelid = pi2.inhparent + ) + order by pi2.inhseqno + ) as inherits_from + from pg_tables t + left join pg_class c on c.relname = t.tablename + and c.relkind in ('r','p') + and c.relnamespace = (select oid from pg_namespace where nspname = t.schemaname) + left join pg_am am on am.oid = c.relam + left join pg_description d on d.objoid = c.oid + and d.classoid = 'pg_class'::regclass + and d.objsubid = 0 + where + t.schemaname not in ('pg_catalog', 'information_schema') + and t.schemaname in {} + and t.tablename not like 'pg_%' + and not exists ( + select 1 from pg_depend ext_dep + where ext_dep.objid = c.oid + and ext_dep.deptype = 'e' + );", + schema_filter + ) + } + async fn fetch_views_standalone( pool: &PgPool, schema_filter: &str, ) -> Result, Error> { // Fetch regular and materialized views concurrently. - let regular_query = format!( - "select - quote_ident(v.table_schema) as table_schema, - quote_ident(v.table_name) as table_name, - v.view_definition, - quote_ident(pv.viewowner) as view_owner, - array_agg(distinct vtu.table_schema || '.' || vtu.table_name) as table_relation, - d.description as view_comment, - (select cc.relacl::text[] from pg_class cc where cc.oid = c.oid) as view_acl, - coalesce(c.reloptions::text[] @> array['security_invoker=true']::text[], false) as security_invoker, - v.check_option - from information_schema.views v - join information_schema.view_table_usage vtu on v.table_name = vtu.view_name and v.table_schema = vtu.view_schema - left join pg_views pv on pv.schemaname = v.table_schema and pv.viewname = v.table_name - left join pg_class c on c.relname = v.table_name and c.relnamespace = (select oid from pg_namespace where nspname = v.table_schema) - left join pg_description d on d.objoid = c.oid and d.objsubid = 0 - where - v.table_schema not in ('pg_catalog', 'information_schema') - and v.table_schema in {} - and not exists ( - select 1 from pg_depend ext_dep - where ext_dep.objid = c.oid - and ext_dep.deptype = 'e' - ) - group by v.table_schema, v.table_name, v.view_definition, pv.viewowner, d.description, c.oid, c.reloptions, v.check_option;", - schema_filter - ); - - let mat_query = format!( - "select - mv.schemaname as table_schema, - mv.matviewname as table_name, - mv.definition as view_definition, - mv.matviewowner as view_owner, - array( - select distinct n.nspname || '.' || dc.relname - from pg_depend dep - join pg_class dc on dc.oid = dep.refobjid - join pg_namespace n on n.oid = dc.relnamespace - where dep.objid = c.oid - and dep.deptype = 'n' - and dc.relkind in ('r', 'v', 'm') - ) as table_relation, - d.description as view_comment, - c.relacl::text[] as view_acl, - c.reloptions as storage_options, - (select spcname from pg_tablespace where oid = c.reltablespace) as tablespace_name - from pg_matviews mv - join pg_class c on c.relname = mv.matviewname - and c.relnamespace = (select oid from pg_namespace where nspname = mv.schemaname) - left join pg_description d on d.objoid = c.oid and d.objsubid = 0 - where mv.schemaname not in ('pg_catalog', 'information_schema') - and mv.schemaname in {} - and not exists ( - select 1 from pg_depend ext_dep - where ext_dep.objid = c.oid - and ext_dep.deptype = 'e' - );", - schema_filter - ); - - // Column comments query (works for both regular and materialized views) - let col_comments_query = format!( - "select - quote_ident(n.nspname) as schema_name, - quote_ident(c.relname) as view_name, - quote_ident(a.attname) as column_name, - d.description as col_comment - from pg_class c - join pg_namespace n on n.oid = c.relnamespace - join pg_attribute a on a.attrelid = c.oid and a.attnum > 0 and not a.attisdropped - join pg_description d on d.objoid = c.oid and d.objsubid = a.attnum - where c.relkind in ('v', 'm') - and n.nspname not in ('pg_catalog', 'information_schema') - and n.nspname in {} - order by n.nspname, c.relname, a.attnum;", - schema_filter - ); + let regular_query = Self::build_regular_views_query(schema_filter); + let mat_query = Self::build_materialized_views_query(schema_filter); + let col_comments_query = Self::build_view_column_comments_query(schema_filter); let (regular_rows, mat_rows, col_comment_rows) = tokio::try_join!( async { @@ -1599,6 +1535,96 @@ impl Dump { Ok(views) } + fn build_regular_views_query(schema_filter: &str) -> String { + format!( + "select + quote_ident(v.table_schema) as table_schema, + quote_ident(v.table_name) as table_name, + v.view_definition, + quote_ident(pv.viewowner) as view_owner, + array_agg(distinct vtu.table_schema || '.' || vtu.table_name) as table_relation, + d.description as view_comment, + (select cc.relacl::text[] from pg_class cc where cc.oid = c.oid) as view_acl, + coalesce(c.reloptions::text[] @> array['security_invoker=true']::text[], false) as security_invoker, + v.check_option + from information_schema.views v + join information_schema.view_table_usage vtu on v.table_name = vtu.view_name and v.table_schema = vtu.view_schema + left join pg_views pv on pv.schemaname = v.table_schema and pv.viewname = v.table_name + left join pg_class c on c.relname = v.table_name and c.relnamespace = (select oid from pg_namespace where nspname = v.table_schema) + left join pg_description d on d.objoid = c.oid + and d.classoid = 'pg_class'::regclass + and d.objsubid = 0 + where + v.table_schema not in ('pg_catalog', 'information_schema') + and v.table_schema in {} + and not exists ( + select 1 from pg_depend ext_dep + where ext_dep.objid = c.oid + and ext_dep.deptype = 'e' + ) + group by v.table_schema, v.table_name, v.view_definition, pv.viewowner, d.description, c.oid, c.reloptions, v.check_option;", + schema_filter + ) + } + + fn build_materialized_views_query(schema_filter: &str) -> String { + format!( + "select + mv.schemaname as table_schema, + mv.matviewname as table_name, + mv.definition as view_definition, + mv.matviewowner as view_owner, + array( + select distinct n.nspname || '.' || dc.relname + from pg_depend dep + join pg_class dc on dc.oid = dep.refobjid + join pg_namespace n on n.oid = dc.relnamespace + where dep.objid = c.oid + and dep.deptype = 'n' + and dc.relkind in ('r', 'v', 'm') + ) as table_relation, + d.description as view_comment, + c.relacl::text[] as view_acl, + c.reloptions as storage_options, + (select spcname from pg_tablespace where oid = c.reltablespace) as tablespace_name + from pg_matviews mv + join pg_class c on c.relname = mv.matviewname + and c.relnamespace = (select oid from pg_namespace where nspname = mv.schemaname) + left join pg_description d on d.objoid = c.oid + and d.classoid = 'pg_class'::regclass + and d.objsubid = 0 + where mv.schemaname not in ('pg_catalog', 'information_schema') + and mv.schemaname in {} + and not exists ( + select 1 from pg_depend ext_dep + where ext_dep.objid = c.oid + and ext_dep.deptype = 'e' + );", + schema_filter + ) + } + + fn build_view_column_comments_query(schema_filter: &str) -> String { + format!( + "select + quote_ident(n.nspname) as schema_name, + quote_ident(c.relname) as view_name, + quote_ident(a.attname) as column_name, + d.description as col_comment + from pg_class c + join pg_namespace n on n.oid = c.relnamespace + join pg_attribute a on a.attrelid = c.oid and a.attnum > 0 and not a.attisdropped + join pg_description d on d.objoid = c.oid + and d.classoid = 'pg_class'::regclass + and d.objsubid = a.attnum + where c.relkind in ('v', 'm') + and n.nspname not in ('pg_catalog', 'information_schema') + and n.nspname in {} + order by n.nspname, c.relname, a.attnum;", + schema_filter + ) + } + async fn fetch_foreign_tables_standalone( pool: &PgPool, schema_filter: &str, @@ -3764,4 +3790,49 @@ mod tests { "regular_a before regular_b alphabetically" ); } + + #[test] + fn build_tables_standalone_query_filters_by_pg_class() { + let query = Dump::build_tables_standalone_query("('public')"); + assert!( + query.contains("d.classoid = 'pg_class'::regclass"), + "expected pg_class classoid filter for table comments" + ); + } + + #[test] + fn build_regular_views_query_filters_by_pg_class() { + let query = Dump::build_regular_views_query("('public')"); + assert!( + query.contains("d.classoid = 'pg_class'::regclass"), + "expected pg_class classoid filter for regular view comments" + ); + } + + #[test] + fn build_materialized_views_query_filters_by_pg_class() { + let query = Dump::build_materialized_views_query("('public')"); + assert!( + query.contains("d.classoid = 'pg_class'::regclass"), + "expected pg_class classoid filter for materialized view comments" + ); + } + + #[test] + fn build_view_column_comments_query_filters_by_pg_class() { + let query = Dump::build_view_column_comments_query("('public')"); + assert!( + query.contains("d.classoid = 'pg_class'::regclass"), + "expected pg_class classoid filter for view column comments" + ); + } + + #[test] + fn build_sequences_standalone_query_filters_by_pg_class() { + let query = Dump::build_sequences_standalone_query("('public')"); + assert!( + query.contains("seq_desc.classoid = 'pg_class'::regclass"), + "expected pg_class classoid filter for sequence comments" + ); + } } diff --git a/app/src/dump/table.rs b/app/src/dump/table.rs index 71fd5e8..a4ce45a 100644 --- a/app/src/dump/table.rs +++ b/app/src/dump/table.rs @@ -421,6 +421,7 @@ impl Table { AND a.attisdropped = false LEFT JOIN pg_description pd ON pd.objoid = cls.oid + AND pd.classoid = 'pg_class'::regclass AND pd.objsubid = a.attnum WHERE c.table_schema IN {schema_filter} ORDER BY c.table_schema, c.table_name, c.ordinal_position" @@ -545,29 +546,7 @@ impl Table { pool: &PgPool, schema_filter: &str, ) -> Result>, Error> { - let query = format!( - "SELECT - quote_ident(i.schemaname) as schemaname, - quote_ident(i.tablename) as tablename, - i.schemaname as raw_schemaname, - i.tablename as raw_tablename, - quote_ident(i.indexname) as indexname, - i.tablespace, - i.indexdef, - EXISTS (SELECT 1 FROM pg_inherits inh WHERE inh.inhrelid = ic.oid) AS is_partition_index, - d.description as index_comment - FROM pg_indexes i - JOIN pg_class ic ON ic.relname = i.indexname - JOIN pg_namespace n ON n.oid = ic.relnamespace AND n.nspname = i.schemaname - JOIN pg_index idx ON idx.indexrelid = ic.oid - LEFT JOIN pg_constraint puc ON puc.conindid = ic.oid AND puc.contype IN ('p', 'u') - LEFT JOIN pg_description d ON d.objoid = ic.oid AND d.objsubid = 0 - WHERE idx.indisprimary = false - AND (idx.indisunique = false OR puc.oid IS NULL) - AND NOT EXISTS (SELECT 1 FROM pg_constraint xc WHERE xc.conindid = ic.oid AND xc.contype = 'x') - AND i.schemaname IN {schema_filter} - ORDER BY i.schemaname, i.tablename, i.indexname" - ); + let query = Self::build_indexes_bulk_query(schema_filter); let rows = sqlx::query(&query).fetch_all(pool).await?; let mut indexes_by_key: HashMap<(String, String), Vec> = HashMap::new(); @@ -592,6 +571,44 @@ impl Table { Ok(indexes_by_key) } + fn build_indexes_bulk_query(schema_filter: &str) -> String { + format!( + "select + quote_ident(i.schemaname) as schemaname, + quote_ident(i.tablename) as tablename, + i.schemaname as raw_schemaname, + i.tablename as raw_tablename, + quote_ident(i.indexname) as indexname, + i.tablespace, + i.indexdef, + EXISTS (SELECT 1 FROM pg_inherits inh WHERE inh.inhrelid = ic.oid) AS is_partition_index, + d.description as index_comment + from + pg_indexes i + join pg_class ic on ic.relname = i.indexname + join pg_namespace n on n.oid = ic.relnamespace and n.nspname = i.schemaname + join pg_index idx on idx.indexrelid = ic.oid + left join pg_constraint puc on puc.conindid = ic.oid and puc.contype in ('p', 'u') + left join pg_description d + on d.objoid = ic.oid + and d.classoid = 'pg_class'::regclass + and d.objsubid = 0 + where + idx.indisprimary = false + and (idx.indisunique = false or puc.oid is null) + and not exists ( + select 1 from pg_constraint xc + where xc.conindid = ic.oid + and xc.contype = 'x' + ) + and i.schemaname in {schema_filter} + order by + i.schemaname, + i.tablename, + i.indexname" + ) + } + /// Fetch constraints for every table in the accessible schemas in one query. async fn fetch_constraints_bulk( pool: &PgPool, @@ -4511,5 +4528,19 @@ mod tests { query.contains("a.attstattarget::int4"), "expected ::int4 cast for attstattarget" ); + assert!( + query.contains("pd.classoid = 'pg_class'::regclass"), + "expected pg_class classoid filter for table column comments" + ); + } + + #[test] + fn build_indexes_bulk_query_filters_by_pg_class() { + let query = Table::build_indexes_bulk_query("('public')"); + + assert!( + query.contains("d.classoid = 'pg_class'::regclass"), + "expected pg_class classoid filter for table index comments" + ); } }