On Sun, Jan 01, 2017 at 07:57:33PM +0900, Michael Paquier wrote:
> 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.
> 
> 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.

Done.

> --- /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.

Removed.

> 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.

Please find attached the next version of the patch including Pavel's
suggestion that I provide some motivation for including those
Asserts.

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..933eb00
--- /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 simple DELETEs and UPDATEs 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/expected/require_where.out 
b/contrib/require_where/expected/require_where.out
new file mode 100644
index 0000000..a36dd1f
--- /dev/null
+++ b/contrib/require_where/expected/require_where.out
@@ -0,0 +1,16 @@
+--
+--   Test require_where
+--
+\set echo all
+CREATE TABLE test_require_where(t TEXT);
+UPDATE test_require_where SET t=t; -- succeeds
+DELETE FROM test_require_where; -- succeeds
+LOAD 'require_where';
+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
+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
diff --git a/contrib/require_where/require_where.c 
b/contrib/require_where/require_where.c
new file mode 100644
index 0000000..09f2578
--- /dev/null
+++ b/contrib/require_where/require_where.c
@@ -0,0 +1,73 @@
+/*
+ * --------------------------------------------------------------------------
+ *
+ * require_where.c
+ *
+ * Copyright (C) 2017, 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;
+
+/*
+ * This module makes simple UPDATE and DELETE statements require a WHERE clause
+ * and complains when this is not present.
+ */
+static void
+require_where_check(ParseState *pstate, Query *query)
+{
+
+       if (query->commandType == CMD_DELETE)
+       {
+               /* Make sure there's something to look at. */
+               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 (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)
+{
+       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..db8e078
--- /dev/null
+++ b/contrib/require_where/sql/require_where.sql
@@ -0,0 +1,21 @@
+--
+--   Test require_where
+--
+
+\set echo all
+
+CREATE TABLE test_require_where(t TEXT);
+
+UPDATE test_require_where SET t=t; -- succeeds
+
+DELETE FROM test_require_where; -- succeeds
+
+LOAD 'require_where';
+
+UPDATE test_require_where SET t=t; -- fails
+
+UPDATE test_require_where SET t=t WHERE true; -- succeeds
+
+DELETE FROM test_require_where; -- fails
+
+DELETE FROM test_require_where WHERE true; -- 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..290ce7b
--- /dev/null
+++ b/doc/src/sgml/require_where.sgml
@@ -0,0 +1,54 @@
+<!-- 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 makes it a requirement that WHERE on both DELETE or UPDATE be 
present.
+ </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>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

Reply via email to