Simplify calls of pg_class_aclcheck when multiple modes are used

Started by Michael Paquierover 11 years ago7 messageshackers
Jump to latest
#1Michael Paquier
michael@paquier.xyz

Hi all,

In a couple of code paths we do the following to check permissions on an
object:
if (pg_class_aclcheck(relid, userid, ACL_USAGE) != ACLCHECK_OK &&
pg_class_aclcheck(relid, userid, ACL_UPDATE) != ACLCHECK_OK)
ereport(ERROR, blah);

Wouldn't it be better to simplify that with a single call of
pg_class_aclcheck, gathering together the modes that need to be checked? In
the case above, the call to pg_class_aclcheck would become like that:
if (pg_class_aclcheck(relid, userid,
ACL_USAGE | ACL_UPDATE) != ACLCHECK_OK)
ereport(ERROR, blah);

That's not a critical thing, but it would save some cycles. Patch is
attached.
Regards,
--
Michael

Attachments:

20140827_aclcheck_simplify.patchtext/x-patch; charset=US-ASCII; name=20140827_aclcheck_simplify.patchDownload+6-7
#2Fabrízio de Royes Mello
fabriziomello@gmail.com
In reply to: Michael Paquier (#1)
Re: Simplify calls of pg_class_aclcheck when multiple modes are used

On Wed, Aug 27, 2014 at 9:02 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:

Hi all,

In a couple of code paths we do the following to check permissions on an

object:

if (pg_class_aclcheck(relid, userid, ACL_USAGE) != ACLCHECK_OK &&
pg_class_aclcheck(relid, userid, ACL_UPDATE) != ACLCHECK_OK)
ereport(ERROR, blah);

Wouldn't it be better to simplify that with a single call of

pg_class_aclcheck, gathering together the modes that need to be checked? In
the case above, the call to pg_class_aclcheck would become like that:

if (pg_class_aclcheck(relid, userid,
ACL_USAGE | ACL_UPDATE) != ACLCHECK_OK)
ereport(ERROR, blah);

That's not a critical thing, but it would save some cycles. Patch is

attached.

I did a little review:
- applied to master without errors
- no compiler warnings
- no regressions

It's a minor change and as Michael already said would save some cycles.

Marked as "Ready for commiter".

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

Show quoted text

Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Michael Paquier (#1)
Re: Simplify calls of pg_class_aclcheck when multiple modes are used

On 8/27/14 8:02 AM, Michael Paquier wrote:

In a couple of code paths we do the following to check permissions on an
object:
if (pg_class_aclcheck(relid, userid, ACL_USAGE) != ACLCHECK_OK &&
pg_class_aclcheck(relid, userid, ACL_UPDATE) != ACLCHECK_OK)
ereport(ERROR, blah);

Wouldn't it be better to simplify that with a single call of
pg_class_aclcheck, gathering together the modes that need to be checked?

Yes, it's probably just an oversight.

While looking at this, I wrote a few tests cases for sequence
privileges, because that was not covered at all. That patch is attached.

That led me to discover this issue:
/messages/by-id/5446B819.1020600@gmx.net

I'll wait for the resolution of that and then commit this.

Attachments:

sequence-privileges-tests.patchtext/x-diff; name=sequence-privileges-tests.patchDownload+198-0
#4Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#3)
Re: Simplify calls of pg_class_aclcheck when multiple modes are used

On Wed, Oct 22, 2014 at 5:03 AM, Peter Eisentraut <peter_e@gmx.net> wrote:

While looking at this, I wrote a few tests cases for sequence
privileges, because that was not covered at all. That patch is attached.

+1 for those tests.
-- 
Michael
#5Fabrízio de Royes Mello
fabriziomello@gmail.com
In reply to: Michael Paquier (#4)
Re: Simplify calls of pg_class_aclcheck when multiple modes are used

Em terça-feira, 21 de outubro de 2014, Michael Paquier <
michael.paquier@gmail.com> escreveu:

On Wed, Oct 22, 2014 at 5:03 AM, Peter Eisentraut <peter_e@gmx.net
<javascript:_e(%7B%7D,'cvml','peter_e@gmx.net');>> wrote:

While looking at this, I wrote a few tests cases for sequence
privileges, because that was not covered at all. That patch is attached.

+1 for those tests.

+1

Fabrízio Mello

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

Show quoted text

Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello

#6Peter Eisentraut
peter_e@gmx.net
In reply to: Michael Paquier (#4)
Re: Simplify calls of pg_class_aclcheck when multiple modes are used

On 10/21/14 6:19 PM, Michael Paquier wrote:

On Wed, Oct 22, 2014 at 5:03 AM, Peter Eisentraut <peter_e@gmx.net
<mailto:peter_e@gmx.net>> wrote:

While looking at this, I wrote a few tests cases for sequence
privileges, because that was not covered at all. That patch is
attached.

+1 for those tests.
-- 
Michael

Committed your patch and tests.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#6)
Re: Simplify calls of pg_class_aclcheck when multiple modes are used

On Thu, Oct 23, 2014 at 10:45 AM, Peter Eisentraut <peter_e@gmx.net> wrote:

Committed your patch and tests.

Thanks!
--
Michael