From: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
---|---|
To: | David Fetter <david(at)fetter(dot)org> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: PoC: Make it possible to disallow WHERE-less UPDATE and DELETE |
Date: | 2016-09-09 13:57:21 |
Message-ID: | f5f248ee-9c47-7bec-0b56-5bcb2760f246@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
- Unusual indentation in the Makefile
- Needs tests
- 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.
- errhint() string should end with a period.
- The 7th argument of DefineCustomBoolVariable() is of type int, not
bool, so passing false is somewhat wrong, even if it works.
- There ought to be a _PG_fini() function that undoes what _PG_init()
does.
- 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.
- 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.
- The claim in the documentation that only superusers can do things
with this module is not generally correct.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2016-09-09 14:15:43 | Re: Add support for restrictive RLS policies |
Previous Message | Pavel Stehule | 2016-09-09 13:44:07 | Re: patch: function xmltable |