Re: [PATCH] remove redundant ownership checks

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] remove redundant ownership checks
Date: 2009-12-17 21:38:05
Message-ID: 20091217213805.GD17756@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

KaiGai,

* KaiGai Kohei (kaigai(at)ak(dot)jp(dot)nec(dot)com) wrote:
> The patch was not attached...

This patch either does too much, or not enough.

I would either leave the Assert() in-place as a double-check (I presume
that's why it was there in the first place, and if that Assert() fails
then our assumption about the permissions check being already done on
the object in question would be wrong, since the check is done against
the passed-in 'rel' and the assert is that 'rel' and 'ruletup->ev_class'
are the same; if they're not, then we might need to do perms checking on
ruletup->ev_class)

Or

Remove the now-unused variable eventRelationOid.

Overall, I agree with removing this check as it's already done by
ATSimplePermissions() and we don't double-check the permissions in the
other things called through ATExecCmd() (though there are cases where
specific commands have to do *additional* checks beyond what
ATSimplePermissions() does.. it might be worth looking into what those
are and thinking about if we should move/consolidate/etc them, or if it
makes sense to leave them where they are).

Thanks,

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2009-12-17 22:54:16 Re: An example of bugs for Hot Standby
Previous Message Bruce Momjian 2009-12-17 20:50:55 Re: PATCH: Spurious "22" in hstore.sgml