Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently

From: Tender Wang <tndrwang(at)gmail(dot)com>
To: alvherre <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: feichanghong <feichanghong(at)qq(dot)com>, pgsql-bugs <pgsql-bugs(at)lists(dot)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>
Subject: Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
Date: 2024-11-19 09:57:09
Message-ID: CAHewXNkbnLQRPmNSWsVW9Z+dwnJXiMLSkS6mTmFMk_yRC-FBjA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

alvherre <alvherre(at)alvh(dot)no-ip(dot)org> 于2024年7月16日周二 03:35写道:

> On 2024-Feb-29, feichanghong wrote:
>
> > &gt; This can work normally on range partitions. However, the constraint
> on hash
> > &gt; partitions uses satisfies_hash_partition with the OID of the parent
> table, and
> > &gt; the newly created constraint does not take effect. For example, in
> the following
> > &gt; case, although there is a t_p1_a_check constraint on t_p1, it is
> still possible
> > &gt; to perform an insert:
> > What I said here is wrong, the constraints on the hash partition will
> also take
> > effect. But this constraint depends on the oid of the parent partition.
>
> We should definitely not have this constraint on hash-partition tables
> after the detach. However, I wonder if instead of adding it and later
> removing it as you propose, it wouldn't be better to just not add it in
> the first place. As a first step, I tried commenting out and found that
> no interesting test fails (only alter_table.sql fails but only because
> the constraint is not there when looking for it specifically.)
>
> The current code does not explain *why* we have to add this constraint,
> and I had forgotten, so I went to look at the first patch submission in
> that thread [1] and saw this comment there:
>
> + /*
> + * Concurrent mode has to work harder; first we add a new constraint
> to the
> + * partition that matches the partition constraint. The reason for
> this is
> + * that the planner may have made optimizations that depend on the
> + * constraint. XXX Isn't it sufficient to invalidate the partition's
> + * relcache entry?
>
>
> I'm trying to figure out whether it's possible that the planner would
> make optimizations based on the hashing function. Quite possibly it
> won't. If that's so, then we should just not make the constraint at
> all, which would make the fix even simpler. I also wonder, maybe that
> XXX comment (which I removed before committing the patch) is right and
> we don't actually need the constraint with _any_ partition strategy, not
> just hash.
>

I pick up this issue again. I skim through the codes and I agree with your
point.
We don't acually need the constraint with any partition strategy, not just
hash.

If we have old queries when we are doing detach, the detach session will be
stopped at
WaitForLockersMultiple(). Other sessions can't insert tuples that violate
partition constraint.
Because the detached partition still has partition constranit.

--
Thanks,
Tender Wang

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message David G. Johnston 2024-11-19 13:07:43 Re: BUG #18715: replace() function silently fails if 3rd argument is null
Previous Message Sandeep Thakkar 2024-11-19 07:01:53 Re: [EXTERNAL] Re: BUG #18707: Installation issue