Feedback-ID: ia2694551:Fastmail
Date: Mon, 15 Jul 2024 21:35:13 +0200
From: alvherre <alvherre@alvh.no-ip.org>
To: feichanghong <feichanghong@qq.com>
Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #18371: There are wrong constraint residues when detach hash
 partiton concurrently
Message-ID: <202407151935.jzia4tm6j3kj@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <tencent_4DA59E77900C87CEBA0F0213000E64040D07@qq.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202407151935.jzia4tm6j3kj%40alvherre.pgsql>
Precedence: bulk

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.


[1] https://postgr.es/m/20200803234854.GA24158@alvherre.pgsql


-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/