Feedback-ID: ia2694551:Fastmail
Date: Fri, 20 Sep 2024 09:31:13 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Tender Wang <tndrwang@gmail.com>
Cc: jian he <jian.universality@gmail.com>,
	Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: not null constraints, again
Message-ID: <202409200731.rxqstsmup5nl@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAHewXNm+qmT6V0HJCsjsO9R7EyJwXyz8mbBOY3ES7cR7K3djvA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202409200731.rxqstsmup5nl%40alvherre.pgsql>
Precedence: bulk

On 2024-Sep-20, Tender Wang wrote:

> jian he <jian.universality@gmail.com> 于2024年9月20日周五 11:34写道：
> 
> > another bug.
> > I will dig later, just want to share it first.
> >
> > minimum producer:
> > drop table if exists pp1,cc1, cc2,cc3;
> > create table pp1 (f1 int );
> > create table cc1 () inherits (pp1);
> > create table cc2() inherits(pp1,cc1);
> > create table cc3() inherits(pp1,cc1,cc2);
> >
> > alter table pp1 alter f1 set not null;
> > ERROR:  tuple already updated by self
> 
> I guess some place needs call CommandCounterIncrement().

Yeah ... this fixes it:

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 579b8075b5..3f66e43b9a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7877,12 +7877,6 @@ ATExecSetNotNull(List **wqueue, Relation rel, char *conName, char *colName,
 	{
 		List	   *children;
 
-		/*
-		 * Make previous addition visible, in case we process the same
-		 * relation again while chasing down multiple inheritance trees.
-		 */
-		CommandCounterIncrement();
-
 		children = find_inheritance_children(RelationGetRelid(rel),
 											 lockmode);
 
@@ -7890,6 +7884,8 @@ ATExecSetNotNull(List **wqueue, Relation rel, char *conName, char *colName,
 		{
 			Relation	childrel = table_open(childoid, NoLock);
 
+			CommandCounterIncrement();
+
 			ATExecSetNotNull(wqueue, childrel, conName, colName,
 							 recurse, true, lockmode);
 			table_close(childrel, NoLock);


I was trying to save on the number of CCIs that we perform, but it's
likely not a wise expenditure of time given that this isn't a very
common scenario anyway.  (Nobody with thousands of millions of children
tables will try to run thousands of commands in a single transaction
anyway ... so saving a few increments doesn't make any actual
difference.  If such people exist, they can show us their use case and
we can investigate and fix it then.)

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"This is what I like so much about PostgreSQL.  Most of the surprises
are of the "oh wow!  That's cool" Not the "oh shit!" kind.  :)"
Scott Marlowe, http://archives.postgresql.org/pgsql-admin/2008-10/msg00152.php