Re: WITH CHECK and Column-Level Privileges

* Dean Rasheed (dean(dot)a(dot)rasheed(at)gmail(dot)com) wrote:
> On 30 September 2014 20:17, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > * Dean Rasheed (dean(dot)a(dot)rasheed(at)gmail(dot)com) wrote:
> >> One of the main things that detail is useful for is identifying the
> >> failing row in a multi-row update. In most real-world cases, I would
> >> expect the column-level privileges to include the table's PK, so the
> >> detail would meet that requirement. In fact the column-level
> >> privileges would pretty much have to include sufficient columns to
> >> usefully identify rows, otherwise updates wouldn't be practical.
> >
> > That may or may not be true- the user needs sufficient information to
> > identify a row, but that doesn't mean they have access to all columns
> > make up a unique constraint. It's not uncommon to have a surrogate key
> > for identifying the rows and then an independent uniqueness constraint
> > on some natural key for the table, which the user may not have access
> > to.
>
> True, but then the surrogate key would be included in the error
> details which would allow the failing row to be identified.

Right, and is information which the user would have provided in the
query itself.

> >> > What do you think about returning just what the user provided in the
> >> > first place in both of these cases..? I'm not quite sure what that
> >> > would look like in the UPDATE case but for INSERT (and COPY) it would be
> >> > the subset of columns being inserted and the values originally provided.
> >> > That may not be what the actual conflict is due to, as there could be
> >> > before triggers changing things in the middle, or the conflict could be
> >> > on default values, but at least the input row could be identified and
> >> > there wouldn't be this information leak risk. Not sure how difficult
> >> > that would be to implement though.
> >>
> >> I could see that working for INSERTs, but for UPDATEs I don't think
> >> that would be very useful in practice, because the columns most likely
> >> to be useful for identifying the failing row (e.g., key columns) are
> >> also the columns least likely to have been updated.
> >
> > I'm not sure that I follow this- if you're not updating the key columns
> > then you're not likely to be having a conflict due to them...
>
> The constraint violation could well be due to updating a non-key
> column such as a column with a NOT NULL constraint on it, in which
> case only including that column in the error detail wouldn't do much
> good -- you'd want to include the key columns if possible.

This I can agree with- if the query doesn't include row-identifying
information (which implies that they're updating multiple records with
one statement) then it'd be helpful to know what row was failing the
update.

Practically, things get a bit tricky with this though- if we're only
going to return the columns which the user has access to, how do we
communicate which columns those are? The current error message doesn't
contain that information explicitly, it depends on the user being
knowledgable of (or able to look up) the table definition. The key
violation case only returns the columns of the key violated and we could
check that the user has either full table-level SELECT or has SELECT
rights to all of the columns involved in the key.

We could also check if the user has either table-level SELECT rights or
has SELECT rights to all columns in the primary key of the table and
then return the primary key in these other cases (what to do if there is
no primary key?). I'm not sure if we'd want to back-patch a change like
that and I'm a bit worried about the complexity of it in general- having
the error message depend so much on the permissions seems like it would
make things difficult for anything which is currently using that error
message in a programatic way (which I fully expect there are cases
of..).

Thanks,

Stephen