Date: Mon, 29 Dec 2014 20:34:36 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Adam Brightwell <adam.brightwell@crunchydatasolutions.com>
Cc: Noah Misch <noah@leadboat.com>, Tom Lane <tgl@sss.pgh.pa.us>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: CATUPDATE confusion?
Message-ID: <20141230013435.GI3062@tamriel.snowman.net>
References: 
 <CAKRt6CQOvT2KiyKg2gFf7h9k8+JVU1149zLb0EXTkKk7TAQGMQ@mail.gmail.com>
 <20141227080926.GA2035611@tornado.leadboat.com>
 <20141227151034.GU3062@tamriel.snowman.net>
 <28382.1419703324@sss.pgh.pa.us>
 <20141227231829.GX3062@tamriel.snowman.net>
 <1952.1419722762@sss.pgh.pa.us>
 <20141227233150.GA2046947@tornado.leadboat.com>
 <CAKRt6CTrOf6w5sE5GDRQAJ+JQrfK4h8VM=jVCtiGC+mdFsmXGg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
 protocol="application/pgp-signature"; boundary="G4GkWxNJALccptvH"
Content-Disposition: inline
In-Reply-To: 
 <CAKRt6CTrOf6w5sE5GDRQAJ+JQrfK4h8VM=jVCtiGC+mdFsmXGg@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


--G4GkWxNJALccptvH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Adam,

* Adam Brightwell (adam.brightwell@crunchydatasolutions.com) wrote:
> Given this discussion, I have attached a patch that removes CATUPDATE for
> review/discussion.

Thanks!

> One of the interesting behaviors (or perhaps not) is how 'pg_class_aclmask'
> handles an invalid role id when checking permissions against 'rolsuper'
> instead of 'rolcatupdate'.  This is demonstrated by the
> 'has_table_privilege' regression test expected results.  In summary,
> 'has_rolcatupdate' raises an error when an invalid OID is provided,
> however, as documented in the source 'superuser_arg' does not, simply
> returning false.  Therefore, when checking for superuser-ness of an
> non-existent role, the result will be false and not an error.  Perhaps this
> is OK, but my concern would be on the expected behavior around items like
> 'has_table_privilege'.  My natural thought would be that we would want to
> preserve that specific functionality, though short of adding a
> 'has_rolsuper' function that will raise an appropriate error, I'm uncertain
> of an approach.  Thoughts?

This role oid check is only getting called in this case because it's
pg_authid which is getting checked (because it's a system catalog table)
and it's then falling into this one case.  I don't think we need to
preserve that.

If you do the same query with that bogus OID but a normal table, you get
the same 'f' result that the regression test gives with this change.
We're not back-patching this and I seriously doubt anyone is relying on
this special response for system catalog tables.

So, unless anyone wants to object, I'll continue to look over this patch
for eventual application against master.  If there are objections, it'd
be great if they could be voiced sooner rather than later.

	Thanks!

		Stephen

--G4GkWxNJALccptvH
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJUogErAAoJEO1sijiDR2RVbUwQAMYR+Fhdl0gsQUg7V2JoAiZ7
/YpQiz+Fn1crx5Y44gk48c2zzeNo4fQ3psXHYIFNe9MdWYrpM3i88n0WQnExhZ9o
gafM0Q7+ouKhd8di1sOLtKjPuZevuUsF++NBqJVif36w2ZCezipHCUp3kuuemqzj
woXDa7NF0BoniZfeYy7ZRKvtMwdO/x8l2B59n9YqX1o+nr8gJ6+VQ/ztHCGffczz
9g4TwLvGCDSeYBAHW8oI/vUowbTXehUKWrTkExOvRZPKQP+aHchEQYw/Rkh5oexN
BLXOoYOe2/kqNd7tVRaCDiSPF+xwckpaI6FIP3AVXU6ep3OWGfW6l3svfuur7sSq
Ks0VPnVEpDY604eSzO+AWwwWDDOTgxhiG/1GLUUXv9/BAQqWBbZ6GWeCz5hYdM5s
EZUjqA+Aqc35xAtj6WlV7nU2y8RKK67HfrtDKUs0Quo6IyWaA4+OBg2VVop2D3jd
T1tSpbpQn0maMyzYoC4t+b2ga3DmcOCch+WBPd5AUNnTIG29Ti+QL2a14pp5WvBR
uapqhvdE2QUWY0hauQBJfznfcf+nZIEWQBO433xgfhjRj0WiJYxK5E1J47eGUf4u
X3fryRBURkTI1Lp1poE8IWETaI6KIXT8P/VM6ULg8WnRRxgBbBKMU5dLoBLQEk8t
zmDJFk2Shy/k6MK1ekQw
=1jW2
-----END PGP SIGNATURE-----

--G4GkWxNJALccptvH--