Re: relcache not invalidated when ADD PRIMARY KEY USING INDEX

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Euler Taveira" <euler(at)eulerto(dot)com>
Cc: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: relcache not invalidated when ADD PRIMARY KEY USING INDEX
Date: 2022-01-22 18:38:04
Message-ID: 1476151.1642876684@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"Euler Taveira" <euler(at)eulerto(dot)com> writes:
> On Mon, Dec 20, 2021, at 3:45 AM, houzj(dot)fnst(at)fujitsu(dot)com wrote:
>> When reviewing some replica identity related patches, I found that when adding
>> primary key using an existing unique index on not null columns, the
>> target table's relcache won't be invalidated.

> Good catch.

Indeed.

> It seems you can simplify your checking indexForm->indisprimary directly, no?

The logic seems fine to me --- it avoids an unnecessary cache flush
if the index was already the pkey. (Whether we actually reach this
code in such a case, I dunno, but it's not costing much to be smart
if we are.)

> Why did you add new tests for test_decoding? I think the TAP tests alone are
> fine. BTW, this test is similar to publisher3/subscriber3. Isn't it better to
> use the same pub/sub to reduce the test execution time?

I agree, the proposed test cases are expensive overkill. The repro
shown in the original message is sufficient and much cheaper.
Moreover, we already have some test cases very much like that in
regress/sql/publication.sql, so that's where I put it.

Pushed with some minor cosmetic adjustments.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2022-01-22 18:52:41 Re: XLogReadRecord() error in XlogReadTwoPhaseData()
Previous Message Justin Pryzby 2022-01-22 18:37:49 pg_upgrade/test.sh and v9.5