On Sun, Jan 1, 2017 at 12:34 PM, David Fetter <da...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers