Skip to content

Commit

Permalink
[#23163] YSQL: pg_partman: Make partition creation idempotent
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Devansh Saxena committed Jul 23, 2024
1 parent 2248dcd commit 7b32d05
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
/*
Expand Down Expand Up @@ -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 ';
Expand All @@ -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+
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -158,16 +174,30 @@ 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
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_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;
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 7b32d05

Please sign in to comment.