pg_partman icon indicating copy to clipboard operation
pg_partman copied to clipboard

Review FK handling - (Renamed from: drop_cascade_fk is always ignored)

Open lorenzo opened this issue 3 years ago • 13 comments

I'm having an issue where I want partitions to the deleted with CASCADE, but cannot find out how to do it.

While inspecting the function, I noticed that v_drop_cascade_fk is never assigned, and therefore always evaluates to false:

https://github.com/pgpartman/pg_partman/blob/master/sql/functions/drop_partition_time.sql#L15

(You can search for the variable name and see that it is never set to any value)

lorenzo avatar Feb 18 '22 08:02 lorenzo

Thank you for this fix. That was definitely an oversight, but just to let you know this will not work with native partitioning. See Issue #365. When you cascade removal of FK's to a partition set with native partitioning, it completely removes the FK on ALL child tables in the partition set which is likely undesired. The option is only left there for non-native partitioning. See the Changelog entries for version 4.6.0 and the documentation entry for this option - https://github.com/pgpartman/pg_partman/blob/master/doc/pg_partman.md#tables

keithf4 avatar Feb 18 '22 14:02 keithf4

Interesting, thanks for the heads up.

Does this mean that I cannot have foreign keys to the partitioned tables while using retention?

lorenzo avatar Feb 18 '22 15:02 lorenzo

That is potentially an issue yes. If the FK is created through the parent table and you cascade delete the FK on a child table, it deletes it for the entire partition set.

Now that I'm thinking about this, I could see about turning the template FK inheritance back on again for native partitioning in versions of PG above 10. I'd disabled it since FK inheritance was included in 11+, but now seeing how it can interfere with retention, it could still be a useful template feature. Since then the FK would not be tied to the parent and could be removed individually.

Would need to very carefully document this and would also need to ensure it is not enabled by default.

https://github.com/pgpartman/pg_partman/blob/master/sql/functions/inherit_template_properties.sql#L169

keithf4 avatar Feb 18 '22 16:02 keithf4

Also noting that I would not add this feature back in until PG10 support is dropped later this year along with trigger-based partitioning potentially. Since then I could repurpose this feature much more clearly and not have it be confused with the old non-native uses.

keithf4 avatar Feb 18 '22 16:02 keithf4

That makes sense. I'll look forward to the re-introduciton of that feature and workaround this limitation in the meantime.

Thanks for your help!!

lorenzo avatar Feb 18 '22 17:02 lorenzo

Is it just not currently possible to mix retention and FKs to a partitioned table without a delete, detach and drop?

calebj avatar Feb 24 '22 16:02 calebj

If you try and detach a child table that has values referenced by another table's FK, that detach operation fails. See https://github.com/pgpartman/pg_partman/issues/294 That worked for that PR because the CASCADE clause was included and I did not know at the time that doing so would remove the FK reference for the entire partition set.

Trying to delete the values first before the detachment could potentially be a very expensive operation for large partition sets and kind of the defeats one of the reasons for partitioning in the first place to avoid that. Yes, that could potentially solve this, but would not want to have that be part of normal maintenance in pg_partman.

So for now if you want to mix retention and FKs that are managed by the parent table, then yes, you're going to have to have some sort of process that removes the referring rows first. I'm going to see if adding the FKs directly to the child tables via the template table is a possible workaround for this.

keithf4 avatar Feb 24 '22 18:02 keithf4

I'm going to see if adding the FKs directly to the child tables via the template table is a possible workaround for this.

I tried that, and the foreign key on the template table was blocking direct drops for the same reason. It wouldn't block a detach, though, because obviously the template table is empty.

So for now if you want to mix retention and FKs that are managed by the parent table, then yes, you're going to have to have some sort of process that removes the referring rows first.

How much work would it be to make it work like this if the table with the foreign key constraint is also partitioned, and has the same interval? I think partman could safely drop the dependent partition first, iff the ranges align, and the constraint is ON DELETE CASCADE on a NOT NULL column, or is a MATCH FULL made up of at least one NOT NULL column.

I have two such tables like this already, but the table that would have the constraint has a shorter retention setting. Other planned tables would have the same retention settings, so dropping their old partition first would sidestep the FK check on detach.

calebj avatar Feb 24 '22 18:02 calebj

I'm going to see if adding the FKs directly to the child tables via the template table is a possible workaround for this.

I tried that, and the foreign key on the template table was blocking direct drops for the same reason. It wouldn't block a detach, though, because obviously the template table is empty.

Yes, but did you do a DROP CASCADE? That is the entire purposed of the CASCADE with the drop to handle this. I believe the actual problem is that the FK was added through the parent table which then automatically adds it to the children and is somehow linked together then. When that is done, the CASCADE removed the constraint from the entire partition set. My theory is that if the constraint is added directly to the child, you can then safely use the CASCADE option with the DROP command and not remove it for the entire partition set.

So for now if you want to mix retention and FKs that are managed by the parent table, then yes, you're going to have to have some sort of process that removes the referring rows first.

How much work would it be to make it work like this if the table with the foreign key constraint is also partitioned, and has the same interval? I think partman could safely drop the dependent partition first, iff the ranges align, and the constraint is ON DELETE CASCADE on a NOT NULL column, or is a MATCH FULL made up of at least one NOT NULL column.

I'm not sure this would be easy to do in a general case. The above solution, if it works, would be a lot simpler. Ideally a patch in core to not drop the partition for the entire partition set when only one child table is dropped would be the best fix.

I have two such tables like this already, but the table that would have the constraint has a shorter retention setting. Other planned tables would have the same retention settings, so dropping their old partition first would sidestep the FK check on detach.

If you have something set up to remove the dependent data from the referencing table, then there shouldn't be any issues dropping it anymore. So yes, doing this with partitions on the source table to remove it there first before it tries to drop it on the referenced table would be a solution. Sounds like that is working for you already if I'm understanding?

keithf4 avatar Feb 24 '22 21:02 keithf4

Yes, but did you do a DROP CASCADE?

Of course DROP CASCADE works, I didn't mention it because it already came up. I'm looking for something that mimics the behavior of ON DELETE CASCADE, and postgres simply doesn't want to use that when dropping or deleting a child partition without also dropping the entire constraint in question. Perhaps it's for the reason you mentioned, deleting a massive number of rows, but isn't that also a possibility for low-cardinality reference tables? Probably something for the mailing list.

I believe the actual problem is that the FK was added through the parent table which then automatically adds it to the children and is somehow linked together then. [...]

I don't know what this is a response to. The scenario I'm having a problem with is foreign keys to a partitioned table, not foreign keys added to a partitioned table. I have no problems with FKs from a partitioned table to standard tables.

My theory is that if the constraint is added directly to the child, you can then safely use the CASCADE option with the DROP command and not remove it for the entire partition set.

Assuming I'm interpreting this correctly, yes, for FKs between aligned partitions in different tables this would work. But dropping both at the same time would be the only way to preserve the referential integrity. I attempted creating multiple foreign keys from a standard table to several children, and it didn't validate since postgres expects all constraints to be valid simultaneously. Stacking CHECK constraints with the referenced partition ranges would work if other tables could be referenced safely, but the documentation says it isn't safe.

If you have something set up to remove the dependent data from the referencing table, then there shouldn't be any issues dropping it anymore. So yes, doing this with partitions on the source table to remove it there first before it tries to drop it on the referenced table would be a solution. Sounds like that is working for you already if I'm understanding? Not using a foreign key. I just have the two tables I mentioned, and the second table has a much shorter retention. Since the rows in this table come from processing the data in the other one, and the only "deletes" are from partitions aging off, I know that the keys should always be valid. I came to this thread looking for a way to have the DDL enforce it.

It does work when I switch it to detach-only, but for the case of tables that I want to keep the same amount of history on, it would become indeterminate because the order in which partman performs maintenance doesn't take FK dependency into account. If the old behavior of detach-then-drop was still used, and I always kept at least one extra partition on the referenced table around as a safety margin, that would work too.

calebj avatar Feb 24 '22 22:02 calebj

Apologies I am conflating two different FK issues here.

Either way, I will be re-evaluating the handling of FKs in an upcoming release. I will definitely keep your ideas in mind so we try to handling all use-cases properly. Thanks!

keithf4 avatar Feb 25 '22 20:02 keithf4