Re: RLS feature has been committed

From: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>, Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: Peter Geoghegan <pg(at)heroku(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, "Brightwell, Adam" <adam(dot)brightwell(at)crunchydatasolutions(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Yeb Havinga <yeb(dot)havinga(at)portavita(dot)nl>
Subject: Re: RLS feature has been committed
Date: 2014-09-26 07:48:03
Message-ID: 54251A33.9050205@vmware.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 09/26/2014 01:07 AM, Stephen Frost wrote:
> * Simon Riggs (simon(at)2ndquadrant(dot)com) wrote:
>> My major reason to revert is the following: the documentation contains
>> no examples of real world usage, making the feature essentially
>> unusable out of the box.
>
> I find this to be an interesting argument considering most of our
> documentation doesn't include real-world examples.

+1 for adding examples.

> This wouldn't be the only case of documentation (indeed, *any*
> documentation) being added after a commit, and so I'm mystified by this
> requirement for *real-world* examples in documentation to be provided
> prior to commit.

IMO it depends on the situation. Yeah, we've had a lot of commits
without docs, where the documentation has been added later. I'm OK with
that, if for example the committed patch is part of a series of patches,
where it doesn't make sense to add documentation until the whole series
has been committed. Then there are situations where the patch author
just hasn't gotten around to adding documentation yet (like in this
case). That's more questionable; documentation is an important part of
adding a feature, and there's the risk that the author never gets around
to add documentation, after the code has been committed, or does only
lip service to it. It's usually worked out fine, though - people who
have successfully added major features are conscientious enough to get
it done.

But in many cases, lack of good documentation make even reviewing the
patch difficult. How do you determine if the patch works as intended, if
you don't know what it's supposed to do?

Wrt. reverting or not, I'm strongly of the opinion that whatever. If you
just add the docs, I'm happy. This is a big and security-sensitive
feature, and it requires documentation not only on how to use the
feature, but an introduction to what row-level security is, what level
of security to expect, what circumstances you would use it in,
comparison to other approaches, the side-channels, etc. We'll probably
have to go through a few rounds of review on the docs alone. I'd leave
it up to Stephen to decided how he wants to handle this. But I'd really
like to see the documentation added (at least posted as a patch if not
committed), well before the next commitfest, so that Robert and others
who want to review the code, have the documentation available.

- Heikki

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2014-09-26 08:06:07 Re: Yet another abort-early plan disaster on 9.3
Previous Message Andres Freund 2014-09-26 07:37:50 Re: copy & pastos in atomics.h comments