Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit80ffcb8

Browse files
committed
Improve ALTER PUBLICATION validation and error messages
Attempting to add a system column for a table to an existing publicationwould result in the not very intuitive error message of:ERROR: negative bitmapset member not allowedHere we improve that to have it display the same error message as a userwould see if they tried adding a system column for a table when addingit to the publication in the first place.Doing this requires making the function which validates the list ofcolumns an extern function. The signature of the static function wasn'tan ideal external API as it made the code more complex than it needed to be.Here we adjust the function to have it populate a Bitmapset of attributenumbers. Doing it this way allows code simplification.There was no particular bug here other than the weird error message, sono backpatch.Bug: #18558Reported-by: Alexander Lakhin <exclusion@gmail.com>Author: Peter Smith, David RowleyDiscussion:https://postgr.es/m/18558-411bc81b03592125@postgresql.org
1 parentef6e028 commit80ffcb8

File tree

6 files changed

+66
-80
lines changed

6 files changed

+66
-80
lines changed

‎src/backend/catalog/pg_publication.c

Lines changed: 46 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,6 @@ typedef struct
4848
* table. */
4949
}published_rel;
5050

51-
staticvoidpublication_translate_columns(Relationtargetrel,List*columns,
52-
int*natts,AttrNumber**attrs);
53-
5451
/*
5552
* Check if relation can be in given publication and throws appropriate
5653
* error if not.
@@ -351,6 +348,33 @@ GetTopMostAncestorInPublication(Oid puboid, List *ancestors, int *ancestor_level
351348
returntopmost_relid;
352349
}
353350

351+
/*
352+
* attnumstoint2vector
353+
*Convert a Bitmapset of AttrNumbers into an int2vector.
354+
*
355+
* AttrNumber numbers are 0-based, i.e., not offset by
356+
* FirstLowInvalidHeapAttributeNumber.
357+
*/
358+
staticint2vector*
359+
attnumstoint2vector(Bitmapset*attrs)
360+
{
361+
int2vector*result;
362+
intn=bms_num_members(attrs);
363+
inti=-1;
364+
intj=0;
365+
366+
result=buildint2vector(NULL,n);
367+
368+
while ((i=bms_next_member(attrs,i)) >=0)
369+
{
370+
Assert(i <=PG_INT16_MAX);
371+
372+
result->values[j++]= (int16)i;
373+
}
374+
375+
returnresult;
376+
}
377+
354378
/*
355379
* Insert new publication / relation mapping.
356380
*/
@@ -365,12 +389,12 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri,
365389
Relationtargetrel=pri->relation;
366390
Oidrelid=RelationGetRelid(targetrel);
367391
Oidpubreloid;
392+
Bitmapset*attnums;
368393
Publication*pub=GetPublication(pubid);
369-
AttrNumber*attarray=NULL;
370-
intnatts=0;
371394
ObjectAddressmyself,
372395
referenced;
373396
List*relids=NIL;
397+
inti;
374398

375399
rel=table_open(PublicationRelRelationId,RowExclusiveLock);
376400

@@ -395,13 +419,8 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri,
395419

396420
check_publication_add_relation(targetrel);
397421

398-
/*
399-
* Translate column names to attnums and make sure the column list
400-
* contains only allowed elements (no system or generated columns etc.).
401-
* Also build an array of attnums, for storing in the catalog.
402-
*/
403-
publication_translate_columns(pri->relation,pri->columns,
404-
&natts,&attarray);
422+
/* Validate and translate column names into a Bitmapset of attnums. */
423+
attnums=pub_collist_validate(pri->relation,pri->columns);
405424

406425
/* Form a tuple. */
407426
memset(values,0,sizeof(values));
@@ -423,7 +442,7 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri,
423442

424443
/* Add column list, if available */
425444
if (pri->columns)
426-
values[Anum_pg_publication_rel_prattrs-1]=PointerGetDatum(buildint2vector(attarray,natts));
445+
values[Anum_pg_publication_rel_prattrs-1]=PointerGetDatum(attnumstoint2vector(attnums));
427446
else
428447
nulls[Anum_pg_publication_rel_prattrs-1]= true;
429448

@@ -451,9 +470,10 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri,
451470
false);
452471

453472
/* Add dependency on the columns, if any are listed */
454-
for (inti=0;i<natts;i++)
473+
i=-1;
474+
while ((i=bms_next_member(attnums,i)) >=0)
455475
{
456-
ObjectAddressSubSet(referenced,RelationRelationId,relid,attarray[i]);
476+
ObjectAddressSubSet(referenced,RelationRelationId,relid,i);
457477
recordDependencyOn(&myself,&referenced,DEPENDENCY_NORMAL);
458478
}
459479

@@ -476,47 +496,23 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri,
476496
returnmyself;
477497
}
478498

479-
/* qsort comparator for attnums */
480-
staticint
481-
compare_int16(constvoid*a,constvoid*b)
482-
{
483-
intav=*(constint16*)a;
484-
intbv=*(constint16*)b;
485-
486-
/* this can't overflow if int is wider than int16 */
487-
return (av-bv);
488-
}
489-
490499
/*
491-
*Translate a list of column names to an array of attribute numbers
492-
* anda Bitmapset with them; verify that each attribute is appropriate
493-
* tohave in a publication column list (no system or generated attributes,
494-
*no duplicates). Additional checks with replica identity are done later;
495-
* see pub_collist_contains_invalid_column.
500+
*pub_collist_validate
501+
*Process andvalidate the 'columns' list and ensure the columns are all
502+
*valid touse for a publication. Checks for and raises an ERROR for
503+
*any; unknown columns, system columns, duplicate columns or generated
504+
*columns.
496505
*
497-
* Note that the attribute numbers are *not* offset by
498-
* FirstLowInvalidHeapAttributeNumber; system columns are forbidden so this
499-
* is okay.
506+
* Looks up each column's attnum and returns a 0-based Bitmapset of the
507+
* corresponding attnums.
500508
*/
501-
staticvoid
502-
publication_translate_columns(Relationtargetrel,List*columns,
503-
int*natts,AttrNumber**attrs)
509+
Bitmapset*
510+
pub_collist_validate(Relationtargetrel,List*columns)
504511
{
505-
AttrNumber*attarray=NULL;
506512
Bitmapset*set=NULL;
507513
ListCell*lc;
508-
intn=0;
509514
TupleDesctupdesc=RelationGetDescr(targetrel);
510515

511-
/* Bail out when no column list defined. */
512-
if (!columns)
513-
return;
514-
515-
/*
516-
* Translate list of columns to attnums. We prohibit system attributes and
517-
* make sure there are no duplicate columns.
518-
*/
519-
attarray=palloc(sizeof(AttrNumber)*list_length(columns));
520516
foreach(lc,columns)
521517
{
522518
char*colname=strVal(lfirst(lc));
@@ -547,16 +543,9 @@ publication_translate_columns(Relation targetrel, List *columns,
547543
colname));
548544

549545
set=bms_add_member(set,attnum);
550-
attarray[n++]=attnum;
551546
}
552547

553-
/* Be tidy, so that the catalog representation is always sorted */
554-
qsort(attarray,n,sizeof(AttrNumber),compare_int16);
555-
556-
*natts=n;
557-
*attrs=attarray;
558-
559-
bms_free(set);
548+
returnset;
560549
}
561550

562551
/*
@@ -569,19 +558,12 @@ publication_translate_columns(Relation targetrel, List *columns,
569558
Bitmapset*
570559
pub_collist_to_bitmapset(Bitmapset*columns,Datumpubcols,MemoryContextmcxt)
571560
{
572-
Bitmapset*result=NULL;
561+
Bitmapset*result=columns;
573562
ArrayType*arr;
574563
intnelems;
575564
int16*elems;
576565
MemoryContextoldcxt=NULL;
577566

578-
/*
579-
* If an existing bitmap was provided, use it. Otherwise just use NULL and
580-
* build a new bitmap.
581-
*/
582-
if (columns)
583-
result=columns;
584-
585567
arr=DatumGetArrayTypeP(pubcols);
586568
nelems=ARR_DIMS(arr)[0];
587569
elems= (int16*)ARR_DATA_PTR(arr);

‎src/backend/commands/publicationcmds.c

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,21 +1176,13 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup,
11761176
newrelid=RelationGetRelid(newpubrel->relation);
11771177

11781178
/*
1179-
* If the new publication has column list, transform it to a
1180-
* bitmap too.
1179+
* Validate the column list. If the column list or WHERE
1180+
* clause changes, then the validation done here will be
1181+
* duplicated inside PublicationAddTables(). The validation
1182+
* is cheap enough that that seems harmless.
11811183
*/
1182-
if (newpubrel->columns)
1183-
{
1184-
ListCell*lc;
1185-
1186-
foreach(lc,newpubrel->columns)
1187-
{
1188-
char*colname=strVal(lfirst(lc));
1189-
AttrNumberattnum=get_attnum(newrelid,colname);
1190-
1191-
newcolumns=bms_add_member(newcolumns,attnum);
1192-
}
1193-
}
1184+
newcolumns=pub_collist_validate(newpubrel->relation,
1185+
newpubrel->columns);
11941186

11951187
/*
11961188
* Check if any of the new set of relations matches with the
@@ -1199,7 +1191,7 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup,
11991191
* expressions also match. Same for the column list. Drop the
12001192
* rest.
12011193
*/
1202-
if (RelationGetRelid(newpubrel->relation)==oldrelid)
1194+
if (newrelid==oldrelid)
12031195
{
12041196
if (equal(oldrelwhereclause,newpubrel->whereClause)&&
12051197
bms_equal(oldcolumns,newcolumns))

‎src/backend/commands/subscriptioncmds.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2266,7 +2266,7 @@ fetch_table_list(WalReceiverConn *wrconn, List *publications)
22662266
*
22672267
* Note that attrs are always stored in sorted order so we don't need
22682268
* to worry if different publications have specified them in a
2269-
* different order. Seepublication_translate_columns.
2269+
* different order. Seepub_collist_validate.
22702270
*/
22712271
appendStringInfo(&cmd,"SELECT DISTINCT n.nspname, c.relname, gpt.attrs\n"
22722272
" FROM pg_class c\n"

‎src/include/catalog/pg_publication.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ extern bool is_publishable_relation(Relation rel);
152152
externboolis_schema_publication(Oidpubid);
153153
externObjectAddresspublication_add_relation(Oidpubid,PublicationRelInfo*pri,
154154
boolif_not_exists);
155+
externBitmapset*pub_collist_validate(Relationtargetrel,List*columns);
155156
externObjectAddresspublication_add_schema(Oidpubid,Oidschemaid,
156157
boolif_not_exists);
157158

‎src/test/regress/expected/publication.out

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,13 @@ ERROR: cannot use generated column "d" in publication column list
693693
-- error: system attributes "ctid" not allowed in column list
694694
ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, ctid);
695695
ERROR: cannot use system column "ctid" in publication column list
696+
ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl1 (id, ctid);
697+
ERROR: cannot use system column "ctid" in publication column list
698+
-- error: duplicates not allowed in column list
699+
ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, a);
700+
ERROR: duplicate column "a" in publication column list
701+
ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl5 (a, a);
702+
ERROR: duplicate column "a" in publication column list
696703
-- ok
697704
ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, c);
698705
ALTER TABLE testpub_tbl5 DROP COLUMN c;-- no dice

‎src/test/regress/sql/publication.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,10 @@ ALTER PUBLICATION testpub_fortable DROP TABLE testpub_tbl5;
417417
ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, d);
418418
-- error: system attributes "ctid" not allowed in column list
419419
ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, ctid);
420+
ALTER PUBLICATION testpub_fortableSET TABLE testpub_tbl1 (id, ctid);
421+
-- error: duplicates not allowed in column list
422+
ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, a);
423+
ALTER PUBLICATION testpub_fortableSET TABLE testpub_tbl5 (a, a);
420424
-- ok
421425
ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, c);
422426
ALTERTABLE testpub_tbl5 DROP COLUMN c;-- no dice

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp