Re: PoC: Make it possible to disallow WHERE-less UPDATE and DELETE

On Fri, Sep 09, 2016 at 09:57:21AM -0400, Peter Eisentraut wrote:
> Review of the patch in the commit fest:
>
> - Various naming/spelling inconsistencies: In the source, the module
> is require_where, the documentation titles it require-where, the GUC
> parameters are requires_where.*, but incorrectly documented.

Fixed.

> - Unusual indentation in the Makefile

Fixed.

> - Needs tests

Still needs some fixing.

> - Not sure about errcode(ERRCODE_CARDINALITY_VIOLATION), which is
> documented in the code as "this means something returned the wrong
> number of rows". I think ERRCODE_SYNTAX_ERROR or something from
> nearby there would be better.

Changed to ERRCODE_SYNTAX_ERROR. CARDINALITY_VIOLATION was a bit too
cute.

> - errhint() string should end with a period.

Fixed.

> - The 7th argument of DefineCustomBoolVariable() is of type int, not
> bool, so passing false is somewhat wrong, even if it works.

Fixed.

> - There ought to be a _PG_fini() function that undoes what _PG_init()
> does.

Fixed.

> - The documentation should be expanded and clarified. Given that this
> is a "training wheels" module, we can be extra clear here. I would
> like to see some examples at least.

Working on this.

> - The documentation is a bit incorrect about the ways to load this
> module. shared_preload_libraries is not necessary. session_ and
> local_ (with prep) should also work.

I'm not 100% sure I understand what you want here. I did manage to
get the thing loaded without a restart via LOAD, but that's it so far.
Will continue to poke at it.

> - The claim in the documentation that only superusers can do things
> with this module is not generally correct.

I think that the claims are fixed. This is SUSET, at least in this
patch, because anything short of that that changes query behavior
seems incautious.

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate