Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-13 Thread Michael Paquier
On Thu, Jul 13, 2023 at 02:01:49PM +0530, Shruthi Gowda wrote: > While analyzing the issue I did notice that validatePartitionedIndex() is > the only place where the index tuple was copied from rel->rd_indextuple > however was not clear about the motive behind it. No idea either. Anyway, I've

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-13 Thread Shruthi Gowda
On Thu, Jul 13, 2023 at 1:40 PM Michael Paquier wrote: > On Thu, Jul 13, 2023 at 02:26:42PM +0900, Michael Paquier wrote: > > On Thu, Jul 13, 2023 at 09:35:17AM +0530, Dilip Kumar wrote: > >> Or do we actually need to update all the tuple header information as > >> well in

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-13 Thread Michael Paquier
On Thu, Jul 13, 2023 at 02:26:42PM +0900, Michael Paquier wrote: > On Thu, Jul 13, 2023 at 09:35:17AM +0530, Dilip Kumar wrote: >> Or do we actually need to update all the tuple header information as >> well in RelationReloadIndexInfo() in order to fix that invariant so >> that it can be used for

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-12 Thread Michael Paquier
On Thu, Jul 13, 2023 at 09:35:17AM +0530, Dilip Kumar wrote: > Yeah, It seems that using pg_index tuples from relcache is not safe, > at least for updating the catalog tuples. However, are there known > rules or do we need to add some comments saying that the pg_index > tuple from the relcache

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-12 Thread Dilip Kumar
On Thu, Jul 13, 2023 at 3:56 AM Michael Paquier wrote: > > On Wed, Jul 12, 2023 at 04:02:23PM -0400, Robert Haas wrote: > > Oh, interesting. The fact that indisreplident isn't copied seems like > > a pretty clear mistake, but I'm guessing that the fact that t_self > > wasn't refreshed was

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-12 Thread Michael Paquier
On Wed, Jul 12, 2023 at 10:01:49AM -0400, Robert Haas wrote: > I'm not sure exactly what is happening here, but it looks to me like > ATExecReplicaIdentity() only takes ShareLock on the index and > nevertheless feels entitled to update the pg_index tuple, which is > pretty strange. We normally

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-12 Thread Michael Paquier
On Wed, Jul 12, 2023 at 04:02:23PM -0400, Robert Haas wrote: > Oh, interesting. The fact that indisreplident isn't copied seems like > a pretty clear mistake, but I'm guessing that the fact that t_self > wasn't refreshed was deliberate and that the author of this code > didn't really intend for

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-12 Thread Robert Haas
On Wed, Jul 12, 2023 at 12:28 PM Shruthi Gowda wrote: > I reviewed the function RelationReloadIndexInfo() and observed that the > 'indisreplident' field and the SelfItemPointer 't_self' are not refreshed to > the pg_index tuple of the index. > Attached is the patch that fixes the above

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-12 Thread Shruthi Gowda
On Wed, Jul 12, 2023 at 5:46 PM Dilip Kumar wrote: > On Wed, Jul 12, 2023 at 1:56 PM Michael Paquier > wrote: > > > > On Wed, Jul 12, 2023 at 11:38:05AM +0530, Dilip Kumar wrote: > > > On Wed, Jul 12, 2023 at 11:12 AM Michael Paquier > wrote: > > >> > > >> On Wed, Jul 12, 2023 at 09:38:41AM

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-12 Thread Robert Haas
On Wed, Jul 12, 2023 at 4:26 AM Michael Paquier wrote: > Taking the test case of upthread, from what I can see, the ALTER TABLE > .. REPLICA IDENTITY registers two relcache invalidations for pk_foo > (via RegisterRelcacheInvalidation), which is the relcache entry whose > stuff is messed up. I

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-12 Thread Robert Haas
On Tue, Jul 11, 2023 at 1:22 PM Shruthi Gowda wrote: > BEGIN; > > CREATE TABLE foo ( > id INT NOT NULL, > ts TIMESTAMP WITH TIME ZONE NOT NULL > ) PARTITION BY RANGE (ts); > > CREATE TABLE foo_2023 ( > id INT NOT NULL, > ts TIMESTAMP WITH TIME ZONE NOT NULL > ); > > ALTER TABLE ONLY foo

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-12 Thread Dilip Kumar
On Wed, Jul 12, 2023 at 1:56 PM Michael Paquier wrote: > > On Wed, Jul 12, 2023 at 11:38:05AM +0530, Dilip Kumar wrote: > > On Wed, Jul 12, 2023 at 11:12 AM Michael Paquier > > wrote: > >> > >> On Wed, Jul 12, 2023 at 09:38:41AM +0900, Michael Paquier wrote: > >> > While working recently on

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-12 Thread Michael Paquier
On Wed, Jul 12, 2023 at 11:38:05AM +0530, Dilip Kumar wrote: > On Wed, Jul 12, 2023 at 11:12 AM Michael Paquier wrote: >> >> On Wed, Jul 12, 2023 at 09:38:41AM +0900, Michael Paquier wrote: >> > While working recently on what has led to cfc43ae and fc55c7f, I >> > really got the feeling that

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-12 Thread Dilip Kumar
On Wed, Jul 12, 2023 at 11:12 AM Michael Paquier wrote: > > On Wed, Jul 12, 2023 at 09:38:41AM +0900, Michael Paquier wrote: > > While working recently on what has led to cfc43ae and fc55c7f, I > > really got the feeling that there could be some command sequences that > > lacked some CCIs (or

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-11 Thread Michael Paquier
On Wed, Jul 12, 2023 at 09:38:41AM +0900, Michael Paquier wrote: > While working recently on what has led to cfc43ae and fc55c7f, I > really got the feeling that there could be some command sequences that > lacked some CCIs (or CommandCounterIncrement calls) to make sure that > the catalog updates

Re: 'ERROR: attempted to update invisible tuple' from 'ALTER INDEX ... ATTACH PARTITION' on parent index

2023-07-11 Thread Michael Paquier
On Tue, Jul 11, 2023 at 10:52:16PM +0530, Shruthi Gowda wrote: > While testing some use cases, I encountered 'ERROR: attempted to update > invisible tuple' when a partitioned index is attached to a parent index > which is also a replica identity index. > Below is the reproducible test case. The