From 7b32d05895d50fe3fd273548832bae4d4bd69fbf Mon Sep 17 00:00:00 2001 From: Devansh Saxena Date: Tue, 23 Jul 2024 00:29:24 +0530 Subject: [PATCH] [#23163] YSQL: pg_partman: Make partition creation idempotent Summary: Currently transactional DDL is not supported in YugabyteDB . Due to which if any procedure or function which are performing multiple DDLs in a transactional context can lead to issue of DDLs getting executed and committed even if stored procedure failed due to some exception while running stored procedure or ungraceful Postgres backend process or tserver process kill. pg_partman extension uses `create_parent` function as main function to create child partitions of a partitioned table. `create_parent` itself calls other pg_partman functions that performs DDLs to create and attach partitioned tables and add some other objects like indexes, constraints and adding privileges. Hence it required to make complete workflow of `create_parent` idempotent. create_parent.sql - Base function called for creating child partitions for a partitioned table. - Changes done - Make default table creation idempotent with not-exist addition - Check before attaching default table if it is already attached create_partition_time/id.sql - Functions called by create_parent and run_maintenance. Creation and attaching of child tables except for the default child table occurs here. the default child is handled at create_parent only. - Changes done - Get list of child tables attached to parent table into v_child_tables - Loop over the partition names for which want to create and attach new partition, name it is partition_name - If partition_name is present in v_child_tables, then continue - If partition_name is not attached but created, then skip creation -> have to test this -> made changes. Jira: DB-12101 Test Plan: jenkins: compile only Reviewers: skumar, hsunder, jason Reviewed By: jason Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D36428 --- .../sql/functions/create_parent.sql | 10 +++- .../sql/functions/create_partition_id.sql | 35 ++++++++++- .../sql/functions/create_partition_time.sql | 60 +++++++++++++++---- 3 files changed, 91 insertions(+), 14 deletions(-) diff --git a/src/postgres/third-party-extensions/pg_partman/sql/functions/create_parent.sql b/src/postgres/third-party-extensions/pg_partman/sql/functions/create_parent.sql index 75c09cea6ee8..567f392656ae 100644 --- a/src/postgres/third-party-extensions/pg_partman/sql/functions/create_parent.sql +++ b/src/postgres/third-party-extensions/pg_partman/sql/functions/create_parent.sql @@ -74,6 +74,7 @@ v_top_datetime_string text; v_top_parent_schema text := split_part(p_parent_table, '.', 1); v_top_parent_table text := split_part(p_parent_table, '.', 2); v_unlogged char; +yb_v_parent_table_default regclass; BEGIN /* @@ -744,7 +745,7 @@ IF p_type = 'native' AND current_setting('server_version_num')::int >= 110000 TH */ -- Same INCLUDING list is used in create_partition_*(). INDEXES is handled when partition is attached if it's supported. - v_sql := v_sql || format(' TABLE %I.%I (LIKE %I.%I INCLUDING DEFAULTS INCLUDING CONSTRAINTS INCLUDING STORAGE INCLUDING COMMENTS ' + v_sql := v_sql || format(' TABLE IF NOT EXISTS %I.%I (LIKE %I.%I INCLUDING DEFAULTS INCLUDING CONSTRAINTS INCLUDING STORAGE INCLUDING COMMENTS ' , v_parent_schema, v_default_partition, v_parent_schema, v_parent_tablename); IF current_setting('server_version_num')::int >= 120000 THEN v_sql := v_sql || ' INCLUDING GENERATED '; @@ -753,7 +754,12 @@ IF p_type = 'native' AND current_setting('server_version_num')::int >= 110000 TH EXECUTE v_sql; v_sql := format('ALTER TABLE %I.%I ATTACH PARTITION %I.%I DEFAULT' , v_parent_schema, v_parent_tablename, v_parent_schema, v_default_partition); - EXECUTE v_sql; + SELECT inhparent::regclass INTO yb_v_parent_table_default + FROM pg_inherits + WHERE inhrelid = (quote_ident(v_parent_schema) || '.' || quote_ident(v_default_partition))::regclass; + IF yb_v_parent_table_default is NULL THEN + EXECUTE v_sql; + END IF; IF p_publications IS NOT NULL THEN -- NOTE: Native publication inheritance is only supported on PG14+ diff --git a/src/postgres/third-party-extensions/pg_partman/sql/functions/create_partition_id.sql b/src/postgres/third-party-extensions/pg_partman/sql/functions/create_partition_id.sql index e4eb825cd83a..0bf15bef6b4b 100644 --- a/src/postgres/third-party-extensions/pg_partman/sql/functions/create_partition_id.sql +++ b/src/postgres/third-party-extensions/pg_partman/sql/functions/create_partition_id.sql @@ -41,12 +41,27 @@ v_sub_id_max bigint; v_sub_id_min bigint; v_template_table text; v_unlogged char; +yb_v_child_tables text[] := '{}'; +yb_v_child_table record; +yb_v_is_child_table_attached boolean; BEGIN /* * Function to create id partitions */ +-- Fetch child tables using pg_inherits +FOR yb_v_child_table IN + SELECT child.relname + FROM pg_inherits + JOIN pg_class parent ON pg_inherits.inhparent = parent.oid + JOIN pg_class child ON pg_inherits.inhrelid = child.oid + WHERE parent.relname = split_part(p_parent_table, '.', 2) +LOOP + -- Append each child table name to the array + yb_v_child_tables := array_append(yb_v_child_tables, yb_v_child_table.relname::text); +END LOOP; + SELECT control , partition_type , partition_interval @@ -114,15 +129,29 @@ FOREACH v_id IN ARRAY p_partition_ids LOOP END IF; v_partition_name := @extschema@.check_name_length(v_parent_tablename, v_id::text, TRUE); + + -- YB: check if child table is already attached + yb_v_is_child_table_attached := v_partition_name = ANY(yb_v_child_tables); + IF yb_v_is_child_table_attached THEN + RAISE NOTICE 'Table % already attached', v_partition_name; + v_partition_created := true; + CONTINUE; + END IF; + -- If child table already exists, skip creation -- Have to check pg_class because if subpartitioned, table will not be in pg_tables SELECT c.relname INTO v_exists FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid WHERE n.nspname = v_parent_schema::name AND c.relname = v_partition_name::name; + /* YB: Don't continue whole loop iteration, there could be + case that child partition was created but not attached due to + previous create_partition_id failed as YB does not support + transactional DDL. IF v_exists IS NOT NULL THEN CONTINUE; END IF; + */ -- Ensure analyze is run if a new partition is created. Otherwise if one isn't, will be false and analyze will be skipped v_analyze := TRUE; @@ -180,8 +209,10 @@ FOREACH v_id IN ARRAY p_partition_ids LOOP END IF; END IF; - RAISE DEBUG 'create_partition_id v_sql: %', v_sql; - EXECUTE v_sql; + IF v_exists IS NULL THEN + RAISE DEBUG 'create_partition_id v_sql: %', v_sql; + EXECUTE v_sql; + END IF; IF v_partition_type = 'native' THEN diff --git a/src/postgres/third-party-extensions/pg_partman/sql/functions/create_partition_time.sql b/src/postgres/third-party-extensions/pg_partman/sql/functions/create_partition_time.sql index 3dde5d3ec304..db8459ca45b9 100644 --- a/src/postgres/third-party-extensions/pg_partman/sql/functions/create_partition_time.sql +++ b/src/postgres/third-party-extensions/pg_partman/sql/functions/create_partition_time.sql @@ -54,12 +54,28 @@ v_time timestamptz; v_partition_type text; v_unlogged char; v_year text; +yb_v_child_table record; +yb_v_child_tables text[] := '{}'; +yb_v_is_child_table_attached boolean; +yb_v_constraint_name text; BEGIN /* * Function to create a child table in a time-based partition set */ +-- Fetch child tables using pg_inherits +FOR yb_v_child_table IN + SELECT child.relname + FROM pg_inherits + JOIN pg_class parent ON pg_inherits.inhparent = parent.oid + JOIN pg_class child ON pg_inherits.inhrelid = child.oid + WHERE parent.relname = split_part(p_parent_table, '.', 2) +LOOP + -- Append each child table name to the array + yb_v_child_tables := array_append(yb_v_child_tables, yb_v_child_table.relname::text); +END LOOP; + SELECT partition_type , control , partition_interval @@ -158,6 +174,15 @@ FOREACH v_time IN ARRAY p_partition_times LOOP -- This suffix generation code is in partition_data_time() as well v_partition_suffix := to_char(v_time, v_datetime_string); v_partition_name := @extschema@.check_name_length(v_parent_tablename, v_partition_suffix, TRUE); + + -- YB: check if child table is already attached + yb_v_is_child_table_attached := v_partition_name = ANY(yb_v_child_tables); + IF yb_v_is_child_table_attached THEN + RAISE NOTICE 'Table % already attached', v_partition_name; + v_partition_created := true; + CONTINUE; + END IF; + -- Check if child exists. SELECT count(*) INTO v_exists FROM pg_catalog.pg_class c @@ -165,9 +190,14 @@ FOREACH v_time IN ARRAY p_partition_times LOOP WHERE n.nspname = v_parent_schema::name AND c.relname = v_partition_name::name; + /* YB: Don't continue whole loop iteration, there could be + case that child partition was created but not attached due to + previous create_partition_time failed as YB does not support + transactional DDL. IF v_exists > 0 THEN CONTINUE; END IF; + */ -- Ensure analyze is run if a new partition is created. Otherwise if one isn't, will be false and analyze will be skipped v_analyze := TRUE; @@ -229,9 +259,10 @@ FOREACH v_time IN ARRAY p_partition_times LOOP END IF; END IF; - RAISE DEBUG 'create_partition_time v_sql: %', v_sql; - EXECUTE v_sql; - + IF v_exists IS NULL THEN + RAISE DEBUG 'create_partition_time v_sql: %', v_sql; + EXECUTE v_sql; + END IF; IF v_partition_type = 'native' THEN @@ -284,13 +315,22 @@ FOREACH v_time IN ARRAY p_partition_times LOOP , EXTRACT('epoch' FROM v_partition_timestamp_end)::bigint * 1000000000); END IF; -- Create secondary, time-based constraint since native's constraint is already integer based - EXECUTE format('ALTER TABLE %I.%I ADD CONSTRAINT %I CHECK (%s >= %L AND %4$s < %6$L)' - , v_parent_schema - , v_partition_name - , v_partition_name||'_partition_check' - , v_partition_expression - , v_partition_timestamp_start - , v_partition_timestamp_end); + -- YB: Check if the constraint already exists + SELECT conname INTO yb_v_constraint_name + FROM pg_constraint + WHERE conname = v_partition_name||'_partition_check' + AND conrelid = (quote_ident(v_parent_schema) || '.' || quote_ident(v_partition_name))::regclass; + + -- YB: If the constraint does not exist, create it + IF yb_v_constraint_name IS NULL THEN + EXECUTE format('ALTER TABLE %I.%I ADD CONSTRAINT %I CHECK (%s >= %L AND %4$s < %6$L)' + , v_parent_schema + , v_partition_name + , v_partition_name||'_partition_check' + , v_partition_expression + , v_partition_timestamp_start + , v_partition_timestamp_end); + END IF; END IF; ELSE -- non-native