Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug with replica identity USING INDEX inheritance in 5.1 #721

Open
mor-lehr-deel opened this issue Dec 17, 2024 · 7 comments
Open

Bug with replica identity USING INDEX inheritance in 5.1 #721

mor-lehr-deel opened this issue Dec 17, 2024 · 7 comments

Comments

@mor-lehr-deel
Copy link

mor-lehr-deel commented Dec 17, 2024

Hi,

we are using pg_partman version 5.1.
when we run CALL partman.run_maintenance_proc(); we get the following error:

ERROR: "parent_index_name" is not an index for table "child_table_p20250501"
CONTEXT: SQL statement "ALTER TABLE public.child_table_p20250501 REPLICA IDENTITY USING INDEX parent_index_name"

It seems like there is a bug in partman.inherit_replica_identity - it finds the correct replica identity index on the parent - however it assumes that the child table would have an index with the exact same name, which is not possible...

instead - it should find the index on the child table which is attached to the parent index (assuming the index was attached to the parent index as part of the partition creation), something like:

SELECT partition_index.indexrelid::regclass::text
FROM pg_index parent_index -- Main index
INNER JOIN pg_inherits index_inheritance ON (index_inheritance.inhparent=parent_index.indexrelid) -- Connection between Main and Partition indexes
INNER JOIN pg_index partition_index ON (index_inheritance.inhrelid=partition_index.indexrelid) -- Partition index
INNER JOIN pg_class partition_table ON (partition_table.oid=partition_index.indrelid) -- Partition table
WHERE partition_table.oid=v_parent_oid
AND parent_index.indexrelid=v_parent_replident_index::regclass
@keithf4 keithf4 self-assigned this Dec 17, 2024
@keithf4
Copy link
Collaborator

keithf4 commented Dec 17, 2024

Any chance you could share the schema of the parent table (\d+ parent table) and the child table involved. Also the contents of the part_config entry for this partition set?

@keithf4
Copy link
Collaborator

keithf4 commented Dec 17, 2024

Was just checking why my unit test was passing here. Looks like I'd only covered replica identity FULL. If I can get your table's schema, I can try and work out a unit test with that to fix the bug and make sure to cover an index based on then as well.

@mor-lehr-deel
Copy link
Author

mor-lehr-deel commented Dec 18, 2024

partman config:

parent_table       |control   |partition_interval|partition_type|premake|automatic_maintenance|template_table                      |retention|retention_schema|retention_keep_index|retention_keep_table|epoch|constraint_cols|optimize_constraint|infinite_time_partitions|datetime_string|jobmon|sub_partition_set_full|undo_in_progress|inherit_privileges|constraint_valid|ignore_default_data|default_table|date_trunc_interval|maintenance_order|retention_keep_publication|maintenance_last_run         |
-------------------+----------+------------------+--------------+-------+---------------------+------------------------------------+---------+----------------+--------------------+--------------------+-----+---------------+-------------------+------------------------+---------------+------+----------------------+----------------+------------------+----------------+-------------------+-------------+-------------------+-----------------+--------------------------+-----------------------------+
public.parent_table|created_at|1 mon             |range         |      4|on                   |partman.template_public_parent_table|         |                |true                |true                |none |NULL           |                 30|true                    |YYYYMMDD       |true  |false                 |false           |false             |true            |false              |true         |                   |                 |false                     |2024-12-16 00:00:00.233498+00|

parent_table definition (simplified - but should be enough to reproduce):

CREATE TABLE public.parent_table (
	id int8 GENERATED ALWAYS AS IDENTITY NOT NULL,
	created_at timestamptz NOT NULL,
	updated_at timestamptz NOT NULL,
	col1 varchar(320) NOT NULL,
	CONSTRAINT parent_table_index UNIQUE (id, created_at)
)
PARTITION BY RANGE (created_at);
CREATE INDEX parent_table_col1_idx ON ONLY public.parent_table USING btree (col1);
ALTER TABLE public.parent_table REPLICA IDENTITY USING INDEX parent_table_index;

the full log of running

SET time ZONE utc;
SET client_min_messages=debug1;
CALL partman.run_maintenance_proc();
v_sql run_maintenance_proc: SELECT partman.run_maintenance('public.parent_table', p_jobmon := 't', p_analyze := 'f')
run_maint: v_tables_list_sql: SELECT parent_table
                , partition_type
                , partition_interval
                , control
                , premake
                , undo_in_progress
                , sub_partition_set_full
                , epoch
                , infinite_time_partitions
                , retention
                , ignore_default_data
                , datetime_string
                , maintenance_order
            FROM partman.part_config
            WHERE undo_in_progress = false AND parent_table = 'public.parent_table'  ORDER BY maintenance_order ASC NULLS LAST, parent_table ASC NULLS LAST 
show_partitions: v_parent_schema: public, v_parent_tablename: parent_table, v_datetime_string: YYYYMMDD, v_control_type: time, v_exact_control_type: timestamptz
show_partitions: v_default_sql: SELECT n.nspname::text AS partition_schemaname
        , c.relname::text AS partition_name
        FROM pg_catalog.pg_inherits h
        JOIN pg_catalog.pg_class c ON c.oid = h.inhrelid
        JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid
        WHERE h.inhparent = 'public.parent_table'::regclass
        AND pg_get_expr(relpartbound, c.oid) = 'DEFAULT'
show_partitions: v_sql: SELECT n.nspname::text AS partition_schemaname
        , c.relname::text AS partition_name
        FROM pg_catalog.pg_inherits h
        JOIN pg_catalog.pg_class c ON c.oid = h.inhrelid
        JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid
        WHERE h.inhparent = 'public.parent_table'::regclass
    AND pg_get_expr(relpartbound, c.oid) != 'DEFAULT' 
        ORDER BY (regexp_match(pg_get_expr(c.relpartbound, c.oid, true), $REGEX$\(([^)]+)\) TO \(([^)]+)\)$REGEX$))[1]::text::timestamptz ASC 
run_maint: v_partition_expression: created_at
show_partitions: v_parent_schema: public, v_parent_tablename: parent_table, v_datetime_string: YYYYMMDD, v_control_type: time, v_exact_control_type: timestamptz
show_partitions: v_sql: SELECT n.nspname::text AS partition_schemaname
        , c.relname::text AS partition_name
        FROM pg_catalog.pg_inherits h
        JOIN pg_catalog.pg_class c ON c.oid = h.inhrelid
        JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid
        WHERE h.inhparent = 'public.parent_table'::regclass
    AND pg_get_expr(relpartbound, c.oid) != 'DEFAULT' 
        ORDER BY (regexp_match(pg_get_expr(c.relpartbound, c.oid, true), $REGEX$\(([^)]+)\) TO \(([^)]+)\)$REGEX$))[1]::text::timestamptz DESC 
run_maint: parent_table: public.parent_table, v_last_partition: parent_table_p20250401
show_partition_info: v_child_schema: public, v_child_tablename: parent_table_p20250401, v_control_type: time, v_exact_control_type: timestamptz
show_partitions: v_parent_schema: public, v_parent_tablename: parent_table, v_datetime_string: YYYYMMDD, v_control_type: time, v_exact_control_type: timestamptz
show_partitions: v_sql: SELECT n.nspname::text AS partition_schemaname
        , c.relname::text AS partition_name
        FROM pg_catalog.pg_inherits h
        JOIN pg_catalog.pg_class c ON c.oid = h.inhrelid
        JOIN pg_catalog.pg_namespace n ON c.relnamespace = n.oid
        WHERE h.inhparent = 'public.parent_table'::regclass
    AND pg_get_expr(relpartbound, c.oid) != 'DEFAULT' 
        ORDER BY (regexp_match(pg_get_expr(c.relpartbound, c.oid, true), $REGEX$\(([^)]+)\) TO \(([^)]+)\)$REGEX$))[1]::text::timestamptz DESC 
run_maint: v_current_partition_timestamp: 2024-12-18 07:46:15.602161+00, v_max_time_default: <NULL>
run_maint: v_max_timestamp: 2024-12-18 07:43:57.989+00, v_current_partition_timestamp: 2024-12-18 07:46:15.602161+00, v_max_time_default: <NULL>
run_maint before loop: last_partition_timestamp: 2025-04-01 00:00:00+00, current_partition_timestamp: 2024-12-18 07:46:15.602161+00, v_premade_count: 3, v_sub_timestamp_min: <NULL>, v_sub_timestamp_max: <NULL>
run_maint: parent_table: public.parent_table, v_premade_count: 3, v_next_partition_timestamp: 2025-04-01 00:00:00+00
create_partition_time: v_partition_expression: created_at
rehashing catalog cache id 11 for pg_authid; 33 tups, 16 buckets
rehashing catalog cache id 37 for pg_operator; 33 lists, 16 buckets
rehashing catalog cache id 44 for pg_proc; 33 lists, 16 buckets
create_partition_time v_sql: CREATE TABLE public.parent_table_p20250501 (LIKE public.parent_table INCLUDING DEFAULTS INCLUDING CONSTRAINTS INCLUDING STORAGE INCLUDING COMMENTS INCLUDING GENERATED) 
building index "pg_toast_1664206158_index" on table "pg_toast_1664206158" serially
index "pg_toast_1664206158_index" can safely use deduplication
ALTER TABLE / ADD UNIQUE will create implicit index "parent_table_p20250501_id_created_at_key" for table "parent_table_p20250501"
building index "parent_table_p20250501_id_created_at_key" on table "parent_table_p20250501" serially
index "parent_table_p20250501_id_created_at_key" can safely use deduplication
building index "parent_table_p20250501_col1_idx" on table "parent_table_p20250501" serially
index "parent_table_p20250501_to_col1_idx" can safely use deduplication
verifying table "parent_table_p20250501"
verifying table "parent_table_default"
inherit_replica_ident: v_parent_oid: 107107123, v_parent_replident: i,  v_parent_replident_index: parent_table_index
inherit_replica_identity: replident v_sql: ALTER TABLE public.parent_table_p20250501 REPLICA IDENTITY USING INDEX parent_table_index

note: we recently migrated from v4.7.3 ,which didn't have this problem, to v5.1.0

as a matter of fact, we were perfectly fine with pg_partman not handling replica identity - because we have other code which handles that for partitioned tables.

@keithf4
Copy link
Collaborator

keithf4 commented Dec 19, 2024

I have a fix for this up in the development branch (https://github.com/pgpartman/pg_partman). See commit 2052caf

Thanks for the query suggestion to match the parent and child inherited indexes. Had to make one adjustment in that it needs to match on the child table's OID, not the parent's. I liked the commenting on it too to help clarify the joins, but I adjusted it a bit for how the joins made sense in my own head.

Understand that you may have had the replica identities handled already, but everyone else may not so I did want to try and handle it in this extension for now. Honestly, I believe that it not being inherited in core is a bug, so hopefully they'll fix that upstream at some point.

Will work on a unit test as well, but if you're able to test this too, that would be great. Hopefully have a new release out soon. Just need to run

alter extension pg_partman update to '5.2.3';

with the update file in place where the PG instance can see it. Please don't run this on production, though, since this may not be the final 5.2.3 release version.

@keithf4 keithf4 added this to the Next Patch milestone Dec 19, 2024
@keithf4
Copy link
Collaborator

keithf4 commented Dec 20, 2024

Glad I did some unit testing first. So that query lookup only works properly if the schema of the partition set is in the search path. Adjusted things to just use OIDs more and get the actual child index name from pg_class instead of doing a ::regclass cast.

Should be working better now.

@mor-lehr-deel
Copy link
Author

Thanks a lot for the quick fix!
Unfortunately we are using AWS RDS and so we depend on official releases and making sure AWS adds support for them - the latest supported version is 5.1.0.
I inspected the release notes for 5.2 and the actual change in 5.2.2->5.2.3 and based on our usage it seemed like the rest of the changes are not relevant for us so I manually re-created the inherit_replica_identity function from commit 41bc480.
Now run_maintenance_proc() seems to be working properly 👍 , I will continue to test.

Understand that you may have had the replica identities handled already, but everyone else may not so I did want to try and handle it in this extension for now

I was not suggesting that pg_partman shouldn't handle it - and it's a great addition. apologies if it sounded like that.

Honestly, I believe that it not being inherited in core is a bug, so hopefully they'll fix that upstream at some point.

Totally agree, I hope one day we will have interval partitioning (Oracle term) as part of core pg... maybe they can incorporate some of your work...

@mor-lehr-deel mor-lehr-deel changed the title Possible bug with replica identity inheritance in 5.1 Bug with replica identity USING INDEX inheritance in 5.1 Dec 22, 2024
@keithf4
Copy link
Collaborator

keithf4 commented Dec 31, 2024

I've released version 5.2.3. Not sure how long it takes RDS to get patch releases out. If this issue is still a problem when you have a chance to test that version in production, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants