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