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

Commitd31bbfb

Browse files
committed
Proper object locking for GRANT/REVOKE
Refactor objectNamesToOids() to use get_object_address() internally ifpossible. Not only does this save a lot of code, it also allows us touse the object locking provided by get_object_address() forGRANT/REVOKE. There was previously a code comment that complainedabout the lack of locking in objectNamesToOids(), which is now fixed.The check in ExecGrant_Type_check() is obsolete becauseget_object_address_type() already does the same check.Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>Discussion:https://www.postgresql.org/message-id/flat/bf72b82c-124d-4efa-a484-bb928e9494e4@eisentraut.org
1 parentcfd7f36 commitd31bbfb

File tree

2 files changed

+39
-120
lines changed

2 files changed

+39
-120
lines changed

‎src/backend/catalog/aclchk.c‎

Lines changed: 38 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -659,147 +659,77 @@ ExecGrantStmt_oids(InternalGrant *istmt)
659659
* objectNamesToOids
660660
*
661661
* Turn a list of object names of a given type into an Oid list.
662-
*
663-
* XXX: This function doesn't take any sort of locks on the objects whose
664-
* names it looks up. In the face of concurrent DDL, we might easily latch
665-
* onto an old version of an object, causing the GRANT or REVOKE statement
666-
* to fail.
667662
*/
668663
staticList*
669664
objectNamesToOids(ObjectTypeobjtype,List*objnames,boolis_grant)
670665
{
671666
List*objects=NIL;
672667
ListCell*cell;
668+
constLOCKMODElockmode=AccessShareLock;
673669

674670
Assert(objnames!=NIL);
675671

676672
switch (objtype)
677673
{
678-
caseOBJECT_TABLE:
679-
caseOBJECT_SEQUENCE:
680-
foreach(cell,objnames)
681-
{
682-
RangeVar*relvar= (RangeVar*)lfirst(cell);
683-
OidrelOid;
684-
685-
relOid=RangeVarGetRelid(relvar,NoLock, false);
686-
objects=lappend_oid(objects,relOid);
687-
}
688-
break;
689-
caseOBJECT_DATABASE:
690-
foreach(cell,objnames)
691-
{
692-
char*dbname=strVal(lfirst(cell));
693-
Oiddbid;
694-
695-
dbid=get_database_oid(dbname, false);
696-
objects=lappend_oid(objects,dbid);
697-
}
698-
break;
699-
caseOBJECT_DOMAIN:
700-
caseOBJECT_TYPE:
701-
foreach(cell,objnames)
702-
{
703-
List*typname= (List*)lfirst(cell);
704-
Oidoid;
705-
706-
oid=typenameTypeId(NULL,makeTypeNameFromNameList(typname));
707-
objects=lappend_oid(objects,oid);
708-
}
709-
break;
710-
caseOBJECT_FUNCTION:
711-
foreach(cell,objnames)
712-
{
713-
ObjectWithArgs*func= (ObjectWithArgs*)lfirst(cell);
714-
Oidfuncid;
674+
default:
715675

716-
funcid=LookupFuncWithArgs(OBJECT_FUNCTION,func, false);
717-
objects=lappend_oid(objects,funcid);
718-
}
719-
break;
720-
caseOBJECT_LANGUAGE:
676+
/*
677+
* For most object types, we use get_object_address() directly.
678+
*/
721679
foreach(cell,objnames)
722680
{
723-
char*langname=strVal(lfirst(cell));
724-
Oidoid;
681+
ObjectAddressaddress;
725682

726-
oid=get_language_oid(langname, false);
727-
objects=lappend_oid(objects,oid);
683+
address=get_object_address(objtype,lfirst(cell),NULL,lockmode, false);
684+
objects=lappend_oid(objects,address.objectId);
728685
}
729686
break;
730-
caseOBJECT_LARGEOBJECT:
731-
foreach(cell,objnames)
732-
{
733-
OidlobjOid=oidparse(lfirst(cell));
734687

735-
if (!LargeObjectExists(lobjOid))
736-
ereport(ERROR,
737-
(errcode(ERRCODE_UNDEFINED_OBJECT),
738-
errmsg("large object %u does not exist",
739-
lobjOid)));
740-
741-
objects=lappend_oid(objects,lobjOid);
742-
}
743-
break;
744-
caseOBJECT_SCHEMA:
745-
foreach(cell,objnames)
746-
{
747-
char*nspname=strVal(lfirst(cell));
748-
Oidoid;
688+
caseOBJECT_TABLE:
689+
caseOBJECT_SEQUENCE:
749690

750-
oid=get_namespace_oid(nspname, false);
751-
objects=lappend_oid(objects,oid);
752-
}
753-
break;
754-
caseOBJECT_PROCEDURE:
691+
/*
692+
* Here, we don't use get_object_address(). It requires that the
693+
* specified object type match the actual type of the object, but
694+
* in GRANT/REVOKE, all table-like things are addressed as TABLE.
695+
*/
755696
foreach(cell,objnames)
756697
{
757-
ObjectWithArgs*func= (ObjectWithArgs*)lfirst(cell);
758-
Oidprocid;
698+
RangeVar*relvar= (RangeVar*)lfirst(cell);
699+
OidrelOid;
759700

760-
procid=LookupFuncWithArgs(OBJECT_PROCEDURE,func, false);
761-
objects=lappend_oid(objects,procid);
701+
relOid=RangeVarGetRelid(relvar,lockmode, false);
702+
objects=lappend_oid(objects,relOid);
762703
}
763704
break;
764-
caseOBJECT_ROUTINE:
765-
foreach(cell,objnames)
766-
{
767-
ObjectWithArgs*func= (ObjectWithArgs*)lfirst(cell);
768-
Oidroutid;
769705

770-
routid=LookupFuncWithArgs(OBJECT_ROUTINE,func, false);
771-
objects=lappend_oid(objects,routid);
772-
}
773-
break;
774-
caseOBJECT_TABLESPACE:
775-
foreach(cell,objnames)
776-
{
777-
char*spcname=strVal(lfirst(cell));
778-
Oidspcoid;
706+
caseOBJECT_DOMAIN:
707+
caseOBJECT_TYPE:
779708

780-
spcoid=get_tablespace_oid(spcname, false);
781-
objects=lappend_oid(objects,spcoid);
782-
}
783-
break;
784-
caseOBJECT_FDW:
709+
/*
710+
* The parse representation of types and domains in privilege
711+
* targets is different from that expected by get_object_address()
712+
* (for parse conflict reasons), so we have to do a bit of
713+
* conversion here.
714+
*/
785715
foreach(cell,objnames)
786716
{
787-
char*fdwname=strVal(lfirst(cell));
788-
Oidfdwid=get_foreign_data_wrapper_oid(fdwname, false);
717+
List*typname= (List*)lfirst(cell);
718+
TypeName*tn=makeTypeNameFromNameList(typname);
719+
ObjectAddressaddress;
720+
Relationrelation;
789721

790-
objects=lappend_oid(objects,fdwid);
722+
address=get_object_address(objtype, (Node*)tn,&relation,lockmode, false);
723+
Assert(relation==NULL);
724+
objects=lappend_oid(objects,address.objectId);
791725
}
792726
break;
793-
caseOBJECT_FOREIGN_SERVER:
794-
foreach(cell,objnames)
795-
{
796-
char*srvname=strVal(lfirst(cell));
797-
Oidsrvid=get_foreign_server_oid(srvname, false);
798727

799-
objects=lappend_oid(objects,srvid);
800-
}
801-
break;
802728
caseOBJECT_PARAMETER_ACL:
729+
730+
/*
731+
* Parameters are handled completely differently.
732+
*/
803733
foreach(cell,objnames)
804734
{
805735
/*
@@ -830,9 +760,6 @@ objectNamesToOids(ObjectType objtype, List *objnames, bool is_grant)
830760
objects=lappend_oid(objects,parameterId);
831761
}
832762
break;
833-
default:
834-
elog(ERROR,"unrecognized GrantStmt.objtype: %d",
835-
(int)objtype);
836763
}
837764

838765
returnobjects;
@@ -2458,14 +2385,6 @@ ExecGrant_Type_check(InternalGrant *istmt, HeapTuple tuple)
24582385
(errcode(ERRCODE_INVALID_GRANT_OPERATION),
24592386
errmsg("cannot set privileges of multirange types"),
24602387
errhint("Set the privileges of the range type instead.")));
2461-
2462-
/* Used GRANT DOMAIN on a non-domain? */
2463-
if (istmt->objtype==OBJECT_DOMAIN&&
2464-
pg_type_tuple->typtype!=TYPTYPE_DOMAIN)
2465-
ereport(ERROR,
2466-
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
2467-
errmsg("\"%s\" is not a domain",
2468-
NameStr(pg_type_tuple->typname))));
24692388
}
24702389

24712390
staticvoid

‎src/test/isolation/expected/intra-grant-inplace.out‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,6 @@ relhasindex
248248
-----------
249249
(0 rows)
250250

251-
s4: WARNING: got:cache lookup failed for relation REDACTED
251+
s4: WARNING: got:relation "intra_grant_inplace" does not exist
252252
step revoke4: <... completed>
253253
step r3: ROLLBACK;

0 commit comments

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