On Sun, Jan 1, 2017 at 12:34 PM, David Fetter <david@fetter.org> wrote:
> I've rolled your patches into this next one and clarified the commit
> message, as there appears to have been some confusion about the scope.
Not all the comments have been considered!
Here are some example..
=# set require_where.delete to on;
SET
=# copy (delete from aa returning a) to stdout;
ERROR: 42601: DELETE requires a WHERE clause when
require_where.delete is set to on
HINT: To delete all rows, use "WHERE true" or similar.
LOCATION: require_where_check, require_where.c:42
COPY with DML returning rows are correctly restricted.
Now if I look at WITH clauses...
=# with delete_query as (delete from aa returning a) select * from delete_query;a
---
(0 rows)
=# with update_query as (update aa set a = 3 returning a) select *
from update_query;a
---
(0 rows)
Oops. That's not cool.
CTAS also perform no checks:
=# create table ab as with delete_query as (delete from aa returning
a) select * from delete_query;
SELECT 0
Is there actually a meaning to have two options? Couldn't we leave
with just one? Actually, I'd just suggest to rip off the option and
just to make the checks enabled when the library is loaded, to get a
module as simple as passwordcheck.
--- /dev/null
+++ b/contrib/require_where/data/test_require_where.data
@@ -0,0 +1,16 @@
+Four
There is no need to load fake data as you need to just check the
parsing of the query. So let's simplify the tests and remove that.
Except that the shape of the module is not that bad. The copyright
notices need to be updated to 2017, and it would be nice to have some
comments at the top of require_where.c to describe what it does.
--
Michael