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

From: feichanghong <feichanghong(at)qq(dot)com>
To: alvherre <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: pgsql-bugs <pgsql-bugs(at)lists(dot)postgresql(dot)org>
Subject: Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
Date: 2024-07-16 05:03:57
Message-ID: tencent_C870731821182831BC3B661077727DA0990A@qq.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Hi Alvaro,

> On Jul 16, 2024, at 03:35, alvherre <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> 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 agree with your point of view. There is no need to add an extra partition
constraint for "DETACH PARTITION CONCURRENTLY". In the first transaction, we
merely set inhdetachpending to true without actually dropping the existing
partition constraint.
At the start of the second transaction, we wait for all queries that were
planned with the partition to finish. Then, we acquire an AccessExclusiveLock
on the sub-partition and drop the original partition constraint. Most likely,
the newly added constraint is useless for the optimizer.

The only useful scenario I can think of is that, if we were to attach this
sub-partition back to the parent table using the same strategy, it might reduce
the checks for partition constraints.

Best Regards,
Fei Changhong

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Tender Wang 2024-07-16 07:39:12 Re: BUG #18371: There are wrong constraint residues when detach hash partiton concurrently
Previous Message Michael Paquier 2024-07-16 01:00:52 Re: BUG #18540: Does PG16 standby database support function pg_replication_origin_advance?