relcache not invalidated when ADD PRIMARY KEY USING INDEX

Started by houzj.fnst@fujitsu.comabout 4 years ago4 messages
#1houzj.fnst@fujitsu.com
houzj.fnst@fujitsu.com
1 attachment(s)

Hi,

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.

This would cause an error When REPLICA IDENTITY is default and we are
UPDATE/DELETE a published table , because we will refer to
RelationData::rd_pkindex to check if the UPDATE or DELETE can be safety
executed in this case.

---reproduction steps
CREATE TABLE test(a int not null, b int);
CREATE UNIQUE INDEX a ON test(a);
CREATE PUBLICATION PUB for TABLE test;
UPDATE test SET a = 2;
ERROR: cannot update table "test" because it does not have a replica identity and publishes updates
HINT: To enable updating the table, set REPLICA IDENTITY using ALTER TABLE.

alter table test add primary key using index a;
UPDATE test SET a = 2;
ERROR: cannot update table "test" because it does not have a replica identity and publishes updates
HINT: To enable updating the table, set REPLICA IDENTITY using ALTER TABLE.
---

I think the bug exists in HEAD ~ PG11 after the commit(f66e8bf) which remove
relhaspkey from pg_class. In PG10, when adding a primary key, it will always
update the relhaspkey in pg_class which will invalidate the relcache, so it
was OK.

I tried to write a patch to fix this by invalidating the relcache after marking
primary key in index_constraint_create().

Best regards,
Hou zj

Attachments:

0001-Invalid-relcache-when-ADD-PRIMARY-KEY-USING-INDEX.patchapplication/octet-stream; name=0001-Invalid-relcache-when-ADD-PRIMARY-KEY-USING-INDEX.patchDownload
From eafbc011106f53e57bc989bd62a88bf62c639f12 Mon Sep 17 00:00:00 2001
From: "houzj.fnst" <houzj.fnst@fujitsu.com>
Date: Sun, 19 Dec 2021 18:13:39 +0800
Subject: [PATCH] Invalid relcache when ADD PRIMARY KEY USING INDEX

When adding PRIMARY KEY using an existing unique index on not null columns,
the target table's relcache was not being invalidated. If REPLICA IDENTITY is
default, this leads to disallowing UPDATE/DELETE operations on the publisher
side because the primary key is not found.
---
 contrib/test_decoding/expected/ddl.out | 33 +++++++++++-
 contrib/test_decoding/sql/ddl.sql      | 14 +++++
 src/backend/catalog/index.c            |  9 ++++
 src/test/subscription/t/100_bugs.pl    | 74 +++++++++++++++++++++++++-
 4 files changed, 128 insertions(+), 2 deletions(-)

diff --git a/contrib/test_decoding/expected/ddl.out b/contrib/test_decoding/expected/ddl.out
index 4ff0044c78..fc74cd4bfa 100644
--- a/contrib/test_decoding/expected/ddl.out
+++ b/contrib/test_decoding/expected/ddl.out
@@ -594,6 +594,19 @@ UPDATE table_dropped_index_no_pk SET b = 6, c = 7 WHERE a = 3;
 DELETE FROM table_dropped_index_no_pk WHERE b = 1;
 DELETE FROM table_dropped_index_no_pk WHERE a = 3;
 DROP TABLE table_dropped_index_no_pk;
+-- check table with newly added primary key
+CREATE TABLE table_with_not_null(id int NOT NULL, data int);
+CREATE UNIQUE INDEX table_with_not_null_idx ON table_with_not_null(id);
+INSERT INTO table_with_not_null VALUES(1, 2);
+-- won't log old keys
+UPDATE table_with_not_null SET data = 3 WHERE data = 2;
+UPDATE table_with_not_null SET id = -id;
+UPDATE table_with_not_null SET id = -id;
+-- should log the old pkey
+ALTER TABLE table_with_not_null ADD PRIMARY KEY USING INDEX table_with_not_null_idx;
+UPDATE table_with_not_null SET data = 3 WHERE data = 2;
+UPDATE table_with_not_null SET id = -id;
+UPDATE table_with_not_null SET id = -id;
 -- check toast support
 BEGIN;
 CREATE SEQUENCE toasttable_rand_seq START 79 INCREMENT 1499; -- portable "random"
@@ -751,6 +764,24 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc
  table public.table_dropped_index_no_pk: DELETE: (no-tuple-data)
  COMMIT
  BEGIN
+ table public.table_with_not_null: INSERT: id[integer]:1 data[integer]:2
+ COMMIT
+ BEGIN
+ table public.table_with_not_null: UPDATE: id[integer]:1 data[integer]:3
+ COMMIT
+ BEGIN
+ table public.table_with_not_null: UPDATE: id[integer]:-1 data[integer]:3
+ COMMIT
+ BEGIN
+ table public.table_with_not_null: UPDATE: id[integer]:1 data[integer]:3
+ COMMIT
+ BEGIN
+ table public.table_with_not_null: UPDATE: old-key: id[integer]:1 new-tuple: id[integer]:-1 data[integer]:3
+ COMMIT
+ BEGIN
+ table public.table_with_not_null: UPDATE: old-key: id[integer]:-1 new-tuple: id[integer]:1 data[integer]:3
+ COMMIT
+ BEGIN
  table public.toasttable: INSERT: id[integer]:1 toasted_col1[text]:'12345678910111213141516171819202122232425262728293031323334353637383940414243444546474849505152535455565758596061626364656667686970717273747576777879808182838485868788899091929394959697989910010110210310410510610710810911011111211311411511611711811912012112212312412512612712812913013113213313413513613713813914014114214314414514614714814915015115215315415515615715815916016116216316416516616716816917017117217317417517617717817918018118218318418518618718818919019119219319419519619719819920020120220320420520620720820921021121221321421521621721821922022122222322422522622722822923023123223323423523623723823924024124224324424524624724824925025125225325425525625725825926026126226326426526626726826927027127227327427527627727827928028128228328428528628728828929029129229329429529629729829930030130230330430530630730830931031131231331431531631731831932032132232332432532632732832933033133233333433533633733833934034134234334434534634734834935035135235335435535635735835936036136236336436536636736836937037137237337437537637737837938038138238338438538638738838939039139239339439539639739839940040140240340440540640740840941041141241341441541641741841942042142242342442542642742842943043143243343443543643743843944044144244344444544644744844945045145245345445545645745845946046146246346446546646746846947047147247347447547647747847948048148248348448548648748848949049149249349449549649749849950050150250350450550650750850951051151251351451551651751851952052152252352452552652752852953053153253353453553653753853954054154254354454554654754854955055155255355455555655755855956056156256356456556656756856957057157257357457557657757857958058158258358458558658758858959059159259359459559659759859960060160260360460560660760860961061161261361461561661761861962062162262362462562662762862963063163263363463563663763863964064164264364464564664764864965065165265365465565665765865966066166266366466566666766866967067167267367467567667767867968068168268368468568668768868969069169269369469569669769869970070170270370470570670770870971071171271371471571671771871972072172272372472572672772872973073173273373473573673773873974074174274374474574674774874975075175275375475575675775875976076176276376476576676776876977077177277377477577677777877978078178278378478578678778878979079179279379479579679779879980080180280380480580680780880981081181281381481581681781881982082182282382482582682782882983083183283383483583683783883984084184284384484584684784884985085185285385485585685785885986086186286386486586686786886987087187287387487587687787887988088188288388488588688788888989089189289389489589689789889990090190290390490590690790890991091191291391491591691791891992092192292392492592692792892993093193293393493593693793893994094194294394494594694794894995095195295395495595695795895996096196296396496596696796896997097197297397497597697797897998098198298398498598698798898999099199299399499599699799899910001001100210031004100510061007100810091010101110121013101410151016101710181019102010211022102310241025102610271028102910301031103210331034103510361037103810391040104110421043104410451046104710481049105010511052105310541055105610571058105910601061106210631064106510661067106810691070107110721073107410751076107710781079108010811082108310841085108610871088108910901091109210931094109510961097109810991100110111021103110411051106110711081109111011111112111311141115111611171118111911201121112211231124112511261127112811291130113111321133113411351136113711381139114011411142114311441145114611471148114911501151115211531154115511561157115811591160116111621163116411651166116711681169117011711172117311741175117611771178117911801181118211831184118511861187118811891190119111921193119411951196119711981199120012011202120312041205120612071208120912101211121212131214121512161217121812191220122112221223122412251226122712281229123012311232123312341235123612371238123912401241124212431244124512461247124812491250125112521253125412551256125712581259126012611262126312641265126612671268126912701271127212731274127512761277127812791280128112821283128412851286128712881289129012911292129312941295129612971298129913001301130213031304130513061307130813091310131113121313131413151316131713181319132013211322132313241325132613271328132913301331133213331334133513361337133813391340134113421343134413451346134713481349135013511352135313541355135613571358135913601361136213631364136513661367136813691370137113721373137413751376137713781379138013811382138313841385138613871388138913901391139213931394139513961397139813991400140114021403140414051406140714081409141014111412141314141415141614171418141914201421142214231424142514261427142814291430143114321433143414351436143714381439144014411442144314441445144614471448144914501451145214531454145514561457145814591460146114621463146414651466146714681469147014711472147314741475147614771478147914801481148214831484148514861487148814891490149114921493149414951496149714981499150015011502150315041505150615071508150915101511151215131514151515161517151815191520152115221523152415251526152715281529153015311532153315341535153615371538153915401541154215431544154515461547154815491550155115521553155415551556155715581559156015611562156315641565156615671568156915701571157215731574157515761577157815791580158115821583158415851586158715881589159015911592159315941595159615971598159916001601160216031604160516061607160816091610161116121613161416151616161716181619162016211622162316241625162616271628162916301631163216331634163516361637163816391640164116421643164416451646164716481649165016511652165316541655165616571658165916601661166216631664166516661667166816691670167116721673167416751676167716781679168016811682168316841685168616871688168916901691169216931694169516961697169816991700170117021703170417051706170717081709171017111712171317141715171617171718171917201721172217231724172517261727172817291730173117321733173417351736173717381739174017411742174317441745174617471748174917501751175217531754175517561757175817591760176117621763176417651766176717681769177017711772177317741775177617771778177917801781178217831784178517861787178817891790179117921793179417951796179717981799180018011802180318041805180618071808180918101811181218131814181518161817181818191820182118221823182418251826182718281829183018311832183318341835183618371838183918401841184218431844184518461847184818491850185118521853185418551856185718581859186018611862186318641865186618671868186918701871187218731874187518761877187818791880188118821883188418851886188718881889189018911892189318941895189618971898189919001901190219031904190519061907190819091910191119121913191419151916191719181919192019211922192319241925192619271928192919301931193219331934193519361937193819391940194119421943194419451946194719481949195019511952195319541955195619571958195919601961196219631964196519661967196819691970197119721973197419751976197719781979198019811982198319841985198619871988198919901991199219931994199519961997199819992000' rand1[double precision]:79 toasted_col2[text]:null rand2[double precision]:1578
  COMMIT
  BEGIN
@@ -759,7 +790,7 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc
  BEGIN
  table public.toasttable: UPDATE: id[integer]:1 toasted_col1[text]:'12345678910111213141516171819202122232425262728293031323334353637383940414243444546474849505152535455565758596061626364656667686970717273747576777879808182838485868788899091929394959697989910010110210310410510610710810911011111211311411511611711811912012112212312412512612712812913013113213313413513613713813914014114214314414514614714814915015115215315415515615715815916016116216316416516616716816917017117217317417517617717817918018118218318418518618718818919019119219319419519619719819920020120220320420520620720820921021121221321421521621721821922022122222322422522622722822923023123223323423523623723823924024124224324424524624724824925025125225325425525625725825926026126226326426526626726826927027127227327427527627727827928028128228328428528628728828929029129229329429529629729829930030130230330430530630730830931031131231331431531631731831932032132232332432532632732832933033133233333433533633733833934034134234334434534634734834935035135235335435535635735835936036136236336436536636736836937037137237337437537637737837938038138238338438538638738838939039139239339439539639739839940040140240340440540640740840941041141241341441541641741841942042142242342442542642742842943043143243343443543643743843944044144244344444544644744844945045145245345445545645745845946046146246346446546646746846947047147247347447547647747847948048148248348448548648748848949049149249349449549649749849950050150250350450550650750850951051151251351451551651751851952052152252352452552652752852953053153253353453553653753853954054154254354454554654754854955055155255355455555655755855956056156256356456556656756856957057157257357457557657757857958058158258358458558658758858959059159259359459559659759859960060160260360460560660760860961061161261361461561661761861962062162262362462562662762862963063163263363463563663763863964064164264364464564664764864965065165265365465565665765865966066166266366466566666766866967067167267367467567667767867968068168268368468568668768868969069169269369469569669769869970070170270370470570670770870971071171271371471571671771871972072172272372472572672772872973073173273373473573673773873974074174274374474574674774874975075175275375475575675775875976076176276376476576676776876977077177277377477577677777877978078178278378478578678778878979079179279379479579679779879980080180280380480580680780880981081181281381481581681781881982082182282382482582682782882983083183283383483583683783883984084184284384484584684784884985085185285385485585685785885986086186286386486586686786886987087187287387487587687787887988088188288388488588688788888989089189289389489589689789889990090190290390490590690790890991091191291391491591691791891992092192292392492592692792892993093193293393493593693793893994094194294394494594694794894995095195295395495595695795895996096196296396496596696796896997097197297397497597697797897998098198298398498598698798898999099199299399499599699799899910001001100210031004100510061007100810091010101110121013101410151016101710181019102010211022102310241025102610271028102910301031103210331034103510361037103810391040104110421043104410451046104710481049105010511052105310541055105610571058105910601061106210631064106510661067106810691070107110721073107410751076107710781079108010811082108310841085108610871088108910901091109210931094109510961097109810991100110111021103110411051106110711081109111011111112111311141115111611171118111911201121112211231124112511261127112811291130113111321133113411351136113711381139114011411142114311441145114611471148114911501151115211531154115511561157115811591160116111621163116411651166116711681169117011711172117311741175117611771178117911801181118211831184118511861187118811891190119111921193119411951196119711981199120012011202120312041205120612071208120912101211121212131214121512161217121812191220122112221223122412251226122712281229123012311232123312341235123612371238123912401241124212431244124512461247124812491250125112521253125412551256125712581259126012611262126312641265126612671268126912701271127212731274127512761277127812791280128112821283128412851286128712881289129012911292129312941295129612971298129913001301130213031304130513061307130813091310131113121313131413151316131713181319132013211322132313241325132613271328132913301331133213331334133513361337133813391340134113421343134413451346134713481349135013511352135313541355135613571358135913601361136213631364136513661367136813691370137113721373137413751376137713781379138013811382138313841385138613871388138913901391139213931394139513961397139813991400140114021403140414051406140714081409141014111412141314141415141614171418141914201421142214231424142514261427142814291430143114321433143414351436143714381439144014411442144314441445144614471448144914501451145214531454145514561457145814591460146114621463146414651466146714681469147014711472147314741475147614771478147914801481148214831484148514861487148814891490149114921493149414951496149714981499150015011502150315041505150615071508150915101511151215131514151515161517151815191520152115221523152415251526152715281529153015311532153315341535153615371538153915401541154215431544154515461547154815491550155115521553155415551556155715581559156015611562156315641565156615671568156915701571157215731574157515761577157815791580158115821583158415851586158715881589159015911592159315941595159615971598159916001601160216031604160516061607160816091610161116121613161416151616161716181619162016211622162316241625162616271628162916301631163216331634163516361637163816391640164116421643164416451646164716481649165016511652165316541655165616571658165916601661166216631664166516661667166816691670167116721673167416751676167716781679168016811682168316841685168616871688168916901691169216931694169516961697169816991700170117021703170417051706170717081709171017111712171317141715171617171718171917201721172217231724172517261727172817291730173117321733173417351736173717381739174017411742174317441745174617471748174917501751175217531754175517561757175817591760176117621763176417651766176717681769177017711772177317741775177617771778177917801781178217831784178517861787178817891790179117921793179417951796179717981799180018011802180318041805180618071808180918101811181218131814181518161817181818191820182118221823182418251826182718281829183018311832183318341835183618371838183918401841184218431844184518461847184818491850185118521853185418551856185718581859186018611862186318641865186618671868186918701871187218731874187518761877187818791880188118821883188418851886188718881889189018911892189318941895189618971898189919001901190219031904190519061907190819091910191119121913191419151916191719181919192019211922192319241925192619271928192919301931193219331934193519361937193819391940194119421943194419451946194719481949195019511952195319541955195619571958195919601961196219631964196519661967196819691970197119721973197419751976197719781979198019811982198319841985198619871988198919901991199219931994199519961997199819992000' rand1[double precision]:79 toasted_col2[text]:null rand2[double precision]:1578
  COMMIT
-(143 rows)
+(161 rows)
 
 INSERT INTO toasttable(toasted_col1) SELECT string_agg(g.i::text, '') FROM generate_series(1, 2000) g(i);
 -- update of second column, first column unchanged
diff --git a/contrib/test_decoding/sql/ddl.sql b/contrib/test_decoding/sql/ddl.sql
index 1b3866d015..d19103115a 100644
--- a/contrib/test_decoding/sql/ddl.sql
+++ b/contrib/test_decoding/sql/ddl.sql
@@ -376,6 +376,20 @@ DELETE FROM table_dropped_index_no_pk WHERE b = 1;
 DELETE FROM table_dropped_index_no_pk WHERE a = 3;
 DROP TABLE table_dropped_index_no_pk;
 
+-- check tables with newly added primary key
+CREATE TABLE table_with_not_null(id int NOT NULL, data int);
+CREATE UNIQUE INDEX table_with_not_null_idx ON table_with_not_null(id);
+INSERT INTO table_with_not_null VALUES(1, 2);
+-- won't log old keys
+UPDATE table_with_not_null SET data = 3 WHERE data = 2;
+UPDATE table_with_not_null SET id = -id;
+UPDATE table_with_not_null SET id = -id;
+-- should log the old pkey
+ALTER TABLE table_with_not_null ADD PRIMARY KEY USING INDEX table_with_not_null_idx;
+UPDATE table_with_not_null SET data = 3 WHERE data = 2;
+UPDATE table_with_not_null SET id = -id;
+UPDATE table_with_not_null SET id = -id;
+
 -- check toast support
 BEGIN;
 CREATE SEQUENCE toasttable_rand_seq START 79 INCREMENT 1499; -- portable "random"
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 1757cd3446..f159453cf2 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2052,6 +2052,7 @@ index_constraint_create(Relation heapRelation,
 		HeapTuple	indexTuple;
 		Form_pg_index indexForm;
 		bool		dirty = false;
+		bool		primary_marked = false;
 
 		pg_index = table_open(IndexRelationId, RowExclusiveLock);
 
@@ -2065,6 +2066,7 @@ index_constraint_create(Relation heapRelation,
 		{
 			indexForm->indisprimary = true;
 			dirty = true;
+			primary_marked = true;
 		}
 
 		if (deferrable && indexForm->indimmediate)
@@ -2079,6 +2081,13 @@ index_constraint_create(Relation heapRelation,
 
 			InvokeObjectPostAlterHookArg(IndexRelationId, indexRelationId, 0,
 										 InvalidOid, is_internal);
+
+			/*
+			 * Invalidate the relcache for the table, so that after we commit
+			 * all sessions will refresh the table's primary key.
+			 */
+			if (primary_marked)
+				CacheInvalidateRelcache(heapRelation);
 		}
 
 		heap_freetuple(indexTuple);
diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 6574e500ff..d002db067b 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -6,7 +6,7 @@ use strict;
 use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 7;
+use Test::More tests => 9;
 
 # Bug #15114
 
@@ -306,3 +306,75 @@ is( $node_subscriber->safe_psql(
 
 $node_publisher->stop('fast');
 $node_subscriber->stop('fast');
+
+# Website
+
+# When adding PRIMARY KEY using an existing unique index on not null columns,
+# the target table's relcache was not being invalidated. If REPLICA IDENTITY
+# is default, this leads to disallowing UPDATE/DELETE operations on the
+# publisher side because the primary key is not found.
+
+$node_publisher = PostgreSQL::Test::Cluster->new('publisher4');
+$node_publisher->init(allows_streaming => 'logical');
+$node_publisher->start;
+
+$node_subscriber = PostgreSQL::Test::Cluster->new('subscriber4');
+$node_subscriber->init(allows_streaming => 'logical');
+$node_subscriber->start;
+
+$node_publisher->safe_psql('postgres',
+	"CREATE TABLE tab_replidentity_default(a int NOT NULL, b int)");
+$node_publisher->safe_psql('postgres',
+	"CREATE UNIQUE INDEX idx_replidentity_index_a ON tab_replidentity_default(a)"
+);
+
+$node_publisher->safe_psql('postgres',
+	"INSERT INTO tab_replidentity_default VALUES(1, 1),(2, 2)");
+
+$node_subscriber->safe_psql('postgres',
+	"CREATE TABLE tab_replidentity_default(a int NOT NULL, b int)");
+$node_subscriber->safe_psql('postgres',
+	"CREATE UNIQUE INDEX idx_replidentity_index_a ON tab_replidentity_default(a)"
+);
+
+$publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
+$node_publisher->safe_psql('postgres',
+	"CREATE PUBLICATION tap_pub FOR TABLE tab_replidentity_default");
+$node_subscriber->safe_psql('postgres',
+	"CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr' PUBLICATION tap_pub"
+);
+
+$node_publisher->wait_for_catchup('tap_sub');
+
+# Also wait for initial table sync to finish
+$node_subscriber->poll_query_until('postgres', $synced_query)
+  or die "Timed out while waiting for subscriber to synchronize data";
+
+is( $node_subscriber->safe_psql(
+		'postgres', "SELECT * FROM tab_replidentity_default"),
+	qq(1|1
+2|2),
+	"check initial data on subscriber");
+
+# Create primary key using idx_replidentity_index_a on subscriber because it
+# reflects the future scenario we are testing: ADD PRIMARY KEY USING INDEX.
+$node_subscriber->safe_psql('postgres',
+	"ALTER TABLE tab_replidentity_default ADD PRIMARY KEY USING INDEX idx_replidentity_index_a"
+);
+
+# Create primary key using idx_replidentity_index_a on publisher, then run UPDATE and DELETE.
+$node_publisher->safe_psql(
+	'postgres', qq[
+	ALTER TABLE tab_replidentity_default ADD PRIMARY KEY USING INDEX idx_replidentity_index_a;
+	UPDATE tab_replidentity_default SET a = -a WHERE a = 1;
+	DELETE FROM tab_replidentity_default WHERE a = 2;
+]);
+
+$node_publisher->wait_for_catchup('tap_sub');
+is( $node_subscriber->safe_psql(
+		'postgres', "SELECT * FROM tab_replidentity_default"),
+	qq(-1|1),
+	"update works with REPLICA IDENTITY");
+
+$node_publisher->stop('fast');
+$node_subscriber->stop('fast');
-- 
2.18.4

#2Euler Taveira
euler@eulerto.com
In reply to: houzj.fnst@fujitsu.com (#1)
Re: relcache not invalidated when ADD PRIMARY KEY USING INDEX

On Mon, Dec 20, 2021, at 3:45 AM, houzj.fnst@fujitsu.com wrote:

Hi,

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.

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

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?

--
Euler Taveira
EDB https://www.enterprisedb.com/

#3Julien Rouhaud
rjuju123@gmail.com
In reply to: houzj.fnst@fujitsu.com (#1)
Re: relcache not invalidated when ADD PRIMARY KEY USING INDEX

Hi,

On Mon, Dec 20, 2021 at 06:45:33AM +0000, houzj.fnst@fujitsu.com wrote:

I tried to write a patch to fix this by invalidating the relcache after marking
primary key in index_constraint_create().

The cfbot reports that you have a typo in your regression tests:

https://api.cirrus-ci.com/v1/artifact/task/4806573636714496/regress_diffs/contrib/test_decoding/regression.diffs
diff -U3 /tmp/cirrus-ci-build/contrib/test_decoding/expected/ddl.out /tmp/cirrus-ci-build/contrib/test_decoding/results/ddl.out
--- /tmp/cirrus-ci-build/contrib/test_decoding/expected/ddl.out	2022-01-11 16:46:51.684727957 +0000
+++ /tmp/cirrus-ci-build/contrib/test_decoding/results/ddl.out	2022-01-11 16:50:35.584089784 +0000
@@ -594,7 +594,7 @@
 DELETE FROM table_dropped_index_no_pk WHERE b = 1;
 DELETE FROM table_dropped_index_no_pk WHERE a = 3;
 DROP TABLE table_dropped_index_no_pk;
--- check table with newly added primary key
+-- check tables with newly added primary key

Could you send an updated patch? As far as I can see the rest of the
regression tests succeed so the patch can probably still be reviewed. That
being said, there are pending comments by Euler so I think it's appropriate to
zsh:1: command not found: q

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Euler Taveira (#2)
Re: relcache not invalidated when ADD PRIMARY KEY USING INDEX

"Euler Taveira" <euler@eulerto.com> writes:

On Mon, Dec 20, 2021, at 3:45 AM, houzj.fnst@fujitsu.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