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

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: David Fetter <david(at)fetter(dot)org>
Cc: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, 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: 2017-01-01 10:57:33
Message-ID: CAB7nPqRh5mK+_zcfLf1UypqowsKrMz8atcCbQr2Eh7o8mqSqAA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Jan 1, 2017 at 12:34 PM, David Fetter <david(at)fetter(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2017-01-01 11:53:13 Re: Fixing pgbench's logging of transaction timestamps
Previous Message Fabien COELHO 2017-01-01 10:28:12 Re: proposal: session server side variables