Thread: handling multiple matching constraints in DetachPartitionFinalize()
Hi,
I was looking at the following code in DetachPartitionFinalize():
/* If there's a constraint associated with the index, detach it too */
constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel),
idxid);
constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel),
idxid);
As mentioned in email thread `parenting a PK constraint to a self-FK one`, there may be multiple matching constraints, I think we should call ConstraintSetParentConstraint() for each of them.
This means adding a helper method similar to get_relation_idx_constraint_oid() which finds constraint and calls ConstraintSetParentConstraint().
I am preparing a patch.
Please let me know if my proposal makes sense.
Thanks
On Tue, Aug 23, 2022 at 10:10 AM Zhihong Yu <zyu@yugabyte.com> wrote:
Hi,I was looking at the following code in DetachPartitionFinalize():/* If there's a constraint associated with the index, detach it too */
constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel),
idxid);As mentioned in email thread `parenting a PK constraint to a self-FK one`, there may be multiple matching constraints, I think we should call ConstraintSetParentConstraint() for each of them.This means adding a helper method similar to get_relation_idx_constraint_oid() which finds constraint and calls ConstraintSetParentConstraint().I am preparing a patch.Please let me know if my proposal makes sense.Thanks
This is what I came up with.
Attachment
On 2022-Aug-23, Zhihong Yu wrote: > This is what I came up with. I suggest you provide a set of SQL commands that provoke some wrong behavior with the original code, and show that they generate good behavior after the patch. Otherwise, it's hard to evaluate the usefulness of this. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "Puedes vivir sólo una vez, pero si lo haces bien, una vez es suficiente"
On 2022-Aug-23, Zhihong Yu wrote: > Toggling enable_seqscan on / off using the example from `parenting a PK > constraint to a self-FK one` thread, it can be shown that different > constraint Id would be detached which is incorrect. > However, I am not sure whether toggling enable_seqscan mid-test is > legitimate. Well, let's see it in action. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
On Tue, Aug 23, 2022 at 10:53 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2022-Aug-23, Zhihong Yu wrote:
> This is what I came up with.
I suggest you provide a set of SQL commands that provoke some wrong
behavior with the original code, and show that they generate good
behavior after the patch. Otherwise, it's hard to evaluate the
usefulness of this.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Puedes vivir sólo una vez, pero si lo haces bien, una vez es suficiente"
Toggling enable_seqscan on / off using the example from `parenting a PK constraint to a self-FK one` thread, it can be shown that different constraint Id would be detached which is incorrect.
However, I am not sure whether toggling enable_seqscan mid-test is legitimate.
Cheers