On Fri, Sep 30, 2016 at 04:23:13PM +1300, Thomas Munro wrote: > On Thu, Sep 29, 2016 at 6:19 PM, David Fetter <da...@fetter.org> wrote: > > On Thu, Sep 29, 2016 at 11:12:11AM +1300, Thomas Munro wrote: > >> On Mon, Sep 26, 2016 at 5:11 PM, Thomas Munro > >> <thomas.mu...@enterprisedb.com> wrote: > >> > On Mon, Sep 26, 2016 at 1:18 PM, Thomas Munro > >> > <thomas.mu...@enterprisedb.com> wrote: > >> >> > >> >> On Mon, Sep 19, 2016 at 4:02 PM, David Fetter <da...@fetter.org> wrote: > >> >> > > >> >> > [training_wheels_004.patch] > >> >> > >> >> [review] > >> > >> Ping. > > > > Please find attached the next revision. > > I'm not sold on ERRCODE_SYNTAX_ERROR. There's nothing wrong with the > syntax, since parsing succeeded. It would be cool if we could use > ERRCODE_E_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED, though I'm not sure > what error class 38 really means. Does require_where's hook function > count as an 'external routine' and on that basis is it it allowed to > raise this error? Thoughts, anyone? > > I am still seeing the underscore problem when building the docs. > Please find attached fix-docs.patch which applies on top of > training_wheels_005.patch. It fixes that problem for me. > > The regression test fails. The expected error messages show the old > wording, so I think you forgot to add a file. Also, should > contrib/require_where/Makefile define REGRESS = require_where? That > would allow 'make check' from inside that directory, which is > convenient and matches other extensions. Please find attached > fix-regression-test.patch which also applies on top of > training_wheels_005.patch and fixes those things for me, and also adds > a couple of extra test cases. > > Robert Haas mentioned upthread that the authors section was too short, > and your follow-up v3 patch addressed that. Somehow two authors got > lost on their way to the v5 patch. Please find attached > fix-authors.patch which also applies on top of > training_wheels_005.patch to restore them. > > 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?
Thanks! 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. Best, David. -- David Fetter <david(at)fetter(dot)org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/contrib/Makefile b/contrib/Makefile index 25263c0..4bd456f 100644 --- a/contrib/Makefile +++ b/contrib/Makefile @@ -40,6 +40,7 @@ SUBDIRS = \ pgstattuple \ pg_visibility \ postgres_fdw \ + require_where \ seg \ spi \ tablefunc \ diff --git a/contrib/require_where/Makefile b/contrib/require_where/Makefile new file mode 100644 index 0000000..9c41691 --- /dev/null +++ b/contrib/require_where/Makefile @@ -0,0 +1,19 @@ +# contrib/require_where/Makefile + +MODULE_big = require_where +OBJS = require_where.o + +PGFILEDESC = 'require_where - require DELETE and/or UPDATE to have a WHERE clause' + +REGRESS = require_where + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS = $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = contrib/require_where +top_builddir = ../.. +include $(top_builddir)/src/Makefile.global +include $(top_builddir)/contrib/contrib-global.mk +endif diff --git a/contrib/require_where/data/test_require_where.data b/contrib/require_where/data/test_require_where.data new file mode 100644 index 0000000..d4a29d8 --- /dev/null +++ b/contrib/require_where/data/test_require_where.data @@ -0,0 +1,16 @@ +Four +score +and +seven +years +ago +our +fathers +brought +forth +on +this +continent +a +new +nation diff --git a/contrib/require_where/expected/require_where.out b/contrib/require_where/expected/require_where.out new file mode 100644 index 0000000..adfd358 --- /dev/null +++ b/contrib/require_where/expected/require_where.out @@ -0,0 +1,22 @@ +-- +-- Test require_where +-- +\set echo all +LOAD 'require_where'; +CREATE TABLE test_require_where(t TEXT); +\copy test_require_where from 'data/test_require_where.data' +UPDATE test_require_where SET t=t; -- succeeds +SET require_where.update TO ON; +UPDATE test_require_where SET t=t; -- fails +ERROR: UPDATE requires a WHERE clause when require_where.delete is set to on +HINT: To update all rows, use "WHERE true" or similar. +UPDATE test_require_where SET t=t WHERE true; -- succeeds +SET require_where.update TO OFF; +UPDATE test_require_where SET t=t; -- succeeds +SET require_where.delete TO ON; +DELETE FROM test_require_where; -- fails +ERROR: DELETE requires a WHERE clause when require_where.delete is set to on +HINT: To delete all rows, use "WHERE true" or similar. +DELETE FROM test_require_where WHERE true; -- succeeds +SET require_where.delete TO OFF; +DELETE FROM test_require_where; -- succeeds diff --git a/contrib/require_where/require_where.c b/contrib/require_where/require_where.c new file mode 100644 index 0000000..27cbc25 --- /dev/null +++ b/contrib/require_where/require_where.c @@ -0,0 +1,92 @@ +/* + * -------------------------------------------------------------------------- + * + * require_where.c + * + * Copyright (C) 2016, PostgreSQL Global Development Group + * + * IDENTIFICATION + * contrib/require_where/require_where.c + * + * -------------------------------------------------------------------------- + */ +#include "postgres.h" + +#include "fmgr.h" + +#include "parser/analyze.h" + +#include "utils/elog.h" +#include "utils/guc.h" + +PG_MODULE_MAGIC; + +void _PG_init(void); +void _PG_fini(void); + +static post_parse_analyze_hook_type original_post_parse_analyze_hook = NULL; +static bool require_where_delete = false; +static bool require_where_update = false; + +static void +require_where_check(ParseState *pstate, Query *query) +{ + + if (require_where_delete && query->commandType == CMD_DELETE) + { + Assert(query->jointree != NULL); + if (query->jointree->quals == NULL) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("DELETE requires a WHERE clause when require_where.delete is set to on"), + errhint("To delete all rows, use \"WHERE true\" or similar."))); + } + + if (require_where_update && query->commandType == CMD_UPDATE) + { + Assert(query->jointree != NULL); + if (query->jointree->quals == NULL) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("UPDATE requires a WHERE clause when require_where.delete is set to on"), + errhint("To update all rows, use \"WHERE true\" or similar."))); + } + + if (original_post_parse_analyze_hook != NULL) + (*original_post_parse_analyze_hook) (pstate, query); +} + +void +_PG_init(void) +{ + DefineCustomBoolVariable("require_where.delete", + "Require every DELETE statement to have a WHERE clause.", + NULL, + &require_where_delete, + false, + PGC_USERSET, + 0, + NULL, + NULL, + NULL); + + DefineCustomBoolVariable("require_where.update", + "Require every UPDATE statement to have a WHERE clause.", + NULL, + &require_where_update, + false, + PGC_USERSET, + 0, + NULL, + NULL, + NULL); + + original_post_parse_analyze_hook = post_parse_analyze_hook; + post_parse_analyze_hook = require_where_check; +} + +void +_PG_fini(void) +{ + post_parse_analyze_hook = original_post_parse_analyze_hook; +} diff --git a/contrib/require_where/sql/require_where.sql b/contrib/require_where/sql/require_where.sql new file mode 100644 index 0000000..4d20b17 --- /dev/null +++ b/contrib/require_where/sql/require_where.sql @@ -0,0 +1,33 @@ +-- +-- Test require_where +-- + +\set echo all + +LOAD 'require_where'; + +CREATE TABLE test_require_where(t TEXT); + +\copy test_require_where from 'data/test_require_where.data' + +UPDATE test_require_where SET t=t; -- succeeds + +SET require_where.update TO ON; + +UPDATE test_require_where SET t=t; -- fails + +UPDATE test_require_where SET t=t WHERE true; -- succeeds + +SET require_where.update TO OFF; + +UPDATE test_require_where SET t=t; -- succeeds + +SET require_where.delete TO ON; + +DELETE FROM test_require_where; -- fails + +DELETE FROM test_require_where WHERE true; -- succeeds + +SET require_where.delete TO OFF; + +DELETE FROM test_require_where; -- succeeds diff --git a/doc/src/sgml/contrib.sgml b/doc/src/sgml/contrib.sgml index c8708ec..48ca717 100644 --- a/doc/src/sgml/contrib.sgml +++ b/doc/src/sgml/contrib.sgml @@ -135,6 +135,7 @@ CREATE EXTENSION <replaceable>module_name</> FROM unpackaged; &pgtrgm; &pgvisibility; &postgres-fdw; + &require-where; &seg; &sepgsql; &contrib-spi; diff --git a/doc/src/sgml/filelist.sgml b/doc/src/sgml/filelist.sgml index 69649a7..4552273 100644 --- a/doc/src/sgml/filelist.sgml +++ b/doc/src/sgml/filelist.sgml @@ -141,6 +141,7 @@ <!ENTITY pgtrgm SYSTEM "pgtrgm.sgml"> <!ENTITY pgvisibility SYSTEM "pgvisibility.sgml"> <!ENTITY postgres-fdw SYSTEM "postgres-fdw.sgml"> +<!ENTITY require-where SYSTEM "require_where.sgml"> <!ENTITY seg SYSTEM "seg.sgml"> <!ENTITY contrib-spi SYSTEM "contrib-spi.sgml"> <!ENTITY sepgsql SYSTEM "sepgsql.sgml"> diff --git a/doc/src/sgml/require_where.sgml b/doc/src/sgml/require_where.sgml new file mode 100644 index 0000000..d9f59c6 --- /dev/null +++ b/doc/src/sgml/require_where.sgml @@ -0,0 +1,98 @@ +<!-- doc/src/sgml/require_where.sgml --> + +<sect1 id="require-where" xreflabel="require_where"> + <title>require_where</title> + + <indexterm zone="require-where"> + <primary>require_where</primary> + </indexterm> + + <para> + This module allows a user to require that WHERE clauses on either DELETE or + UPDATE not be empty using a custom GUC to control each. + </para> + + <para> + To use this module, you need to include <literal>require_where</literal> in + the either the <xref linkend="guc-shared-preload-libraries"> parameter, or + <command>LOAD</command> the library directly. + </para> + + <para> + Here is an example showing how to set up a database cluster with + <literal>require_where</literal>. +<screen> +$ psql -U postgres +# SHOW shared_preload_libraries; /* Make sure not to clobber something by accident */ + +If you found something, +# ALTER SYSTEM SET shared_preload_libraries='the,stuff,you,found,require_where'; + +Otherwise, +# ALTER SYSTEM SET shared_preload_libraries='require_where'; + +Then restart <productname>PostgreSQL</productname> +</screen> + </para> + + <para> + Here is an example using <command>LOAD</command>: +<screen> +$ psql -U postgres +# LOAD '$libdir/require_where'; +</screen> + </para> + + <sect2> + <title>GUC Parameters</title> + + <variablelist> + <varlistentry id="guc-require-where-delete" xreflabel="require_where.delete"> + <term> + <varname>require_where.delete</varname> (<type>boolean</type>) + <indexterm> + <primary><varname>require_where.delete</varname> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + This parameter defaults to <literal>false</literal>. When set + to <literal>true</literal>, every <literal>DELETE</literal> requires + some kind of <literal>WHERE</literal> clause in order to + execute. When <literal>DELETE</literal> is meant to span the + entire relation, one can issue a <literal>WHERE true</literal> + or equivalent. + </para> + </listitem> + </varlistentry> + <varlistentry id="guc-require-where-update" xreflabel="require_where.update"> + <term> + <varname>require_where.update</varname> (<type>boolean</type>) + <indexterm> + <primary><varname>require_where.update</varname> configuration parameter</primary> + </indexterm> + </term> + <listitem> + <para> + This parameter defaults to <literal>false</literal>. When set + to <literal>true</literal>, every <literal>UPDATE</literal> requires + some kind of <literal>WHERE</literal> clause in order to + execute. When <literal>UPDATE</literal> is meant to span the + entire relation, one can issue a <literal>WHERE true</literal> + or equivalent. + </para> + </listitem> + </varlistentry> + </variablelist> + </sect2> + + <sect2> + <title>Authors</title> + + <para> + David Fetter <email>da...@fetter.org</email>, + Robert Haas <email>robertmh...@gmail.com</email> and + Andrew Gierth <email>and...@tao11.riddles.org.uk</email>. + </para> + </sect2> +</sect1>
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers