From 1795eb2cd54f30d2f5c0717bb53c48c436c17c13 Mon Sep 17 00:00:00 2001 From: James Coleman Date: Wed, 16 Oct 2019 09:43:47 -0400 Subject: [PATCH] Fix (unique) index inheritance from template (#278) v4.2.2 Fix (unique) index inheritance from template --- CHANGELOG.txt | 3 + META.json | 6 +- pg_partman.control | 2 +- sql/functions/inherit_template_properties.sql | 2 + test/test_native/test-id-native.sql | 16 +- updates/pg_partman--4.2.1--4.2.2.sql | 221 ++++++++++++++++++ 6 files changed, 242 insertions(+), 8 deletions(-) create mode 100644 updates/pg_partman--4.2.1--4.2.2.sql diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 2fe15992..3c2ddaa4 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,6 @@ +4.2.2 +-- Fix bug in template table index inheritance that prevented any additional indexes from being copied after finding an index definition that exists on both the template and parent tables. Github Pull Request #278. + 4.2.1 -- Fix bug in partition_data_time causing never-ending loop while calculating partition range for natively partitioned tables. Github issue #273 diff --git a/META.json b/META.json index 636d83a4..be58e004 100644 --- a/META.json +++ b/META.json @@ -1,7 +1,7 @@ { "name": "pg_partman", "abstract": "Extension to manage partitioned tables by time or ID", - "version": "4.2.1", + "version": "4.2.2", "maintainer": [ "Keith Fiske " ], @@ -20,9 +20,9 @@ }, "provides": { "pg_partman": { - "file": "sql/pg_partman--4.2.1.sql", + "file": "sql/pg_partman--4.2.2.sql", "docfile": "doc/pg_partman.md", - "version": "4.2.1", + "version": "4.2.2", "abstract": "Extension to manage partitioned tables by time or ID" } }, diff --git a/pg_partman.control b/pg_partman.control index b53995f4..c084e60d 100644 --- a/pg_partman.control +++ b/pg_partman.control @@ -1,3 +1,3 @@ -default_version = '4.2.1' +default_version = '4.2.2' comment = 'Extension to manage partitioned tables by time or ID' relocatable = false diff --git a/sql/functions/inherit_template_properties.sql b/sql/functions/inherit_template_properties.sql index 3972c56e..b19db3e3 100644 --- a/sql/functions/inherit_template_properties.sql +++ b/sql/functions/inherit_template_properties.sql @@ -98,6 +98,8 @@ IF current_setting('server_version_num')::int >= 100000 THEN AND i.indisvalid ORDER BY 1 LOOP + v_dupe_found := false; + IF current_setting('server_version_num')::int >= 110000 THEN FOR v_parent_index_list IN SELECT diff --git a/test/test_native/test-id-native.sql b/test/test_native/test-id-native.sql index 93ef08ea..8e807f9d 100644 --- a/test/test_native/test-id-native.sql +++ b/test/test_native/test-id-native.sql @@ -9,7 +9,7 @@ BEGIN; SELECT set_config('search_path','partman, public',false); -SELECT plan(115); +SELECT plan(120); CREATE SCHEMA partman_test; CREATE SCHEMA partman_retention_test; CREATE ROLE partman_basic; @@ -29,8 +29,6 @@ GRANT SELECT,INSERT,UPDATE ON partman_test.id_taptest_table TO partman_basic, PU GRANT ALL ON partman_test.id_taptest_table TO partman_revoke; -- Template table CREATE UNLOGGED TABLE partman_test.template_id_taptest_table (LIKE partman_test.id_taptest_table); --- Regular unique indexes do not work on native in PG11 if the partition key isn't included -CREATE UNIQUE INDEX ON partman_test.template_id_taptest_table (col4); DO $pg11_objects_check$ BEGIN @@ -43,10 +41,15 @@ ELSE -- Create on template table ALTER TABLE partman_test.template_id_taptest_table ADD PRIMARY KEY (col1); ALTER TABLE partman_test.template_id_taptest_table ADD FOREIGN KEY (col2) REFERENCES partman_test.fk_test_reference(col2); - CREATE INDEX ON partman_test.template_id_taptest_table (col3); END IF; END $pg11_objects_check$; +-- Always create the index on teh template also so that we can test excluding duplicates. +CREATE INDEX ON partman_test.template_id_taptest_table (col3); + +-- Regular unique indexes do not work on native in PG11 if the partition key isn't included +CREATE UNIQUE INDEX ON partman_test.template_id_taptest_table (col4); + SELECT create_parent('partman_test.id_taptest_table', 'col1', 'native', '10', p_jobmon := false, p_start_partition := '3000000000', p_template_table := 'partman_test.template_id_taptest_table'); UPDATE part_config SET inherit_privileges = TRUE; SELECT reapply_privileges('partman_test.id_taptest_table'); @@ -69,6 +72,11 @@ SELECT col_is_fk('partman_test', 'id_taptest_table_p3000000010', 'col2', 'Check SELECT col_is_fk('partman_test', 'id_taptest_table_p3000000020', 'col2', 'Check that foreign key was inherited to id_taptest_table_p3000000020'); SELECT col_is_fk('partman_test', 'id_taptest_table_p3000000030', 'col2', 'Check that foreign key was inherited to id_taptest_table_p3000000030'); SELECT col_is_fk('partman_test', 'id_taptest_table_p3000000040', 'col2', 'Check that foreign key was inherited to id_taptest_table_p3000000040'); +SELECT is_indexed('partman_test', 'id_taptest_table_p3000000000', 'col4', 'Check that unique index was inherited to id_taptest_table_p3000000000'); +SELECT is_indexed('partman_test', 'id_taptest_table_p3000000010', 'col4', 'Check that unique index was inherited to id_taptest_table_p3000000010'); +SELECT is_indexed('partman_test', 'id_taptest_table_p3000000020', 'col4', 'Check that unique index was inherited to id_taptest_table_p3000000020'); +SELECT is_indexed('partman_test', 'id_taptest_table_p3000000030', 'col4', 'Check that unique index was inherited to id_taptest_table_p3000000030'); +SELECT is_indexed('partman_test', 'id_taptest_table_p3000000040', 'col4', 'Check that unique index was inherited to id_taptest_table_p3000000040'); SELECT table_privs_are('partman_test', 'id_taptest_table_p3000000000', 'partman_basic', ARRAY['SELECT','INSERT','UPDATE'], 'Check partman_basic privileges of id_taptest_table_p3000000000'); SELECT table_privs_are('partman_test', 'id_taptest_table_p3000000010', 'partman_basic', ARRAY['SELECT','INSERT','UPDATE'], 'Check partman_basic privileges of id_taptest_table_p3000000010'); SELECT table_privs_are('partman_test', 'id_taptest_table_p3000000020', 'partman_basic', ARRAY['SELECT','INSERT','UPDATE'], 'Check partman_basic privileges of id_taptest_table_p3000000020'); diff --git a/updates/pg_partman--4.2.1--4.2.2.sql b/updates/pg_partman--4.2.1--4.2.2.sql new file mode 100644 index 00000000..7cd93488 --- /dev/null +++ b/updates/pg_partman--4.2.1--4.2.2.sql @@ -0,0 +1,221 @@ +CREATE OR REPLACE FUNCTION @extschema@.inherit_template_properties (p_parent_table text, p_child_schema text, p_child_tablename text) RETURNS boolean + LANGUAGE plpgsql + AS $$ +DECLARE + +v_child_relkind char; +v_child_schema text; +v_child_tablename text; +v_child_unlogged char; +v_dupe_found boolean := false; +v_fk_list record; +v_index_list record; +v_inherit_fk boolean; +v_parent_index_list record; +v_parent_oid oid; +v_parent_table text; +v_sql text; +v_template_oid oid; +v_template_schemaname text; +v_template_table text; +v_template_tablename name; +v_template_tablespace name; +v_template_unlogged char; + +BEGIN +/* + * Function to inherit the properties of the template table to newly created child tables. + * Currently used for PostgreSQL 10 to inherit indexes and FKs since that is not natively available + * For PG11, used to inherit non-partition-key unique indexes & primary keys + */ + +SELECT parent_table, template_table, inherit_fk +INTO v_parent_table, v_template_table, v_inherit_fk +FROM @extschema@.part_config +WHERE parent_table = p_parent_table; +IF v_parent_table IS NULL THEN + RAISE EXCEPTION 'Given parent table has no configuration in pg_partman: %', p_parent_table; +ELSIF v_template_table IS NULL THEN + RAISE EXCEPTION 'No template table set in configuration for given parent table: %', p_parent_table; +END IF; + +SELECT c.oid INTO v_parent_oid +FROM pg_catalog.pg_class c +JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid +WHERE n.nspname = split_part(p_parent_table, '.', 1)::name +AND c.relname = split_part(p_parent_table, '.', 2)::name; + IF v_parent_oid IS NULL THEN + RAISE EXCEPTION 'Unable to find given parent table in system catalogs: %', p_parent_table; + END IF; + +SELECT n.nspname, c.relname, c.relkind INTO v_child_schema, v_child_tablename, v_child_relkind +FROM pg_catalog.pg_class c +JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid +WHERE n.nspname = p_child_schema::name +AND c.relname = p_child_tablename::name; + IF v_child_tablename IS NULL THEN + RAISE EXCEPTION 'Unable to find given child table in system catalogs: %.%', v_child_schema, v_child_tablename; + END IF; + +IF v_child_relkind = 'p' THEN + -- Subpartitioned parent, do not apply properties + RAISE DEBUG 'inherit_template_properties: found given child is subpartition parent, so properties not inherited'; + RETURN false; +END IF; + +v_template_schemaname := split_part(v_template_table, '.', 1)::name; +v_template_tablename := split_part(v_template_table, '.', 2)::name; + + SELECT c.oid, ts.spcname INTO v_template_oid, v_template_tablespace +FROM pg_catalog.pg_class c +JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid +LEFT OUTER JOIN pg_catalog.pg_tablespace ts ON c.reltablespace = ts.oid +WHERE n.nspname = v_template_schemaname +AND c.relname = v_template_tablename; + IF v_template_oid IS NULL THEN + RAISE EXCEPTION 'Unable to find configured template table in system catalogs: %', v_template_table; + END IF; + +-- Index creation (Required for all indexes in PG10. Only for non-unique, non-partition key indexes in PG11) +IF current_setting('server_version_num')::int >= 100000 THEN + FOR v_index_list IN + SELECT + array_to_string(regexp_matches(pg_get_indexdef(indexrelid), ' USING .*'),',') AS statement + , i.indisprimary + , i.indisunique + , ( SELECT array_agg( a.attname ORDER by x.r ) + FROM pg_catalog.pg_attribute a + JOIN ( SELECT k, row_number() over () as r + FROM unnest(i.indkey) k ) as x + ON a.attnum = x.k AND a.attrelid = i.indrelid + ) AS indkey_names + , c.relname AS index_name + , ts.spcname AS tablespace_name + FROM pg_catalog.pg_index i + JOIN pg_catalog.pg_class c ON i.indexrelid = c.oid + LEFT OUTER JOIN pg_catalog.pg_tablespace ts ON c.reltablespace = ts.oid + WHERE i.indrelid = v_template_oid + AND i.indisvalid + ORDER BY 1 + LOOP + v_dupe_found := false; + + IF current_setting('server_version_num')::int >= 110000 THEN + FOR v_parent_index_list IN + SELECT + array_to_string(regexp_matches(pg_get_indexdef(indexrelid), ' USING .*'),',') AS statement + , i.indisprimary + , ( SELECT array_agg( a.attname ORDER by x.r ) + FROM pg_catalog.pg_attribute a + JOIN ( SELECT k, row_number() over () as r + FROM unnest(i.indkey) k ) as x + ON a.attnum = x.k AND a.attrelid = i.indrelid + ) AS indkey_names + FROM pg_catalog.pg_index i + WHERE i.indrelid = v_parent_oid + AND i.indisvalid + ORDER BY 1 + LOOP + + IF v_parent_index_list.indisprimary AND v_index_list.indisprimary THEN + IF v_parent_index_list.indkey_names = v_index_list.indkey_names THEN + RAISE DEBUG 'Ignoring duplicate primary key on template table: % ', v_index_list.indkey_names; + v_dupe_found := true; + CONTINUE; -- only continue within this nested loop + END IF; + END IF; + + IF v_parent_index_list.statement = v_index_list.statement THEN + RAISE DEBUG 'Ignoring duplicate index on template table: %', v_index_list.statement; + v_dupe_found := true; + CONTINUE; -- only continue within this nested loop + END IF; + + END LOOP; -- end parent index loop + END IF; -- End PG11 check + + IF v_dupe_found = true THEN + -- Only used in PG11 and should skip trying to create indexes that already existed on the parent + CONTINUE; + END IF; + + IF v_index_list.indisprimary THEN + v_sql := format('ALTER TABLE %I.%I ADD PRIMARY KEY (%s)' + , v_child_schema + , v_child_tablename + , '"' || array_to_string(v_index_list.indkey_names, '","') || '"'); + IF v_index_list.tablespace_name IS NOT NULL THEN + v_sql := v_sql || format(' USING INDEX TABLESPACE %I', v_index_list.tablespace_name); + END IF; + RAISE DEBUG 'Create pk: %', v_sql; + EXECUTE v_sql; + ELSE + -- statement column should be just the portion of the index definition that defines what it actually is + v_sql := format('CREATE %s INDEX ON %I.%I %s', CASE WHEN v_index_list.indisunique = TRUE THEN 'UNIQUE' ELSE '' END, v_child_schema, v_child_tablename, v_index_list.statement); + IF v_index_list.tablespace_name IS NOT NULL THEN + v_sql := v_sql || format(' TABLESPACE %I', v_index_list.tablespace_name); + END IF; + + RAISE DEBUG 'Create index: %', v_sql; + EXECUTE v_sql; + + END IF; + + END LOOP; +END IF; +-- End index creation + +-- Foreign key creation (PG10 only) +IF current_setting('server_version_num')::int >= 100000 AND current_setting('server_version_num')::int < 110000 THEN + IF v_inherit_fk THEN + FOR v_fk_list IN + SELECT pg_get_constraintdef(con.oid) AS constraint_def + FROM pg_catalog.pg_constraint con + JOIN pg_catalog.pg_class c ON con.conrelid = c.oid + WHERE c.oid = v_template_oid + AND contype = 'f' + LOOP + v_sql := format('ALTER TABLE %I.%I ADD %s', v_child_schema, v_child_tablename, v_fk_list.constraint_def); + RAISE DEBUG 'Create FK: %', v_sql; + EXECUTE v_sql; + END LOOP; + END IF; +END IF; +-- End foreign key creation + +-- Tablespace inheritance +IF v_template_tablespace IS NOT NULL THEN + v_sql := format('ALTER TABLE %I.%I SET TABLESPACE %I', v_child_schema, v_child_tablename, v_template_tablespace); + RAISE DEBUG 'Alter tablespace: %', v_sql; + EXECUTE v_sql; +END IF; + +-- UNLOGGED status. Currently waiting on final stance of how native will handle this property being changed for its children. +-- See release notes for v4.2.0 +SELECT relpersistence INTO v_template_unlogged +FROM pg_catalog.pg_class c +JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid +WHERE n.nspname = v_template_schemaname +AND c.relname = v_template_tablename; + +SELECT relpersistence INTO v_child_unlogged +FROM pg_catalog.pg_class c +JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid +WHERE n.nspname = v_child_schema::name +AND c.relname = v_child_tablename::name; + +IF v_template_unlogged = 'u' AND v_child_unlogged = 'p' THEN + v_sql := format ('ALTER TABLE %I.%I SET UNLOGGED', v_child_schema, v_child_tablename); + RAISE DEBUG 'Alter UNLOGGED: %', v_sql; + EXECUTE v_sql; +ELSIF v_template_unlogged = 'p' AND v_child_unlogged = 'u' THEN + v_sql := format ('ALTER TABLE %I.%I SET LOGGED', v_child_schema, v_child_tablename); + RAISE DEBUG 'Alter UNLOGGED: %', v_sql; + EXECUTE v_sql; +END IF; + +RETURN true; + +END +$$; +