Re: WITH CHECK and Column-Level Privileges

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: WITH CHECK and Column-Level Privileges
Date: 2015-01-21 04:32:53
Message-ID: 20150121043252.GC3062@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

* Noah Misch (noah(at)leadboat(dot)com) wrote:
> On Mon, Jan 19, 2015 at 11:05:22AM -0500, Stephen Frost wrote:
> > One remaining question is about single-column key violations. Should we
> > special-case those and allow them to be shown or no? I can't see a
> > reason not to currently but I wonder if we might have cause to act
> > differently in the future (not that I can think of a reason we'd ever
> > need to).
>
> Keep them hidden. Attempting to delete a PK row should not reveal
> otherwise-inaccessible values involved in any constraint violation. It's
> tempting to make an exception for single-column UNIQUE constraints, but some
> of the ensuing data disclosures are questionable. What if the violation arose
> from a column default expression or from index corruption?

Interesting point. I've kept them hidden in this latest version.

> On Mon, Jan 19, 2015 at 11:46:35AM -0500, Stephen Frost wrote:
> > Right, ExecBuildSlotValueDescription() was made to be consistent with
> > BuildIndexValueDescription. The reason for BuildIndexValueDescription
> > returning an empty string is different from why you hypothosize though-
> > it's exported and I was a bit worried that existing external callers
> > might not be prepared for it to return a NULL instead of a string of
> > some kind. If this was a green field instead of a back-patch fix, I'd
> > certainly return NULL instead.
> >
> > If others feel that's not a good reason to avoid returning NULL, I can
> > certainly change it around.
>
> I won't lose sleep if it does return "" for that reason, but I'm relatively
> unconcerned about extension API compatibility here. That function is called
> from datatype-independent index uniqueness checks. This mailing list has
> discussed the difficulties of implementing an index access method in an
> extension, and no such extension has come forward.

Alright, I've reworked this to have those functions return NULL instead.

> Your latest patch has whitespace errors; visit "git diff --check".

Yeah, I had done that but then made a few additional tweaks and didn't
recheck, sorry about that. I'm playing around w/ my git workflow a bit
and hopefully it won't happen again. :)

Updated patch attached.

Thanks!

Stephen

Attachment Content-Type Size
fix-leak94_v6.patch text/x-diff 26.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2015-01-21 05:14:45 Re: Dereferenced pointers checked as NULL in btree_utils_var.c
Previous Message Andrew Dunstan 2015-01-21 04:00:02 Re: [Pgbuildfarm-members] [HACKERS] Reducing buildfarm disk usage: remove temp installs when done