On Sat, Oct 1, 2016 at 8:32 AM, Julien Rouhaud <julien.rouh...@dalibo.com> wrote: > On 30/09/2016 21:12, David Fetter wrote: >> On Fri, Sep 30, 2016 at 06:37:17PM +0200, Julien Rouhaud wrote: >>> On 30/09/2016 05:23, Thomas Munro wrote: >>>> >>>> It would be really nice to be able to set this to 'Ready for >>>> Committer' in this CF. Do you want to post a v6 patch or are you >>>> happy for me to ask a committer to look at v5 + these three >>>> corrections? >>> >>> I just looked at the patch, and noticed that only plain DELETE and >>> UPDATE commands are handled. Is it intended that writable CTE without >>> WHERE clauses are not detected by this extension? I personally think >>> that wCTE should be handled (everyone can forget a WHERE clause), but if >>> not it should at least be documented. >> >> You are correct in that it should work for every unqualified UPDATE or >> DELETE, not just some. Would you be so kind as to send along the >> tests cases you used so I can add them to the patch? >> > > Given your test case, these queries should fail if the related > require_where GUCs are set (obviously last one should failed with either > of the GUC set): > > WITH d AS (DELETE FROM test_require_where) SELECT 1; > > WITH u AS (UPDATE test_require_where SET t = t) SELECT 1; > > WITH d AS (DELETE FROM test_require_where), u AS (UPDATE > test_require_where SET t = t) SELECT 1;
Right. These cases work because they show up as CMD_DELETE/CMD_UPDATE: postgres=# set require_where.delete = on; SET postgres=# with answer as (select 42) delete from foo; ERROR: DELETE requires a WHERE clause when require_where.delete is set to on HINT: To delete all rows, use "WHERE true" or similar. postgres=# prepare x as delete from foo; ERROR: DELETE requires a WHERE clause when require_where.delete is set to on HINT: To delete all rows, use "WHERE true" or similar. But not this case which shows up as a CMD_SELECT: postgres=# set require_where.delete = on; SET postgres=# with q as (delete from foo) select 42; ┌──────────┐ │ ?column? │ ├──────────┤ │ 42 │ └──────────┘ (1 row) I guess you need something involving query_tree_walker or some other kind of recursive traversal if you want to find DELETE/UPDATE lurking in there. One option would be to document it as working for top level DELETE or UPDATE, and leave the recursive version as an improvement for a later patch. That's the most interesting kind to catch because that's what people are most likely to type directly into a command line. -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers