pgpartman / pg_partman

Partition management extension for PostgreSQL
Other
2.04k stars 277 forks source link

Issues with the partition control column being NOT NULL constraint #673

Open gowdh opened 1 month ago

gowdh commented 1 month ago

Hi, we have issue while creating partition when upgrading with latest partman 5.1.0

we create partition for a table dim_vvv_visit partition by atd

create table dim_vvv_visit (
                               dw_vv_skey bigserial,
                               com_id varchar(20),
                               atd timestamp(3),
                               created_time timestamp(3),
                               last_mod_time timestamp(3)
)
partition by range (atd);

We created this schema and enabled the extension, but when we tried to call the create_parent function, we received an error.

select partman.create_parent(
                        p_parent_table :='public.dim_vvv_visit',
                        p_control := 'atd',
                        p_type := 'declarative',
                        p_interval := '3 months',
                        p_premake := 2,
                        p_automatic_maintenance  :='on',
                        p_start_partition  := '2019-06-01 00:00:00'
                        );

The output :

Creating partition on : dim_vsl_visit table, with start date as : 2019-07-01 00:00:00-07
NOTICE:  Got exception:
                    state  : P0001
                    message: Control column given (atd) for parent table (public.dim_vvv_visit) does not exist or must be set to NOT NULL
CONTEXT: PL/pgSQL function partman.create_parent(text,text,text,text,text,integer,text,boolean,text,text[],text,boolean,text) line 95 at RAISE
SQL statement "SELECT partman.create_parent(
                        p_parent_table := v_partition_config.schema_name || '.' || v_partition_config.table_name,
                        p_control := v_partition_config.partition_key,
                        p_type := v_partition_config.partitioning_type,
                        p_interval := v_partition_config.interval_type,
                        p_premake := v_partition_config.additional_partition_count,
                        p_automatic_maintenance  := v_partition_config.automatic_maintenance,
                        p_start_partition  := v_partition_start_date
                        )"

Technically, based on the project that I work on, most of the records come from the source partition column atd will be null when running the job incrementally, and then it gets updated with values. So we can't set the partition column to not null constraint as the table won't accept null values.

keithf4 commented 1 month ago

From the beginning I've had partman's requirement that the control column be not NULL. At the time is was because I saw no use-case where that would be useful (you're example here could be valid, tho). However when native partitioning came out, this turned out to provide a means of avoiding a huge foot-gun with partitioning so left it in: The only location NULL values can go is the default partition. The problem with the default partition having data in it is that every time a new child table is added, the entire default partition must be fully scanned to see if any of the data it contains matches the new child table's constraints. This can cause maintenance to take a very long time depending how much data that is. And depending on the locks taken during maintenance, this can potentially fully lock up the partition set until it's finished.

My recommendation would be to have a default value of some "invalid" time in the past. I've frequently seen people use the base epoch value (Jan 1 1970) but depending on your data that may not work. Perhaps some other date even further back in time. You can then have a child table for just those values and avoid the performance hit on data existing in the default.

Either way in the end, whether it went to the default or this special child table, your updates that set the proper value would still be having to move the data from one child table to another, so the performance impact for writes would still be the same in the end.

gowdh commented 1 month ago

Hi Keith,

The older version of Partman 4.4.0 worked even with null values in the partition column, but the version after the upgrade didn't work. Is this behavioral change introduced recently? and if that old behavior can be achieved by some means.

gowdh commented 1 month ago

Hi @keithf4, any updates on this?

keithf4 commented 1 month ago

I went back and looked at version 4.4.0 and older of pg_partman. Just looking back at commit history, here's one from version 2.1 where I just re-arranged the code formatting and created the more detailed error message you see now.

https://github.com/pgpartman/pg_partman/commit/c59a5c7e0017cdeead46384bff9f543117b4054d#diff-821369c12eac5bd252682024195911cc93564dfe6a70aaebd0de762d0ab1a262L68

I'm not seeing any bug fixes around this feature since then either. So I'm not sure how you were doing this before. Do you have a working example with 4.4.0 allowing NULLs for the control column? I'd be happy to look into that further.

Also just been discussing having the control column allow nulls and most people I've talked to seem to think that allowing this would be an anti-pattern for good partitioning design, especially with the way PG handles all NULLs going to the default. So at this point I don't believe I'll be removing this requirement.

keithf4 commented 1 month ago

Actually, I think I did find one change as soon as I hit enter on the last update. Was your old partition set trigger-based? It looks like prior to 5.0, the control column not being NULL was not enforced for trigger-based partition sets. Native partitioning has always had that requirement.

gowdh commented 1 month ago

Hi @keithf4, Old partman version 4.4.0 we used is native partitioning not trigger based.

Here is the table we create partition for that,

create table dim_vvv_visit (
                               dw_vv_skey bigserial,
                               com_id varchar(20),
                               atd timestamp(3),
                               created_time timestamp(3),
                               last_mod_time timestamp(3)
)
partition by range (atd);

select partman.create_parent(
                        p_parent_table :='public.dim_vvv_visit',
                        p_control := 'atd',
                        p_type := 'native',
                        p_interval := '3 months',
                        p_premake := 2,
                        p_automatic_maintenance  :='on',
                        p_start_partition  := '2019-06-01 00:00:00'
                        );

Result: It creates partition tables without not null constraints

"parent_schema","parent","child_schema","child" "public","dim_vvv_visit","public","dim_vvv_visit_p2019_06" "public","dim_vvv_visit","public","dim_vvv_visit_p2019_09" "public","dim_vvv_visit","public","dim_vvv_visit_p2019_12" "public","dim_vvv_visit","public","dim_vvv_visit_p2020_03" "public","dim_vvv_visit","public","dim_vvv_visit_p2020_06" "public","dim_vvv_visit","public","dim_vvv_visit_p2020_09" "public","dim_vvv_visit","public","dim_vvv_visit_p2020_12" "public","dim_vvv_visit","public","dim_vvv_visit_p2021_03" "public","dim_vvv_visit","public","dim_vvv_visit_p2021_06" "public","dim_vvv_visit","public","dim_vvv_visit_p2021_09" "public","dim_vvv_visit","public","dim_vvv_visit_p2021_12" "public","dim_vvv_visit","public","dim_vvv_visit_p2022_03" "public","dim_vvv_visit","public","dim_vvv_visit_p2022_06" "public","dim_vvv_visit","public","dim_vvv_visit_p2022_09" "public","dim_vvv_visit","public","dim_vvv_visit_p2022_12" "public","dim_vvv_visit","public","dim_vvv_visit_p2023_03" "public","dim_vvv_visit","public","dim_vvv_visit_p2023_06" "public","dim_vvv_visit","public","dim_vvv_visit_p2023_09" "public","dim_vvv_visit","public","dim_vvv_visit_p2023_12" "public","dim_vvv_visit","public","dim_vvv_visit_p2024_03" "public","dim_vvv_visit","public","dim_vvv_visit_p2024_06" "public","dim_vvv_visit","public","dim_vvv_visit_p2024_09" "public","dim_vvv_visit","public","dim_vvv_visit_p2024_12" "public","dim_vvv_visit","public","dim_vvv_visit_default"

keithf4 commented 1 month ago

I see now. That's actually a bug now that I'm looking at it. It wasn't requiring the column to be NOT NULL of it was NOT natively partitioned. So the opposite of what I'd been intending to do this whole time! Apologies on the confusion here.

I'm still really hesitant on reverting this behavior back to allowing NULLs. I've honestly never run into an example until this one that even came close to being a valid use-case of allowing a NULL for the control column. Even in this example, I don't think it was a wise design choice simply due to the nature of how the default table works in PG. Is some part of the application actually expecting this column to be NULL or is it simply because the source isn't providing a value that you need it to be NULL? If not the latter, just set a default value for the column in PG (like that base epoch value i gave an example of before). If it is the latter, perhaps just revisit that portion to look for a lower than normal timestamp value instead of NULL.

gowdh commented 1 month ago

Hi @keithf4,

As i mentioned earlier, some of the source records we get are expected to be NULL for control column and we require those records in the application. The records moves to default table for null values and when the values gets updated incrementally, records get moves to specific partition table. Also we do require lots of changes in the application for setting default value for the control column.

Even we set default value '1970-01-01' for null value, the retention period we normally set is '5 years', so the records having value '1970-01-01' would gets dropped when we run maintenance procedure.

keithf4 commented 1 month ago

Ok. I'll see about adding a config option to allow nulls when I return to development

gowdh commented 1 month ago

Thanks @keithf4 for considering this, May i know when will be release date for the next version?

keithf4 commented 1 month ago

I'm sorry but I don't have a firm ETA at this time. I did set the milestone for 5.2 to the end of Oct because I'll be pretty busy for the next few months. If I have time before then I will try and see what I can do.