Re: has_column_privilege behavior (was Re: Assert failed insnprintf.c) - Mailing list pgsql-hackers

From Stephen Frost
Subject Re: has_column_privilege behavior (was Re: Assert failed insnprintf.c)
Date
Msg-id 20181001184153.GD4184@tamriel.snowman.net
Whole thread Raw
In response to has_column_privilege behavior (was Re: Assert failed in snprintf.c)  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: has_column_privilege behavior (was Re: Assert failed in snprintf.c)  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: has_column_privilege behavior (was Re: Assert failed insnprintf.c)  (Joe Conway <mail@joeconway.com>)
List pgsql-hackers
Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> I wrote:
> > Jaime Casanova <jaime.casanova@2ndquadrant.com> writes:
> >> sqlsmith made it again, attached is the query (run against regression
> >> database) that makes the assert fail and the backtrace.
> >> this happens in head only (or at least 11 is fine).
>
> > Ah.  Looks like the has_column_privilege stuff is incautious about whether
> > it's handed a valid table OID:
> > regression=# select has_column_privilege(42::oid, 'z'::text, 'q'::text);
> > server closed the connection unexpectedly
>
> So my first thought was to just throw an error for bad table OID,
> as per the attached quick-hack patch.
>
> However, on closer inspection, I wonder whether that's what we really
> want.  The rest of the OID-taking have_foo_privilege functions are
> designed to return null, not fail, if handed a bad OID.  This is meant to
> allow queries scanning system catalogs to not die if an object is
> concurrently dropped.  So I think this should do likewise.

Agreed.

> But it's not quite clear to me what we want the behavior for bad column
> name to be.  A case could be made for either of:
>
> * If either the table OID is bad, or the OID is OK but there's no such
> column, return null.
>
> * Return null for bad OID, but if it's OK, continue to throw error
> for bad column name.
>
> The second case seems weirdly inconsistent, but it might actually
> be the most useful definition.  Not detecting a misspelled column
> name is likely to draw complaints.
>
> Thoughts?

What are we going to do for dropped columns..?  Seems like with what
you're suggesting we'd throw an error, but that'd make querying with
this function similairly annoying at times.

My inclination would be to make the function return NULL in any case
where we can't find what the user is asking for (and to not throw an
error in general).

Thanks!

Stephen

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: has_column_privilege behavior (was Re: Assert failed in snprintf.c)
Next
From: Tom Lane
Date:
Subject: Re: has_column_privilege behavior (was Re: Assert failed in snprintf.c)