Peter Eisentraut <[email protected]> wrote:
> Continuing the ideas in [0], this patch refactors the ExecGrant_Foo()
> functions and replaces many of them by a common function that is driven by the
> ObjectProperty table.
>
> It would be nice to do more here, for example ExecGrant_Language(), which has
> additional non-generalizable checks, but I think this is already a good start.
> For example, the work being discussed on privileges on publications [1] would
> be able to take good advantage of this.
Right, I mostly copy and pasted the code when writing
ExecGrant_Publication(). I agree that your refactoring is very useful.
Attached are my proposals for improvements. One is to avoid memory leak, the
other tries to improve readability a little bit.
> [0]:
> https://www.postgresql.org/message-id/[email protected]
> [1]: https://www.postgresql.org/message-id/flat/20330.1652105397@antos
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index d3121a469f..ac4490c0b8 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -2228,6 +2234,9 @@ ExecGrant_generic(InternalGrant *istmt, Oid classid, AclMode default_privs)
newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values,
nulls, replaces);
+ pfree(values);
+ pfree(nulls);
+ pfree(replaces);
CatalogTupleUpdate(relation, &newtuple->t_self, newtuple);
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index d3121a469f..ac4490c0b8 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -2144,6 +2144,7 @@ ExecGrant_generic(InternalGrant *istmt, Oid classid, AclMode default_privs)
{
Oid objectid = lfirst_oid(cell);
Datum aclDatum;
+ Datum nameDatum;
bool isNull;
AclMode avail_goptions;
AclMode this_privileges;
@@ -2197,6 +2198,11 @@ ExecGrant_generic(InternalGrant *istmt, Oid classid, AclMode default_privs)
old_acl, ownerId,
&grantorId, &avail_goptions);
+ nameDatum = SysCacheGetAttr(cacheid, tuple,
+ get_object_attnum_name(classid),
+ &isNull);
+ Assert(!isNull);
+
/*
* Restrict the privileges to what we can actually grant, and emit the
* standards-mandated warning and error messages.
@@ -2205,7 +2211,7 @@ ExecGrant_generic(InternalGrant *istmt, Oid classid, AclMode default_privs)
restrict_and_check_grant(istmt->is_grant, avail_goptions,
istmt->all_privs, istmt->privileges,
objectid, grantorId, get_object_type(classid, objectid),
- NameStr(*DatumGetName(SysCacheGetAttr(cacheid, tuple, get_object_attnum_name(classid), &isNull))),
+ NameStr(*DatumGetName(nameDatum)),
0, NULL);
/*
