From 9550ebd118bf961bcd504cc8ff40a820d280f11f Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Wed, 18 Jan 2023 15:32:15 +0300 Subject: [PATCH] Remove pg_depend entries from columnar metadata indexes to columnar-am In the past, having columnar tables in the cluster was causing pg upgrades to fail when attempting to access columnar metadata. This is because, pg_dump doesn't see objects that we use for columnar-am related booking as the dependencies of the tables using columnar-am. To fix that; in #5456, we inserted some "normal dependency" edges (from those objects to columnar-am) into pg_depend. This helped us ensuring the existency of a class of metadata objects --such as columnar.storageid_seq-- and helped fixing #5437. However, the normal-dependency edges that we added for indexes on columnar metadata tables --such columnar.stripe_pkey-- didn't help at all because they were indeed causing dependency loops (#5510) and pg_dump was not able to take those dependency edges into the account. For this reason, this commit deletes those dependency edges so that pg_dump stops complaining about them. Note that it's not critical to delete those edges from pg_depend since they're not breaking pg upgrades but were triggering some warning messages. And given that backporting a sql change into older versions is hard a lot, we skip backporting this. --- .../sql/citus_columnar--11.1-1--11.2-1.sql | 18 ++++++++ .../citus_columnar--11.2-1--11.1-1.sql | 3 ++ .../11.2-1.sql | 43 +++++++++++++++++++ .../latest.sql | 17 +++----- src/test/regress/expected/multi_extension.out | 37 ++++++++++++++++ .../expected/upgrade_columnar_after.out | 24 ++++++++--- src/test/regress/sql/multi_extension.sql | 33 ++++++++++++++ .../regress/sql/upgrade_columnar_after.sql | 20 ++++++--- 8 files changed, 172 insertions(+), 23 deletions(-) create mode 100644 src/backend/columnar/sql/udfs/columnar_ensure_am_depends_catalog/11.2-1.sql diff --git a/src/backend/columnar/sql/citus_columnar--11.1-1--11.2-1.sql b/src/backend/columnar/sql/citus_columnar--11.1-1--11.2-1.sql index 60a0401d5..89ccd9e74 100644 --- a/src/backend/columnar/sql/citus_columnar--11.1-1--11.2-1.sql +++ b/src/backend/columnar/sql/citus_columnar--11.1-1--11.2-1.sql @@ -1 +1,19 @@ -- citus_columnar--11.1-1--11.2-1 + +#include "udfs/columnar_ensure_am_depends_catalog/11.2-1.sql" + +DELETE FROM pg_depend +WHERE classid = 'pg_am'::regclass::oid + AND objid IN (select oid from pg_am where amname = 'columnar') + AND objsubid = 0 + AND refclassid = 'pg_class'::regclass::oid + AND refobjid IN ( + 'columnar_internal.stripe_first_row_number_idx'::regclass::oid, + 'columnar_internal.chunk_group_pkey'::regclass::oid, + 'columnar_internal.chunk_pkey'::regclass::oid, + 'columnar_internal.options_pkey'::regclass::oid, + 'columnar_internal.stripe_first_row_number_idx'::regclass::oid, + 'columnar_internal.stripe_pkey'::regclass::oid + ) + AND refobjsubid = 0 + AND deptype = 'n'; diff --git a/src/backend/columnar/sql/downgrades/citus_columnar--11.2-1--11.1-1.sql b/src/backend/columnar/sql/downgrades/citus_columnar--11.2-1--11.1-1.sql index 9acf68da3..c987bfa67 100644 --- a/src/backend/columnar/sql/downgrades/citus_columnar--11.2-1--11.1-1.sql +++ b/src/backend/columnar/sql/downgrades/citus_columnar--11.2-1--11.1-1.sql @@ -1 +1,4 @@ -- citus_columnar--11.2-1--11.1-1 + +-- Note that we intentionally do not re-insert the pg_depend records that we +-- deleted via citus_columnar--11.1-1--11.2-1.sql. diff --git a/src/backend/columnar/sql/udfs/columnar_ensure_am_depends_catalog/11.2-1.sql b/src/backend/columnar/sql/udfs/columnar_ensure_am_depends_catalog/11.2-1.sql new file mode 100644 index 000000000..101db17fb --- /dev/null +++ b/src/backend/columnar/sql/udfs/columnar_ensure_am_depends_catalog/11.2-1.sql @@ -0,0 +1,43 @@ +CREATE OR REPLACE FUNCTION columnar_internal.columnar_ensure_am_depends_catalog() + RETURNS void + LANGUAGE plpgsql + SET search_path = pg_catalog +AS $func$ +BEGIN + INSERT INTO pg_depend + WITH columnar_schema_members(relid) AS ( + SELECT pg_class.oid AS relid FROM pg_class + WHERE relnamespace = + COALESCE( + (SELECT pg_namespace.oid FROM pg_namespace WHERE nspname = 'columnar_internal'), + (SELECT pg_namespace.oid FROM pg_namespace WHERE nspname = 'columnar') + ) + AND relname IN ('chunk', + 'chunk_group', + 'options', + 'storageid_seq', + 'stripe') + ) + SELECT -- Define a dependency edge from "columnar table access method" .. + 'pg_am'::regclass::oid as classid, + (select oid from pg_am where amname = 'columnar') as objid, + 0 as objsubid, + -- ... to some objects registered as regclass and that lives in + -- "columnar" schema. That contains catalog tables and the sequences + -- created in "columnar" schema. + -- + -- Given the possibility of user might have created their own objects + -- in columnar schema, we explicitly specify list of objects that we + -- are interested in. + 'pg_class'::regclass::oid as refclassid, + columnar_schema_members.relid as refobjid, + 0 as refobjsubid, + 'n' as deptype + FROM columnar_schema_members + -- Avoid inserting duplicate entries into pg_depend. + EXCEPT TABLE pg_depend; +END; +$func$; +COMMENT ON FUNCTION columnar_internal.columnar_ensure_am_depends_catalog() + IS 'internal function responsible for creating dependencies from columnar ' + 'table access method to the rel objects in columnar schema'; diff --git a/src/backend/columnar/sql/udfs/columnar_ensure_am_depends_catalog/latest.sql b/src/backend/columnar/sql/udfs/columnar_ensure_am_depends_catalog/latest.sql index ade15390a..101db17fb 100644 --- a/src/backend/columnar/sql/udfs/columnar_ensure_am_depends_catalog/latest.sql +++ b/src/backend/columnar/sql/udfs/columnar_ensure_am_depends_catalog/latest.sql @@ -1,4 +1,4 @@ -CREATE OR REPLACE FUNCTION citus_internal.columnar_ensure_am_depends_catalog() +CREATE OR REPLACE FUNCTION columnar_internal.columnar_ensure_am_depends_catalog() RETURNS void LANGUAGE plpgsql SET search_path = pg_catalog @@ -14,22 +14,17 @@ BEGIN ) AND relname IN ('chunk', 'chunk_group', - 'chunk_group_pkey', - 'chunk_pkey', 'options', - 'options_pkey', 'storageid_seq', - 'stripe', - 'stripe_first_row_number_idx', - 'stripe_pkey') + 'stripe') ) SELECT -- Define a dependency edge from "columnar table access method" .. 'pg_am'::regclass::oid as classid, (select oid from pg_am where amname = 'columnar') as objid, 0 as objsubid, - -- ... to each object that is registered to pg_class and that lives - -- in "columnar" schema. That contains catalog tables, indexes - -- created on them and the sequences created in "columnar" schema. + -- ... to some objects registered as regclass and that lives in + -- "columnar" schema. That contains catalog tables and the sequences + -- created in "columnar" schema. -- -- Given the possibility of user might have created their own objects -- in columnar schema, we explicitly specify list of objects that we @@ -43,6 +38,6 @@ BEGIN EXCEPT TABLE pg_depend; END; $func$; -COMMENT ON FUNCTION citus_internal.columnar_ensure_am_depends_catalog() +COMMENT ON FUNCTION columnar_internal.columnar_ensure_am_depends_catalog() IS 'internal function responsible for creating dependencies from columnar ' 'table access method to the rel objects in columnar schema'; diff --git a/src/test/regress/expected/multi_extension.out b/src/test/regress/expected/multi_extension.out index 092ec9e5c..ead2a5b85 100644 --- a/src/test/regress/expected/multi_extension.out +++ b/src/test/regress/expected/multi_extension.out @@ -1258,6 +1258,43 @@ SELECT * FROM pg_dist_cleanup; 2 | 0 | 1 | table_with_orphaned_shards_102011 | 0 | 0 (2 rows) +ALTER EXTENSION citus_columnar UPDATE TO '11.2-1'; +-- Make sure that we defined dependencies from all rel objects (tables, +-- indexes, sequences ..) to columnar table access method ... +SELECT pg_class.oid INTO columnar_schema_members +FROM pg_class, pg_namespace +WHERE pg_namespace.oid=pg_class.relnamespace AND + pg_namespace.nspname='columnar_internal' AND + pg_class.relname NOT IN ('chunk_group_pkey', + 'chunk_pkey', + 'options_pkey', + 'stripe_first_row_number_idx', + 'stripe_pkey'); +SELECT refobjid INTO columnar_schema_members_pg_depend +FROM pg_depend +WHERE classid = 'pg_am'::regclass::oid AND + objid = (select oid from pg_am where amname = 'columnar') AND + objsubid = 0 AND + refclassid = 'pg_class'::regclass::oid AND + refobjsubid = 0 AND + deptype = 'n'; +-- ... , so this should be empty, +(TABLE columnar_schema_members EXCEPT TABLE columnar_schema_members_pg_depend) +UNION +(TABLE columnar_schema_members_pg_depend EXCEPT TABLE columnar_schema_members); + oid +--------------------------------------------------------------------- +(0 rows) + +-- ... , and both columnar_schema_members_pg_depend & columnar_schema_members +-- should have 5 entries. +SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend; + ?column? +--------------------------------------------------------------------- + t +(1 row) + +DROP TABLE columnar_schema_members, columnar_schema_members_pg_depend; -- error out as cleanup records remain ALTER EXTENSION citus UPDATE TO '11.0-4'; ERROR: pg_dist_cleanup is introduced in Citus 11.1 diff --git a/src/test/regress/expected/upgrade_columnar_after.out b/src/test/regress/expected/upgrade_columnar_after.out index 8bb09d861..768a057f9 100644 --- a/src/test/regress/expected/upgrade_columnar_after.out +++ b/src/test/regress/expected/upgrade_columnar_after.out @@ -267,7 +267,12 @@ ROLLBACK; SELECT pg_class.oid INTO columnar_schema_members FROM pg_class, pg_namespace WHERE pg_namespace.oid=pg_class.relnamespace AND - pg_namespace.nspname='columnar_internal'; + pg_namespace.nspname='columnar_internal' AND + pg_class.relname NOT IN ('chunk_group_pkey', + 'chunk_pkey', + 'options_pkey', + 'stripe_first_row_number_idx', + 'stripe_pkey'); SELECT refobjid INTO columnar_schema_members_pg_depend FROM pg_depend WHERE classid = 'pg_am'::regclass::oid AND @@ -285,8 +290,8 @@ UNION (0 rows) -- ... , and both columnar_schema_members_pg_depend & columnar_schema_members --- should have 10 entries. -SELECT COUNT(*)=10 FROM columnar_schema_members_pg_depend; +-- should have 5 entries. +SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend; ?column? --------------------------------------------------------------------- t @@ -299,7 +304,12 @@ $$ SELECT pg_class.oid INTO columnar_schema_members FROM pg_class, pg_namespace WHERE pg_namespace.oid=pg_class.relnamespace AND - pg_namespace.nspname='columnar_internal'; + pg_namespace.nspname='columnar_internal' AND + pg_class.relname NOT IN ('chunk_group_pkey', + 'chunk_pkey', + 'options_pkey', + 'stripe_first_row_number_idx', + 'stripe_pkey'); SELECT refobjid INTO columnar_schema_members_pg_depend FROM pg_depend WHERE classid = 'pg_am'::regclass::oid AND @@ -312,8 +322,8 @@ $$ ); success | result --------------------------------------------------------------------- - t | SELECT 10 - t | SELECT 10 + t | SELECT 5 + t | SELECT 5 (2 rows) SELECT success, result FROM run_command_on_workers( @@ -331,7 +341,7 @@ $$ SELECT success, result FROM run_command_on_workers( $$ -SELECT COUNT(*)=10 FROM columnar_schema_members_pg_depend; +SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend; $$ ); success | result diff --git a/src/test/regress/sql/multi_extension.sql b/src/test/regress/sql/multi_extension.sql index 8c8ade9d8..d202227ae 100644 --- a/src/test/regress/sql/multi_extension.sql +++ b/src/test/regress/sql/multi_extension.sql @@ -556,6 +556,39 @@ ALTER EXTENSION citus UPDATE TO '11.2-1'; SELECT * FROM pg_dist_placement ORDER BY shardid; SELECT * FROM pg_dist_cleanup; +ALTER EXTENSION citus_columnar UPDATE TO '11.2-1'; + +-- Make sure that we defined dependencies from all rel objects (tables, +-- indexes, sequences ..) to columnar table access method ... +SELECT pg_class.oid INTO columnar_schema_members +FROM pg_class, pg_namespace +WHERE pg_namespace.oid=pg_class.relnamespace AND + pg_namespace.nspname='columnar_internal' AND + pg_class.relname NOT IN ('chunk_group_pkey', + 'chunk_pkey', + 'options_pkey', + 'stripe_first_row_number_idx', + 'stripe_pkey'); +SELECT refobjid INTO columnar_schema_members_pg_depend +FROM pg_depend +WHERE classid = 'pg_am'::regclass::oid AND + objid = (select oid from pg_am where amname = 'columnar') AND + objsubid = 0 AND + refclassid = 'pg_class'::regclass::oid AND + refobjsubid = 0 AND + deptype = 'n'; + +-- ... , so this should be empty, +(TABLE columnar_schema_members EXCEPT TABLE columnar_schema_members_pg_depend) +UNION +(TABLE columnar_schema_members_pg_depend EXCEPT TABLE columnar_schema_members); + +-- ... , and both columnar_schema_members_pg_depend & columnar_schema_members +-- should have 5 entries. +SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend; + +DROP TABLE columnar_schema_members, columnar_schema_members_pg_depend; + -- error out as cleanup records remain ALTER EXTENSION citus UPDATE TO '11.0-4'; diff --git a/src/test/regress/sql/upgrade_columnar_after.sql b/src/test/regress/sql/upgrade_columnar_after.sql index cf54ec80a..133fcfde0 100644 --- a/src/test/regress/sql/upgrade_columnar_after.sql +++ b/src/test/regress/sql/upgrade_columnar_after.sql @@ -139,7 +139,12 @@ ROLLBACK; SELECT pg_class.oid INTO columnar_schema_members FROM pg_class, pg_namespace WHERE pg_namespace.oid=pg_class.relnamespace AND - pg_namespace.nspname='columnar_internal'; + pg_namespace.nspname='columnar_internal' AND + pg_class.relname NOT IN ('chunk_group_pkey', + 'chunk_pkey', + 'options_pkey', + 'stripe_first_row_number_idx', + 'stripe_pkey'); SELECT refobjid INTO columnar_schema_members_pg_depend FROM pg_depend WHERE classid = 'pg_am'::regclass::oid AND @@ -155,8 +160,8 @@ UNION (TABLE columnar_schema_members_pg_depend EXCEPT TABLE columnar_schema_members); -- ... , and both columnar_schema_members_pg_depend & columnar_schema_members --- should have 10 entries. -SELECT COUNT(*)=10 FROM columnar_schema_members_pg_depend; +-- should have 5 entries. +SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend; DROP TABLE columnar_schema_members, columnar_schema_members_pg_depend; @@ -167,7 +172,12 @@ $$ SELECT pg_class.oid INTO columnar_schema_members FROM pg_class, pg_namespace WHERE pg_namespace.oid=pg_class.relnamespace AND - pg_namespace.nspname='columnar_internal'; + pg_namespace.nspname='columnar_internal' AND + pg_class.relname NOT IN ('chunk_group_pkey', + 'chunk_pkey', + 'options_pkey', + 'stripe_first_row_number_idx', + 'stripe_pkey'); SELECT refobjid INTO columnar_schema_members_pg_depend FROM pg_depend WHERE classid = 'pg_am'::regclass::oid AND @@ -189,7 +199,7 @@ $$ SELECT success, result FROM run_command_on_workers( $$ -SELECT COUNT(*)=10 FROM columnar_schema_members_pg_depend; +SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend; $$ );