Re: more RLS oversights

Joe Conway wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 07/03/2015 10:03 AM, Noah Misch wrote:
> > (2) CreatePolicy() and AlterPolicy() omit to create a pg_shdepend
> > entry for each role in the TO clause. Test case:
>
> Please see the attached patch. Note that I used SHARED_DEPENDENCY_ACL
> for this. It seems appropriate, but possibly we should invent a new
> shared dependency type for this use case? Comments?

Hmm, these are not ACL objects, so conceptually it seems cleaner to use
a different symbol for this. I think the catalog state and the error
messages would be a bit confusing otherwise.

> if (spec->roletype == ROLESPEC_PUBLIC)
> {
> ! Datum *tmp_role_oids;
> !
> ! if (*num_roles != 1)
> ereport(WARNING,
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> errmsg("ignoring roles specified other than public"),
> errhint("All roles are members of the public role.")));
> ! *num_roles = 1;
> ! tmp_role_oids = (Datum *) palloc(*num_roles * sizeof(Datum));
> ! tmp_role_oids[0] = ObjectIdGetDatum(ACL_ID_PUBLIC);

Isn't this leaking the previously allocated array? Not sure it's all
that critical, but still. (I don't think you really need to call palloc
at all here.)

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services