On Fri, Sep 09, 2016 at 09:57:21AM -0400, Peter Eisentraut wrote:
> Review of the patch in the commit fest:
>
> - Various naming/spelling inconsistencies: In the source, the module
> is require_where, the documentation titles it require-where, the GUC
> parameters are requires_where.*, but incorrectly documented.
Fixed.
> - Unusual indentation in the Makefile
Fixed.
> - Needs tests
Still needs some fixing.
> - Not sure about errcode(ERRCODE_CARDINALITY_VIOLATION), which is
> documented in the code as "this means something returned the wrong
> number of rows". I think ERRCODE_SYNTAX_ERROR or something from
> nearby there would be better.
Changed to ERRCODE_SYNTAX_ERROR. CARDINALITY_VIOLATION was a bit too
cute.
> - errhint() string should end with a period.
Fixed.
> - The 7th argument of DefineCustomBoolVariable() is of type int, not
> bool, so passing false is somewhat wrong, even if it works.
Fixed.
> - There ought to be a _PG_fini() function that undoes what _PG_init()
> does.
Fixed.
> - The documentation should be expanded and clarified. Given that this
> is a "training wheels" module, we can be extra clear here. I would
> like to see some examples at least.
Working on this.
> - The documentation is a bit incorrect about the ways to load this
> module. shared_preload_libraries is not necessary. session_ and
> local_ (with prep) should also work.
I'm not 100% sure I understand what you want here. I did manage to
get the thing loaded without a restart via LOAD, but that's it so far.
Will continue to poke at it.
> - The claim in the documentation that only superusers can do things
> with this module is not generally correct.
I think that the claims are fixed. This is SUSET, at least in this
patch, because anything short of that that changes query behavior
seems incautious.
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..0cf3663
--- /dev/null
+++ b/contrib/require_where/Makefile
@@ -0,0 +1,17 @@
+# 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'
+
+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/require_where.c
b/contrib/require_where/require_where.c
new file mode 100644
index 0000000..181b3bb
--- /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"),
+ 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"),
+ 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_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+
+ DefineCustomBoolVariable("require_where.update",
+ "Require every UPDATE
statement to have a WHERE clause.",
+ NULL,
+ &require_where_update,
+ false,
+ PGC_SUSET,
+ 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/doc/src/sgml/contrib.sgml b/doc/src/sgml/contrib.sgml
index c8708ec..7ca62a4 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 4383711..54b6bdc 100644
--- a/doc/src/sgml/filelist.sgml
+++ b/doc/src/sgml/filelist.sgml
@@ -140,6 +140,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..0b2e290
--- /dev/null
+++ b/doc/src/sgml/require_where.sgml
@@ -0,0 +1,89 @@
+<!-- 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 superuser 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 must include <literal>require_where</literal>
+ in the <xref linkend="guc-shared-preload-libraries"> parameter.
+ </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>
+
+
+ <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>, Oakland, California, USA
+ </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
