Make foo=null a warning by default.

From 33b9f51b1374ede71a6479d44adfa0588c7fb4c3 Mon Sep 17 00:00:00 2001
From: David Fetter <david@fetter.org>
Date: Sun, 15 Jul 2018 16:11:08 -0700
Subject: [PATCH] Make foo=null a warning by default v2
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.17.1"

This is a multi-part message in MIME format.
--------------2.17.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


The transform_null_equals GUC is now an enum consisting of warn, error, off, on.
---
 doc/src/sgml/config.sgml                      | 20 ++++++---
 src/backend/parser/parse_expr.c               | 30 ++++++++++---
 src/backend/utils/misc/guc.c                  | 44 +++++++++++++------
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 src/include/parser/parse_expr.h               | 11 ++++-
 src/test/regress/expected/guc.out             | 30 +++++++++++++
 src/test/regress/expected/inherit.out         |  1 +
 src/test/regress/sql/guc.sql                  | 18 ++++++++
 8 files changed, 129 insertions(+), 27 deletions(-)


--------------2.17.1
Content-Type: text/x-patch; name="0001-Make-foo-null-a-warning-by-default.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Make-foo-null-a-warning-by-default.patch"

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e307bb4e8e..62cccc6a2d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8036,7 +8036,7 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
      <variablelist>

      <varlistentry id="guc-transform-null-equals" xreflabel="transform_null_equals">
-      <term><varname>transform_null_equals</varname> (<type>boolean</type>)
+      <term><varname>transform_null_equals</varname> (<type>enum</type>)
       <indexterm><primary>IS NULL</primary></indexterm>
       <indexterm>
        <primary><varname>transform_null_equals</varname> configuration parameter</primary>
@@ -8044,15 +8044,23 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </term>
       <listitem>
        <para>
-        When on, expressions of the form <literal><replaceable>expr</replaceable> =
+        When enabled, expressions of the form <literal><replaceable>expr</replaceable> =
         NULL</literal> (or <literal>NULL =
         <replaceable>expr</replaceable></literal>) are treated as
         <literal><replaceable>expr</replaceable> IS NULL</literal>, that is, they
         return true if <replaceable>expr</replaceable> evaluates to the null value,
-        and false otherwise. The correct SQL-spec-compliant behavior of
-        <literal><replaceable>expr</replaceable> = NULL</literal> is to always
-        return null (unknown). Therefore this parameter defaults to
-        <literal>off</literal>.
+        and false otherwise.  Valid values are <literal>warn</literal>, <literal>error</literal>, <literal>off</literal>, and <literal>on</literal>.
+       </para>
+
+       <para>
+        The correct SQL-spec-compliant behavior of
+        <literal><replaceable>expr</replaceable> = <replacable>null_expression</replaceable></literal>
+        is to always return null (unknown); PostgreSQL allows <literal>NULL</literal>
+        as a null-valued expression of unknown type, which is an extension to the spec.
+        The transformation of <literal><replaceable>expr</replaceable> = NULL</literal>
+        to <literal><replaceable>expr</replaceable> IS NULL</literal> is inconsistent
+        with the normal semantics of the SQL specification, and is therefore set to "warn"
+        by default.
        </para>

        <para>
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 385e54a9b6..58e3e07382 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -41,8 +41,8 @@


 /* GUC parameters */
-bool		operator_precedence_warning = false;
-bool		Transform_null_equals = false;
+bool	operator_precedence_warning = false;
+enum	TransformNullEquals transform_null_equals = TRANSFORM_NULL_EQUALS_WARN;

 /*
  * Node-type groups for operator precedence warnings
@@ -850,6 +850,7 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 	Node	   *lexpr = a->lexpr;
 	Node	   *rexpr = a->rexpr;
 	Node	   *result;
+	bool		need_transform_null_equals = false;

 	if (operator_precedence_warning)
 	{
@@ -872,17 +873,34 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 	}

 	/*
-	 * Special-case "foo = NULL" and "NULL = foo" for compatibility with
+	 * Deal with "foo = NULL" and "NULL = foo" for compatibility with
 	 * standards-broken products (like Microsoft's).  Turn these into IS NULL
 	 * exprs. (If either side is a CaseTestExpr, then the expression was
 	 * generated internally from a CASE-WHEN expression, and
 	 * transform_null_equals does not apply.)
 	 */
-	if (Transform_null_equals &&
-		list_length(a->name) == 1 &&
+	need_transform_null_equals = (list_length(a->name) == 1 &&
 		strcmp(strVal(linitial(a->name)), "=") == 0 &&
 		(exprIsNullConstant(lexpr) || exprIsNullConstant(rexpr)) &&
-		(!IsA(lexpr, CaseTestExpr) &&!IsA(rexpr, CaseTestExpr)))
+		(!IsA(lexpr, CaseTestExpr) &&!IsA(rexpr, CaseTestExpr)));
+
+	if (need_transform_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_WARN)
+		ereport(WARNING,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("= NULL can only produce a NULL"),
+				 parser_errposition(pstate, a->location)));
+
+	if (need_transform_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_ERR)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("= NULL does not comply with the SQL specification"),
+				 parser_errposition(pstate, a->location)));
+
+	/*
+	 * We don't need to handle TRANSFORM_NULL_EQUALS_OFF here because it's a noop
+	 */
+
+	if (need_transform_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_ON)
 	{
 		NullTest   *n = makeNode(NullTest);

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 17292e04fe..3f7f5c9b74 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -310,6 +310,23 @@ static const struct config_enum_entry track_function_options[] = {
 	{NULL, 0, false}
 };

+/*
+ * Accept the usual variants of boolean-ish, along with warn and error.
+ */
+static const struct config_enum_entry transform_null_equals_options[] = {
+	{"warn", TRANSFORM_NULL_EQUALS_WARN, false},
+	{"error", TRANSFORM_NULL_EQUALS_ERR, false},
+	{"off", TRANSFORM_NULL_EQUALS_OFF, false},
+	{"false", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"no", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"0", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"on", TRANSFORM_NULL_EQUALS_ON, false},
+	{"true", TRANSFORM_NULL_EQUALS_ON, true},
+	{"yes", TRANSFORM_NULL_EQUALS_ON, true},
+	{"1", TRANSFORM_NULL_EQUALS_ON, true},
+	{NULL, 0, false}
+};
+
 static const struct config_enum_entry xmlbinary_options[] = {
 	{"base64", XMLBINARY_BASE64, false},
 	{"hex", XMLBINARY_HEX, false},
@@ -1407,19 +1424,6 @@ static struct config_bool ConfigureNamesBool[] =
 		false,
 		NULL, NULL, NULL
 	},
-	{
-		{"transform_null_equals", PGC_USERSET, COMPAT_OPTIONS_CLIENT,
-			gettext_noop("Treats \"expr=NULL\" as \"expr IS NULL\"."),
-			gettext_noop("When turned on, expressions of the form expr = NULL "
-						 "(or NULL = expr) are treated as expr IS NULL, that is, they "
-						 "return true if expr evaluates to the null value, and false "
-						 "otherwise. The correct behavior of expr = NULL is to always "
-						 "return null (unknown).")
-		},
-		&Transform_null_equals,
-		false,
-		NULL, NULL, NULL
-	},
 	{
 		{"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH,
 			gettext_noop("Enables per-database user names."),
@@ -4067,6 +4071,20 @@ static struct config_enum ConfigureNamesEnum[] =
 		NULL, NULL, NULL
 	},

+	{
+		{"transform_null_equals", PGC_USERSET, COMPAT_OPTIONS_CLIENT,
+			gettext_noop("Treats \"expr=NULL\" as \"expr IS NULL\"."),
+			gettext_noop("When set to on, expressions of the form expr = NULL "
+						 "(or NULL = expr) are treated as expr IS NULL. When "
+						 "set to off, warn, or error, they are not, with increasing "
+						 "insistence. The default behavior of expr = NULL is to "
+						 "return null (unknown) and issue a warning.")
+		},
+		&transform_null_equals,
+		TRANSFORM_NULL_EQUALS_WARN, transform_null_equals_options,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"wal_level", PGC_POSTMASTER, WAL_SETTINGS,
 			gettext_noop("Set the level of information written to the WAL."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 657c3f81f8..a498c3aebc 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -655,7 +655,7 @@

 # - Other Platforms and Clients -

-#transform_null_equals = off
+#transform_null_equals = warn	# warn, error, off, or on


 #------------------------------------------------------------------------------
diff --git a/src/include/parser/parse_expr.h b/src/include/parser/parse_expr.h
index e5aff61b8f..276a631daf 100644
--- a/src/include/parser/parse_expr.h
+++ b/src/include/parser/parse_expr.h
@@ -17,10 +17,19 @@

 /* GUC parameters */
 extern bool operator_precedence_warning;
-extern bool Transform_null_equals;

 extern Node *transformExpr(ParseState *pstate, Node *expr, ParseExprKind exprKind);

 extern const char *ParseExprKindName(ParseExprKind exprKind);

+typedef enum TransformNullEquals
+{
+	TRANSFORM_NULL_EQUALS_WARN = 0,	/* Issue a warning */
+	TRANSFORM_NULL_EQUALS_ERR,		/* Error out */
+	TRANSFORM_NULL_EQUALS_OFF,		/* Disabled */
+	TRANSFORM_NULL_EQUALS_ON		/* Enabled */
+} TransformNullEquals;
+
+extern TransformNullEquals transform_null_equals;
+
 #endif							/* PARSE_EXPR_H */
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 43ac5f5f11..561b1b6d96 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -767,3 +767,33 @@ NOTICE:  text search configuration "no_such_config" does not exist
 select func_with_bad_set();
 ERROR:  invalid value for parameter "default_text_search_config": "no_such_config"
 reset check_function_bodies;
+set transform_null_equals = on;
+select 1=null;
+ ?column?
+----------
+ f
+(1 row)
+
+set transform_null_equals = off;
+select 1=null;
+ ?column?
+----------
+
+(1 row)
+
+set transform_null_equals = warn;
+select 1=null;
+WARNING:  = NULL can only produce a NULL
+LINE 1: select 1=null;
+                ^
+ ?column?
+----------
+
+(1 row)
+
+set transform_null_equals = error;
+select 1=null;
+ERROR:  = NULL does not comply with the SQL specification
+LINE 1: select 1=null;
+                ^
+reset transform_null_equals;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 4f29d9f891..afbf7cb37b 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1690,6 +1690,7 @@ reset enable_bitmapscan;
 --
 create table cnullparent (f1 int);
 create table cnullchild (check (f1 = 1 or f1 = null)) inherits(cnullparent);
+WARNING:  = NULL can only produce a NULL
 insert into cnullchild values(1);
 insert into cnullchild values(2);
 insert into cnullchild values(null);
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index 23e5029780..29b8c888e2 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -288,3 +288,21 @@ set default_text_search_config = no_such_config;
 select func_with_bad_set();

 reset check_function_bodies;
+
+set transform_null_equals = on;
+
+select 1=null;
+
+set transform_null_equals = off;
+
+select 1=null;
+
+set transform_null_equals = warn;
+
+select 1=null;
+
+set transform_null_equals = error;
+
+select 1=null;
+
+reset transform_null_equals;

--------------2.17.1--

From 87d004f2a054a0c0999145aa9b44d97d7951b19c Mon Sep 17 00:00:00 2001
From: David Fetter <david@fetter.org>
Date: Sun, 15 Jul 2018 16:11:08 -0700
Subject: [PATCH] Make transform_null_equals into an enum
To: pgsql-hackers@postgresql.org

The transform_null_equals GUC can now be off, warn, error, or on.
---
 doc/src/sgml/config.sgml                      | 20 ++++++---
 src/backend/parser/parse_expr.c               | 30 ++++++++++---
 src/backend/utils/misc/guc.c                  | 44 +++++++++++++------
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 src/include/parser/parse_expr.h               | 11 ++++-
 src/test/regress/expected/guc.out             | 30 +++++++++++++
 src/test/regress/sql/guc.sql                  | 18 ++++++++
 7 files changed, 128 insertions(+), 27 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4d48d93305..b0d951190f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8066,7 +8066,7 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
      <variablelist>
 
      <varlistentry id="guc-transform-null-equals" xreflabel="transform_null_equals">
-      <term><varname>transform_null_equals</varname> (<type>boolean</type>)
+      <term><varname>transform_null_equals</varname> (<type>enum</type>)
       <indexterm><primary>IS NULL</primary></indexterm>
       <indexterm>
        <primary><varname>transform_null_equals</varname> configuration parameter</primary>
@@ -8074,15 +8074,23 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </term>
       <listitem>
        <para>
-        When on, expressions of the form <literal><replaceable>expr</replaceable> =
+        When enabled, expressions of the form <literal><replaceable>expr</replaceable> =
         NULL</literal> (or <literal>NULL =
         <replaceable>expr</replaceable></literal>) are treated as
         <literal><replaceable>expr</replaceable> IS NULL</literal>, that is, they
         return true if <replaceable>expr</replaceable> evaluates to the null value,
-        and false otherwise. The correct SQL-spec-compliant behavior of
-        <literal><replaceable>expr</replaceable> = NULL</literal> is to always
-        return null (unknown). Therefore this parameter defaults to
-        <literal>off</literal>.
+        and false otherwise.  Valid values are <literal>off</literal>, <literal>warn</literal>, <literal>error</literal>, and <literal>on</literal>.
+       </para>
+
+       <para>
+        The correct SQL-spec-compliant behavior of
+        <literal><replaceable>expr</replaceable> = <replacable>null_expression</replaceable></literal>
+        is to always return null (unknown); PostgreSQL allows <literal>NULL</literal>
+        as a null-valued expression of unknown type, which is an extension to the spec.
+        The transformation of <literal><replaceable>expr</replaceable> = NULL</literal>
+        to <literal><replaceable>expr</replaceable> IS NULL</literal> is inconsistent
+        with the normal semantics of the SQL specification, and is therefore set to "off"
+        by default.
        </para>
 
        <para>
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 385e54a9b6..4ee31b3cb3 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -41,8 +41,8 @@
 
 
 /* GUC parameters */
-bool		operator_precedence_warning = false;
-bool		Transform_null_equals = false;
+bool	operator_precedence_warning = false;
+enum	TransformNullEquals transform_null_equals = TRANSFORM_NULL_EQUALS_OFF;
 
 /*
  * Node-type groups for operator precedence warnings
@@ -850,6 +850,7 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 	Node	   *lexpr = a->lexpr;
 	Node	   *rexpr = a->rexpr;
 	Node	   *result;
+	bool		need_transform_null_equals = false;
 
 	if (operator_precedence_warning)
 	{
@@ -872,17 +873,34 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 	}
 
 	/*
-	 * Special-case "foo = NULL" and "NULL = foo" for compatibility with
+	 * Deal with "foo = NULL" and "NULL = foo" for compatibility with
 	 * standards-broken products (like Microsoft's).  Turn these into IS NULL
 	 * exprs. (If either side is a CaseTestExpr, then the expression was
 	 * generated internally from a CASE-WHEN expression, and
 	 * transform_null_equals does not apply.)
 	 */
-	if (Transform_null_equals &&
-		list_length(a->name) == 1 &&
+	need_transform_null_equals = (list_length(a->name) == 1 &&
 		strcmp(strVal(linitial(a->name)), "=") == 0 &&
 		(exprIsNullConstant(lexpr) || exprIsNullConstant(rexpr)) &&
-		(!IsA(lexpr, CaseTestExpr) &&!IsA(rexpr, CaseTestExpr)))
+		(!IsA(lexpr, CaseTestExpr) &&!IsA(rexpr, CaseTestExpr)));
+
+	/*
+	 * We don't need to handle TRANSFORM_NULL_EQUALS_OFF here because it's a noop
+	 */
+
+	if (need_transform_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_WARN)
+		ereport(WARNING,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("= NULL can only produce a NULL"),
+				 parser_errposition(pstate, a->location)));
+
+	if (need_transform_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_ERR)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("= NULL does not comply with the SQL specification"),
+				 parser_errposition(pstate, a->location)));
+
+	if (need_transform_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_ON)
 	{
 		NullTest   *n = makeNode(NullTest);
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a88ea6cfc9..d441bccb10 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -310,6 +310,23 @@ static const struct config_enum_entry track_function_options[] = {
 	{NULL, 0, false}
 };
 
+/*
+ * Accept the usual variants of boolean-ish, along with warn and error.
+ */
+static const struct config_enum_entry transform_null_equals_options[] = {
+	{"off", TRANSFORM_NULL_EQUALS_OFF, false},
+	{"false", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"no", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"0", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"warn", TRANSFORM_NULL_EQUALS_WARN, false},
+	{"error", TRANSFORM_NULL_EQUALS_ERR, false},
+	{"on", TRANSFORM_NULL_EQUALS_ON, false},
+	{"true", TRANSFORM_NULL_EQUALS_ON, true},
+	{"yes", TRANSFORM_NULL_EQUALS_ON, true},
+	{"1", TRANSFORM_NULL_EQUALS_ON, true},
+	{NULL, 0, false}
+};
+
 static const struct config_enum_entry xmlbinary_options[] = {
 	{"base64", XMLBINARY_BASE64, false},
 	{"hex", XMLBINARY_HEX, false},
@@ -1414,19 +1431,6 @@ static struct config_bool ConfigureNamesBool[] =
 		false,
 		NULL, NULL, NULL
 	},
-	{
-		{"transform_null_equals", PGC_USERSET, COMPAT_OPTIONS_CLIENT,
-			gettext_noop("Treats \"expr=NULL\" as \"expr IS NULL\"."),
-			gettext_noop("When turned on, expressions of the form expr = NULL "
-						 "(or NULL = expr) are treated as expr IS NULL, that is, they "
-						 "return true if expr evaluates to the null value, and false "
-						 "otherwise. The correct behavior of expr = NULL is to always "
-						 "return null (unknown).")
-		},
-		&Transform_null_equals,
-		false,
-		NULL, NULL, NULL
-	},
 	{
 		{"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH,
 			gettext_noop("Enables per-database user names."),
@@ -4074,6 +4078,20 @@ static struct config_enum ConfigureNamesEnum[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"transform_null_equals", PGC_USERSET, COMPAT_OPTIONS_CLIENT,
+			gettext_noop("Treats \"expr=NULL\" as \"expr IS NULL\"."),
+			gettext_noop("When set to on, expressions of the form expr = NULL "
+						 "(or NULL = expr) are treated as expr IS NULL. When "
+						 "set to off, warn, or error, they are not, with increasing "
+						 "insistence. The default behavior of expr = NULL is to "
+						 "return null (unknown).")
+		},
+		&transform_null_equals,
+		TRANSFORM_NULL_EQUALS_OFF, transform_null_equals_options,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"wal_level", PGC_POSTMASTER, WAL_SETTINGS,
 			gettext_noop("Set the level of information written to the WAL."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index c0d3fb8491..07dda7b746 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -656,7 +656,7 @@
 
 # - Other Platforms and Clients -
 
-#transform_null_equals = off
+#transform_null_equals = off	# off, warn, error, or on
 
 
 #------------------------------------------------------------------------------
diff --git a/src/include/parser/parse_expr.h b/src/include/parser/parse_expr.h
index e5aff61b8f..2f3dc28858 100644
--- a/src/include/parser/parse_expr.h
+++ b/src/include/parser/parse_expr.h
@@ -17,10 +17,19 @@
 
 /* GUC parameters */
 extern bool operator_precedence_warning;
-extern bool Transform_null_equals;
 
 extern Node *transformExpr(ParseState *pstate, Node *expr, ParseExprKind exprKind);
 
 extern const char *ParseExprKindName(ParseExprKind exprKind);
 
+typedef enum TransformNullEquals
+{
+	TRANSFORM_NULL_EQUALS_OFF = 0,	/* Disabled */
+	TRANSFORM_NULL_EQUALS_WARN,		/* Issue a warning */
+	TRANSFORM_NULL_EQUALS_ERR,		/* Error out */
+	TRANSFORM_NULL_EQUALS_ON		/* Enabled */
+} TransformNullEquals;
+
+extern TransformNullEquals transform_null_equals;
+
 #endif							/* PARSE_EXPR_H */
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 43ac5f5f11..f50a1f153c 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -767,3 +767,33 @@ NOTICE:  text search configuration "no_such_config" does not exist
 select func_with_bad_set();
 ERROR:  invalid value for parameter "default_text_search_config": "no_such_config"
 reset check_function_bodies;
+set transform_null_equals = off;
+select 1=null as transform_null_equals_off;
+ transform_null_equals_off 
+---------------------------
+ 
+(1 row)
+
+set transform_null_equals = warn;
+select 1=null as transform_null_equals_warn;
+WARNING:  = NULL can only produce a NULL
+LINE 1: select 1=null as transform_null_equals_warn;
+                ^
+ transform_null_equals_warn 
+----------------------------
+ 
+(1 row)
+
+set transform_null_equals = error;
+select 1=null as transform_null_equals_error;
+ERROR:  = NULL does not comply with the SQL specification
+LINE 1: select 1=null as transform_null_equals_error;
+                ^
+set transform_null_equals = on;
+select 1=null as transform_null_equals_on;
+ transform_null_equals_on 
+--------------------------
+ f
+(1 row)
+
+reset transform_null_equals;
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index 23e5029780..4b76e75048 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -288,3 +288,21 @@ set default_text_search_config = no_such_config;
 select func_with_bad_set();
 
 reset check_function_bodies;
+
+set transform_null_equals = off;
+
+select 1=null as transform_null_equals_off;
+
+set transform_null_equals = warn;
+
+select 1=null as transform_null_equals_warn;
+
+set transform_null_equals = error;
+
+select 1=null as transform_null_equals_error;
+
+set transform_null_equals = on;
+
+select 1=null as transform_null_equals_on;
+
+reset transform_null_equals;
-- 
2.17.1

From 1c0d16c79dba628ec10fd1554574cf497b3dfffb Mon Sep 17 00:00:00 2001
From: David Fetter <david@fetter.org>
Date: Sun, 15 Jul 2018 16:11:08 -0700
Subject: [PATCH] Make transform_null_equals into an enum v4
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.17.1"

This is a multi-part message in MIME format.
--------------2.17.1
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit


The transform_null_equals GUC can now be off, warn, error, or on.
---
 doc/src/sgml/config.sgml                      | 25 ++++++---
 src/backend/parser/parse_expr.c               | 30 ++++++++--
 src/backend/utils/misc/guc.c                  | 44 ++++++++++-----
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 src/include/parser/parse_expr.h               | 11 +++-
 src/test/regress/expected/guc.out             | 55 +++++++++++++++++++
 src/test/regress/sql/guc.sql                  | 26 +++++++++
 7 files changed, 164 insertions(+), 29 deletions(-)


--------------2.17.1
Content-Type: text/x-patch; name="0001-Make-transform_null_equals-into-an-enum-v4.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-Make-transform_null_equals-into-an-enum-v4.patch"

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4d48d93305..6468c2253f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8066,7 +8066,7 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
      <variablelist>
 
      <varlistentry id="guc-transform-null-equals" xreflabel="transform_null_equals">
-      <term><varname>transform_null_equals</varname> (<type>boolean</type>)
+      <term><varname>transform_null_equals</varname> (<type>enum</type>)
       <indexterm><primary>IS NULL</primary></indexterm>
       <indexterm>
        <primary><varname>transform_null_equals</varname> configuration parameter</primary>
@@ -8074,15 +8074,24 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </term>
       <listitem>
        <para>
-        When on, expressions of the form <literal><replaceable>expr</replaceable> =
-        NULL</literal> (or <literal>NULL =
-        <replaceable>expr</replaceable></literal>) are treated as
+        When enabled, expressions of the form
+        <literal><replaceable>expr</replaceable> = NULL</literal> (or
+        <literal>NULL = <replaceable>expr</replaceable></literal>) are treated as
         <literal><replaceable>expr</replaceable> IS NULL</literal>, that is, they
         return true if <replaceable>expr</replaceable> evaluates to the null value,
-        and false otherwise. The correct SQL-spec-compliant behavior of
-        <literal><replaceable>expr</replaceable> = NULL</literal> is to always
-        return null (unknown). Therefore this parameter defaults to
-        <literal>off</literal>.
+        and false otherwise.  Valid values are <literal>off</literal>,
+        <literal>warn</literal>, <literal>error</literal>, and <literal>on</literal>.
+       </para>
+
+       <para>
+        The correct SQL-spec-compliant behavior of
+        <literal><replaceable>expr</replaceable> = <replacable>null_expression</replaceable></literal>
+        is to always return null (unknown); PostgreSQL allows <literal>NULL</literal>
+        as a null-valued expression of unknown type, which is an extension to the spec.
+        The transformation of <literal><replaceable>expr</replaceable> = NULL</literal>
+        to <literal><replaceable>expr</replaceable> IS NULL</literal> is inconsistent
+        with the normal semantics of the SQL specification, and is therefore set to "off"
+        by default.
        </para>
 
        <para>
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 385e54a9b6..53970a8ce6 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -41,8 +41,8 @@
 
 
 /* GUC parameters */
-bool		operator_precedence_warning = false;
-bool		Transform_null_equals = false;
+bool	operator_precedence_warning = false;
+enum	TransformNullEquals transform_null_equals = TRANSFORM_NULL_EQUALS_OFF;
 
 /*
  * Node-type groups for operator precedence warnings
@@ -850,6 +850,7 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 	Node	   *lexpr = a->lexpr;
 	Node	   *rexpr = a->rexpr;
 	Node	   *result;
+	bool		is_null_equals = false;
 
 	if (operator_precedence_warning)
 	{
@@ -872,17 +873,34 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 	}
 
 	/*
-	 * Special-case "foo = NULL" and "NULL = foo" for compatibility with
+	 * Deal with "foo = NULL" and "NULL = foo" for compatibility with
 	 * standards-broken products (like Microsoft's).  Turn these into IS NULL
 	 * exprs. (If either side is a CaseTestExpr, then the expression was
 	 * generated internally from a CASE-WHEN expression, and
 	 * transform_null_equals does not apply.)
 	 */
-	if (Transform_null_equals &&
-		list_length(a->name) == 1 &&
+	is_null_equals = (list_length(a->name) == 1 &&
 		strcmp(strVal(linitial(a->name)), "=") == 0 &&
 		(exprIsNullConstant(lexpr) || exprIsNullConstant(rexpr)) &&
-		(!IsA(lexpr, CaseTestExpr) &&!IsA(rexpr, CaseTestExpr)))
+		(!IsA(lexpr, CaseTestExpr) &&!IsA(rexpr, CaseTestExpr)));
+
+	/*
+	 * We don't need to handle TRANSFORM_NULL_EQUALS_OFF here because it's a noop
+	 */
+
+	if (is_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_WARN)
+		ereport(WARNING,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("= NULL can only produce a NULL"),
+				 parser_errposition(pstate, a->location)));
+
+	if (is_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_ERR)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("= NULL does not comply with the SQL specification"),
+				 parser_errposition(pstate, a->location)));
+
+	if (is_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_ON)
 	{
 		NullTest   *n = makeNode(NullTest);
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a88ea6cfc9..d441bccb10 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -310,6 +310,23 @@ static const struct config_enum_entry track_function_options[] = {
 	{NULL, 0, false}
 };
 
+/*
+ * Accept the usual variants of boolean-ish, along with warn and error.
+ */
+static const struct config_enum_entry transform_null_equals_options[] = {
+	{"off", TRANSFORM_NULL_EQUALS_OFF, false},
+	{"false", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"no", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"0", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"warn", TRANSFORM_NULL_EQUALS_WARN, false},
+	{"error", TRANSFORM_NULL_EQUALS_ERR, false},
+	{"on", TRANSFORM_NULL_EQUALS_ON, false},
+	{"true", TRANSFORM_NULL_EQUALS_ON, true},
+	{"yes", TRANSFORM_NULL_EQUALS_ON, true},
+	{"1", TRANSFORM_NULL_EQUALS_ON, true},
+	{NULL, 0, false}
+};
+
 static const struct config_enum_entry xmlbinary_options[] = {
 	{"base64", XMLBINARY_BASE64, false},
 	{"hex", XMLBINARY_HEX, false},
@@ -1414,19 +1431,6 @@ static struct config_bool ConfigureNamesBool[] =
 		false,
 		NULL, NULL, NULL
 	},
-	{
-		{"transform_null_equals", PGC_USERSET, COMPAT_OPTIONS_CLIENT,
-			gettext_noop("Treats \"expr=NULL\" as \"expr IS NULL\"."),
-			gettext_noop("When turned on, expressions of the form expr = NULL "
-						 "(or NULL = expr) are treated as expr IS NULL, that is, they "
-						 "return true if expr evaluates to the null value, and false "
-						 "otherwise. The correct behavior of expr = NULL is to always "
-						 "return null (unknown).")
-		},
-		&Transform_null_equals,
-		false,
-		NULL, NULL, NULL
-	},
 	{
 		{"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH,
 			gettext_noop("Enables per-database user names."),
@@ -4074,6 +4078,20 @@ static struct config_enum ConfigureNamesEnum[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"transform_null_equals", PGC_USERSET, COMPAT_OPTIONS_CLIENT,
+			gettext_noop("Treats \"expr=NULL\" as \"expr IS NULL\"."),
+			gettext_noop("When set to on, expressions of the form expr = NULL "
+						 "(or NULL = expr) are treated as expr IS NULL. When "
+						 "set to off, warn, or error, they are not, with increasing "
+						 "insistence. The default behavior of expr = NULL is to "
+						 "return null (unknown).")
+		},
+		&transform_null_equals,
+		TRANSFORM_NULL_EQUALS_OFF, transform_null_equals_options,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"wal_level", PGC_POSTMASTER, WAL_SETTINGS,
 			gettext_noop("Set the level of information written to the WAL."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index c0d3fb8491..07dda7b746 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -656,7 +656,7 @@
 
 # - Other Platforms and Clients -
 
-#transform_null_equals = off
+#transform_null_equals = off	# off, warn, error, or on
 
 
 #------------------------------------------------------------------------------
diff --git a/src/include/parser/parse_expr.h b/src/include/parser/parse_expr.h
index e5aff61b8f..2f3dc28858 100644
--- a/src/include/parser/parse_expr.h
+++ b/src/include/parser/parse_expr.h
@@ -17,10 +17,19 @@
 
 /* GUC parameters */
 extern bool operator_precedence_warning;
-extern bool Transform_null_equals;
 
 extern Node *transformExpr(ParseState *pstate, Node *expr, ParseExprKind exprKind);
 
 extern const char *ParseExprKindName(ParseExprKind exprKind);
 
+typedef enum TransformNullEquals
+{
+	TRANSFORM_NULL_EQUALS_OFF = 0,	/* Disabled */
+	TRANSFORM_NULL_EQUALS_WARN,		/* Issue a warning */
+	TRANSFORM_NULL_EQUALS_ERR,		/* Error out */
+	TRANSFORM_NULL_EQUALS_ON		/* Enabled */
+} TransformNullEquals;
+
+extern TransformNullEquals transform_null_equals;
+
 #endif							/* PARSE_EXPR_H */
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 43ac5f5f11..cc4fdf2de8 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -767,3 +767,58 @@ NOTICE:  text search configuration "no_such_config" does not exist
 select func_with_bad_set();
 ERROR:  invalid value for parameter "default_text_search_config": "no_such_config"
 reset check_function_bodies;
+set transform_null_equals = off;
+select 1=null as transform_null_equals_off;
+ transform_null_equals_off 
+---------------------------
+ 
+(1 row)
+
+select null=null as transform_null_equals_off;
+ transform_null_equals_off 
+---------------------------
+ 
+(1 row)
+
+set transform_null_equals = warn;
+select 1=null as transform_null_equals_warn;
+WARNING:  = NULL can only produce a NULL
+LINE 1: select 1=null as transform_null_equals_warn;
+                ^
+ transform_null_equals_warn 
+----------------------------
+ 
+(1 row)
+
+select null=null as transform_null_equals_warn;
+WARNING:  = NULL can only produce a NULL
+LINE 1: select null=null as transform_null_equals_warn;
+                   ^
+ transform_null_equals_warn 
+----------------------------
+ 
+(1 row)
+
+set transform_null_equals = error;
+select 1=null as transform_null_equals_error;
+ERROR:  = NULL does not comply with the SQL specification
+LINE 1: select 1=null as transform_null_equals_error;
+                ^
+select null=null as transform_null_equals_error;
+ERROR:  = NULL does not comply with the SQL specification
+LINE 1: select null=null as transform_null_equals_error;
+                   ^
+set transform_null_equals = on;
+select 1=null as transform_null_equals_on;
+ transform_null_equals_on 
+--------------------------
+ f
+(1 row)
+
+select null=null as transform_null_equals_on;
+ transform_null_equals_on 
+--------------------------
+ t
+(1 row)
+
+reset transform_null_equals;
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index 23e5029780..24fa8c5308 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -288,3 +288,29 @@ set default_text_search_config = no_such_config;
 select func_with_bad_set();
 
 reset check_function_bodies;
+
+set transform_null_equals = off;
+
+select 1=null as transform_null_equals_off;
+
+select null=null as transform_null_equals_off;
+
+set transform_null_equals = warn;
+
+select 1=null as transform_null_equals_warn;
+
+select null=null as transform_null_equals_warn;
+
+set transform_null_equals = error;
+
+select 1=null as transform_null_equals_error;
+
+select null=null as transform_null_equals_error;
+
+set transform_null_equals = on;
+
+select 1=null as transform_null_equals_on;
+
+select null=null as transform_null_equals_on;
+
+reset transform_null_equals;

--------------2.17.1--

From c9d83b3fe39c86a38f7ac8114da90bef538f5cb7 Mon Sep 17 00:00:00 2001
From: David Fetter <david@fetter.org>
Date: Sun, 15 Jul 2018 16:11:08 -0700
Subject: [PATCH] Make transform_null_equals into an enum v5
To: pgsql-hackers@postgresql.org

The transform_null_equals GUC can now be off, warn, error, or on.
---
 doc/src/sgml/config.sgml                      | 25 ++++++---
 src/backend/parser/parse_expr.c               | 30 ++++++++--
 src/backend/utils/misc/guc.c                  | 44 ++++++++++-----
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 src/include/parser/parse_expr.h               | 11 +++-
 src/test/regress/expected/guc.out             | 55 +++++++++++++++++++
 src/test/regress/sql/guc.sql                  | 26 +++++++++
 7 files changed, 164 insertions(+), 29 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4d48d93305..6468c2253f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8066,7 +8066,7 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
      <variablelist>
 
      <varlistentry id="guc-transform-null-equals" xreflabel="transform_null_equals">
-      <term><varname>transform_null_equals</varname> (<type>boolean</type>)
+      <term><varname>transform_null_equals</varname> (<type>enum</type>)
       <indexterm><primary>IS NULL</primary></indexterm>
       <indexterm>
        <primary><varname>transform_null_equals</varname> configuration parameter</primary>
@@ -8074,15 +8074,24 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </term>
       <listitem>
        <para>
-        When on, expressions of the form <literal><replaceable>expr</replaceable> =
-        NULL</literal> (or <literal>NULL =
-        <replaceable>expr</replaceable></literal>) are treated as
+        When enabled, expressions of the form
+        <literal><replaceable>expr</replaceable> = NULL</literal> (or
+        <literal>NULL = <replaceable>expr</replaceable></literal>) are treated as
         <literal><replaceable>expr</replaceable> IS NULL</literal>, that is, they
         return true if <replaceable>expr</replaceable> evaluates to the null value,
-        and false otherwise. The correct SQL-spec-compliant behavior of
-        <literal><replaceable>expr</replaceable> = NULL</literal> is to always
-        return null (unknown). Therefore this parameter defaults to
-        <literal>off</literal>.
+        and false otherwise.  Valid values are <literal>off</literal>,
+        <literal>warn</literal>, <literal>error</literal>, and <literal>on</literal>.
+       </para>
+
+       <para>
+        The correct SQL-spec-compliant behavior of
+        <literal><replaceable>expr</replaceable> = <replacable>null_expression</replaceable></literal>
+        is to always return null (unknown); PostgreSQL allows <literal>NULL</literal>
+        as a null-valued expression of unknown type, which is an extension to the spec.
+        The transformation of <literal><replaceable>expr</replaceable> = NULL</literal>
+        to <literal><replaceable>expr</replaceable> IS NULL</literal> is inconsistent
+        with the normal semantics of the SQL specification, and is therefore set to "off"
+        by default.
        </para>
 
        <para>
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 385e54a9b6..5b3e5caaeb 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -41,8 +41,8 @@
 
 
 /* GUC parameters */
-bool		operator_precedence_warning = false;
-bool		Transform_null_equals = false;
+bool	operator_precedence_warning = false;
+int		transform_null_equals = TRANSFORM_NULL_EQUALS_OFF;
 
 /*
  * Node-type groups for operator precedence warnings
@@ -850,6 +850,7 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 	Node	   *lexpr = a->lexpr;
 	Node	   *rexpr = a->rexpr;
 	Node	   *result;
+	bool		is_null_equals = false;
 
 	if (operator_precedence_warning)
 	{
@@ -872,17 +873,34 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 	}
 
 	/*
-	 * Special-case "foo = NULL" and "NULL = foo" for compatibility with
+	 * Deal with "foo = NULL" and "NULL = foo" for compatibility with
 	 * standards-broken products (like Microsoft's).  Turn these into IS NULL
 	 * exprs. (If either side is a CaseTestExpr, then the expression was
 	 * generated internally from a CASE-WHEN expression, and
 	 * transform_null_equals does not apply.)
 	 */
-	if (Transform_null_equals &&
-		list_length(a->name) == 1 &&
+	is_null_equals = (list_length(a->name) == 1 &&
 		strcmp(strVal(linitial(a->name)), "=") == 0 &&
 		(exprIsNullConstant(lexpr) || exprIsNullConstant(rexpr)) &&
-		(!IsA(lexpr, CaseTestExpr) &&!IsA(rexpr, CaseTestExpr)))
+		(!IsA(lexpr, CaseTestExpr) &&!IsA(rexpr, CaseTestExpr)));
+
+	/*
+	 * We don't need to handle TRANSFORM_NULL_EQUALS_OFF here because it's a noop
+	 */
+
+	if (is_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_WARN)
+		ereport(WARNING,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("= NULL can only produce a NULL"),
+				 parser_errposition(pstate, a->location)));
+
+	if (is_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_ERR)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("= NULL does not comply with the SQL specification"),
+				 parser_errposition(pstate, a->location)));
+
+	if (is_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_ON)
 	{
 		NullTest   *n = makeNode(NullTest);
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a88ea6cfc9..d441bccb10 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -310,6 +310,23 @@ static const struct config_enum_entry track_function_options[] = {
 	{NULL, 0, false}
 };
 
+/*
+ * Accept the usual variants of boolean-ish, along with warn and error.
+ */
+static const struct config_enum_entry transform_null_equals_options[] = {
+	{"off", TRANSFORM_NULL_EQUALS_OFF, false},
+	{"false", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"no", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"0", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"warn", TRANSFORM_NULL_EQUALS_WARN, false},
+	{"error", TRANSFORM_NULL_EQUALS_ERR, false},
+	{"on", TRANSFORM_NULL_EQUALS_ON, false},
+	{"true", TRANSFORM_NULL_EQUALS_ON, true},
+	{"yes", TRANSFORM_NULL_EQUALS_ON, true},
+	{"1", TRANSFORM_NULL_EQUALS_ON, true},
+	{NULL, 0, false}
+};
+
 static const struct config_enum_entry xmlbinary_options[] = {
 	{"base64", XMLBINARY_BASE64, false},
 	{"hex", XMLBINARY_HEX, false},
@@ -1414,19 +1431,6 @@ static struct config_bool ConfigureNamesBool[] =
 		false,
 		NULL, NULL, NULL
 	},
-	{
-		{"transform_null_equals", PGC_USERSET, COMPAT_OPTIONS_CLIENT,
-			gettext_noop("Treats \"expr=NULL\" as \"expr IS NULL\"."),
-			gettext_noop("When turned on, expressions of the form expr = NULL "
-						 "(or NULL = expr) are treated as expr IS NULL, that is, they "
-						 "return true if expr evaluates to the null value, and false "
-						 "otherwise. The correct behavior of expr = NULL is to always "
-						 "return null (unknown).")
-		},
-		&Transform_null_equals,
-		false,
-		NULL, NULL, NULL
-	},
 	{
 		{"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH,
 			gettext_noop("Enables per-database user names."),
@@ -4074,6 +4078,20 @@ static struct config_enum ConfigureNamesEnum[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"transform_null_equals", PGC_USERSET, COMPAT_OPTIONS_CLIENT,
+			gettext_noop("Treats \"expr=NULL\" as \"expr IS NULL\"."),
+			gettext_noop("When set to on, expressions of the form expr = NULL "
+						 "(or NULL = expr) are treated as expr IS NULL. When "
+						 "set to off, warn, or error, they are not, with increasing "
+						 "insistence. The default behavior of expr = NULL is to "
+						 "return null (unknown).")
+		},
+		&transform_null_equals,
+		TRANSFORM_NULL_EQUALS_OFF, transform_null_equals_options,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"wal_level", PGC_POSTMASTER, WAL_SETTINGS,
 			gettext_noop("Set the level of information written to the WAL."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index c0d3fb8491..07dda7b746 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -656,7 +656,7 @@
 
 # - Other Platforms and Clients -
 
-#transform_null_equals = off
+#transform_null_equals = off	# off, warn, error, or on
 
 
 #------------------------------------------------------------------------------
diff --git a/src/include/parser/parse_expr.h b/src/include/parser/parse_expr.h
index e5aff61b8f..22c11e51f2 100644
--- a/src/include/parser/parse_expr.h
+++ b/src/include/parser/parse_expr.h
@@ -17,10 +17,19 @@
 
 /* GUC parameters */
 extern bool operator_precedence_warning;
-extern bool Transform_null_equals;
 
 extern Node *transformExpr(ParseState *pstate, Node *expr, ParseExprKind exprKind);
 
 extern const char *ParseExprKindName(ParseExprKind exprKind);
 
+typedef enum TransformNullEquals
+{
+	TRANSFORM_NULL_EQUALS_OFF = 0,	/* Disabled */
+	TRANSFORM_NULL_EQUALS_WARN,		/* Issue a warning */
+	TRANSFORM_NULL_EQUALS_ERR,		/* Error out */
+	TRANSFORM_NULL_EQUALS_ON		/* Enabled */
+} TransformNullEquals;
+
+int transform_null_equals;
+
 #endif							/* PARSE_EXPR_H */
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 43ac5f5f11..cc4fdf2de8 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -767,3 +767,58 @@ NOTICE:  text search configuration "no_such_config" does not exist
 select func_with_bad_set();
 ERROR:  invalid value for parameter "default_text_search_config": "no_such_config"
 reset check_function_bodies;
+set transform_null_equals = off;
+select 1=null as transform_null_equals_off;
+ transform_null_equals_off 
+---------------------------
+ 
+(1 row)
+
+select null=null as transform_null_equals_off;
+ transform_null_equals_off 
+---------------------------
+ 
+(1 row)
+
+set transform_null_equals = warn;
+select 1=null as transform_null_equals_warn;
+WARNING:  = NULL can only produce a NULL
+LINE 1: select 1=null as transform_null_equals_warn;
+                ^
+ transform_null_equals_warn 
+----------------------------
+ 
+(1 row)
+
+select null=null as transform_null_equals_warn;
+WARNING:  = NULL can only produce a NULL
+LINE 1: select null=null as transform_null_equals_warn;
+                   ^
+ transform_null_equals_warn 
+----------------------------
+ 
+(1 row)
+
+set transform_null_equals = error;
+select 1=null as transform_null_equals_error;
+ERROR:  = NULL does not comply with the SQL specification
+LINE 1: select 1=null as transform_null_equals_error;
+                ^
+select null=null as transform_null_equals_error;
+ERROR:  = NULL does not comply with the SQL specification
+LINE 1: select null=null as transform_null_equals_error;
+                   ^
+set transform_null_equals = on;
+select 1=null as transform_null_equals_on;
+ transform_null_equals_on 
+--------------------------
+ f
+(1 row)
+
+select null=null as transform_null_equals_on;
+ transform_null_equals_on 
+--------------------------
+ t
+(1 row)
+
+reset transform_null_equals;
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index 23e5029780..24fa8c5308 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -288,3 +288,29 @@ set default_text_search_config = no_such_config;
 select func_with_bad_set();
 
 reset check_function_bodies;
+
+set transform_null_equals = off;
+
+select 1=null as transform_null_equals_off;
+
+select null=null as transform_null_equals_off;
+
+set transform_null_equals = warn;
+
+select 1=null as transform_null_equals_warn;
+
+select null=null as transform_null_equals_warn;
+
+set transform_null_equals = error;
+
+select 1=null as transform_null_equals_error;
+
+select null=null as transform_null_equals_error;
+
+set transform_null_equals = on;
+
+select 1=null as transform_null_equals_on;
+
+select null=null as transform_null_equals_on;
+
+reset transform_null_equals;
-- 
2.17.1

From b1c6e4c92226065c82053a4d161daf2822149f76 Mon Sep 17 00:00:00 2001
From: David Fetter <david@fetter.org>
Date: Sun, 15 Jul 2018 16:11:08 -0700
Subject: [PATCH] Make transform_null_equals into an enum v6
To: pgsql-hackers@postgresql.org

The transform_null_equals GUC can now be off, warn, error, or on.
---
 doc/src/sgml/config.sgml                      | 30 ++++++----
 src/backend/parser/parse_expr.c               | 30 ++++++++--
 src/backend/utils/misc/guc.c                  | 44 ++++++++++-----
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 src/include/parser/parse_expr.h               | 11 +++-
 src/test/regress/expected/guc.out             | 55 +++++++++++++++++++
 src/test/regress/sql/guc.sql                  | 26 +++++++++
 7 files changed, 166 insertions(+), 32 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4d48d93305..2e1441f227 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8066,7 +8066,7 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
      <variablelist>
 
      <varlistentry id="guc-transform-null-equals" xreflabel="transform_null_equals">
-      <term><varname>transform_null_equals</varname> (<type>boolean</type>)
+      <term><varname>transform_null_equals</varname> (<type>enum</type>)
       <indexterm><primary>IS NULL</primary></indexterm>
       <indexterm>
        <primary><varname>transform_null_equals</varname> configuration parameter</primary>
@@ -8074,15 +8074,24 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </term>
       <listitem>
        <para>
-        When on, expressions of the form <literal><replaceable>expr</replaceable> =
-        NULL</literal> (or <literal>NULL =
-        <replaceable>expr</replaceable></literal>) are treated as
+        When enabled, expressions of the form
+        <literal><replaceable>expr</replaceable> = NULL</literal> (or
+        <literal>NULL = <replaceable>expr</replaceable></literal>) are treated as
         <literal><replaceable>expr</replaceable> IS NULL</literal>, that is, they
         return true if <replaceable>expr</replaceable> evaluates to the null value,
-        and false otherwise. The correct SQL-spec-compliant behavior of
-        <literal><replaceable>expr</replaceable> = NULL</literal> is to always
-        return null (unknown). Therefore this parameter defaults to
-        <literal>off</literal>.
+        and false otherwise.  Valid values are <literal>off</literal>,
+        <literal>warn</literal>, <literal>error</literal>, and <literal>on</literal>.
+       </para>
+
+       <para>
+        The correct SQL-spec-compliant behavior of
+        <literal><replaceable>expr</replaceable> = <replaceable>null_expression</replaceable></literal>
+        is to always return null (unknown); PostgreSQL allows <literal>NULL</literal>
+        as a null-valued expression of unknown type, which is an extension to the spec.
+        The transformation of <literal><replaceable>expr</replaceable> = NULL</literal>
+        to <literal><replaceable>expr</replaceable> IS NULL</literal> is inconsistent
+        with the normal semantics of the SQL specification, and is therefore set to "off"
+        by default.
        </para>
 
        <para>
@@ -8094,9 +8103,8 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
         form <literal><replaceable>expr</replaceable> = NULL</literal> always
         return the null value (using the SQL standard interpretation), they are not
         very useful and do not appear often in normal applications so
-        this option does little harm in practice.  But new users are
-        frequently confused about the semantics of expressions
-        involving null values, so this option is off by default.
+        this option does little harm in practice.  As this option makes null
+        handling even more confusing, especially for new users, it is off by default.
        </para>
 
        <para>
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 385e54a9b6..5b3e5caaeb 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -41,8 +41,8 @@
 
 
 /* GUC parameters */
-bool		operator_precedence_warning = false;
-bool		Transform_null_equals = false;
+bool	operator_precedence_warning = false;
+int		transform_null_equals = TRANSFORM_NULL_EQUALS_OFF;
 
 /*
  * Node-type groups for operator precedence warnings
@@ -850,6 +850,7 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 	Node	   *lexpr = a->lexpr;
 	Node	   *rexpr = a->rexpr;
 	Node	   *result;
+	bool		is_null_equals = false;
 
 	if (operator_precedence_warning)
 	{
@@ -872,17 +873,34 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
 	}
 
 	/*
-	 * Special-case "foo = NULL" and "NULL = foo" for compatibility with
+	 * Deal with "foo = NULL" and "NULL = foo" for compatibility with
 	 * standards-broken products (like Microsoft's).  Turn these into IS NULL
 	 * exprs. (If either side is a CaseTestExpr, then the expression was
 	 * generated internally from a CASE-WHEN expression, and
 	 * transform_null_equals does not apply.)
 	 */
-	if (Transform_null_equals &&
-		list_length(a->name) == 1 &&
+	is_null_equals = (list_length(a->name) == 1 &&
 		strcmp(strVal(linitial(a->name)), "=") == 0 &&
 		(exprIsNullConstant(lexpr) || exprIsNullConstant(rexpr)) &&
-		(!IsA(lexpr, CaseTestExpr) &&!IsA(rexpr, CaseTestExpr)))
+		(!IsA(lexpr, CaseTestExpr) &&!IsA(rexpr, CaseTestExpr)));
+
+	/*
+	 * We don't need to handle TRANSFORM_NULL_EQUALS_OFF here because it's a noop
+	 */
+
+	if (is_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_WARN)
+		ereport(WARNING,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("= NULL can only produce a NULL"),
+				 parser_errposition(pstate, a->location)));
+
+	if (is_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_ERR)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("= NULL does not comply with the SQL specification"),
+				 parser_errposition(pstate, a->location)));
+
+	if (is_null_equals && transform_null_equals == TRANSFORM_NULL_EQUALS_ON)
 	{
 		NullTest   *n = makeNode(NullTest);
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a88ea6cfc9..d441bccb10 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -310,6 +310,23 @@ static const struct config_enum_entry track_function_options[] = {
 	{NULL, 0, false}
 };
 
+/*
+ * Accept the usual variants of boolean-ish, along with warn and error.
+ */
+static const struct config_enum_entry transform_null_equals_options[] = {
+	{"off", TRANSFORM_NULL_EQUALS_OFF, false},
+	{"false", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"no", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"0", TRANSFORM_NULL_EQUALS_OFF, true},
+	{"warn", TRANSFORM_NULL_EQUALS_WARN, false},
+	{"error", TRANSFORM_NULL_EQUALS_ERR, false},
+	{"on", TRANSFORM_NULL_EQUALS_ON, false},
+	{"true", TRANSFORM_NULL_EQUALS_ON, true},
+	{"yes", TRANSFORM_NULL_EQUALS_ON, true},
+	{"1", TRANSFORM_NULL_EQUALS_ON, true},
+	{NULL, 0, false}
+};
+
 static const struct config_enum_entry xmlbinary_options[] = {
 	{"base64", XMLBINARY_BASE64, false},
 	{"hex", XMLBINARY_HEX, false},
@@ -1414,19 +1431,6 @@ static struct config_bool ConfigureNamesBool[] =
 		false,
 		NULL, NULL, NULL
 	},
-	{
-		{"transform_null_equals", PGC_USERSET, COMPAT_OPTIONS_CLIENT,
-			gettext_noop("Treats \"expr=NULL\" as \"expr IS NULL\"."),
-			gettext_noop("When turned on, expressions of the form expr = NULL "
-						 "(or NULL = expr) are treated as expr IS NULL, that is, they "
-						 "return true if expr evaluates to the null value, and false "
-						 "otherwise. The correct behavior of expr = NULL is to always "
-						 "return null (unknown).")
-		},
-		&Transform_null_equals,
-		false,
-		NULL, NULL, NULL
-	},
 	{
 		{"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH,
 			gettext_noop("Enables per-database user names."),
@@ -4074,6 +4078,20 @@ static struct config_enum ConfigureNamesEnum[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"transform_null_equals", PGC_USERSET, COMPAT_OPTIONS_CLIENT,
+			gettext_noop("Treats \"expr=NULL\" as \"expr IS NULL\"."),
+			gettext_noop("When set to on, expressions of the form expr = NULL "
+						 "(or NULL = expr) are treated as expr IS NULL. When "
+						 "set to off, warn, or error, they are not, with increasing "
+						 "insistence. The default behavior of expr = NULL is to "
+						 "return null (unknown).")
+		},
+		&transform_null_equals,
+		TRANSFORM_NULL_EQUALS_OFF, transform_null_equals_options,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"wal_level", PGC_POSTMASTER, WAL_SETTINGS,
 			gettext_noop("Set the level of information written to the WAL."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index c0d3fb8491..07dda7b746 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -656,7 +656,7 @@
 
 # - Other Platforms and Clients -
 
-#transform_null_equals = off
+#transform_null_equals = off	# off, warn, error, or on
 
 
 #------------------------------------------------------------------------------
diff --git a/src/include/parser/parse_expr.h b/src/include/parser/parse_expr.h
index e5aff61b8f..22c11e51f2 100644
--- a/src/include/parser/parse_expr.h
+++ b/src/include/parser/parse_expr.h
@@ -17,10 +17,19 @@
 
 /* GUC parameters */
 extern bool operator_precedence_warning;
-extern bool Transform_null_equals;
 
 extern Node *transformExpr(ParseState *pstate, Node *expr, ParseExprKind exprKind);
 
 extern const char *ParseExprKindName(ParseExprKind exprKind);
 
+typedef enum TransformNullEquals
+{
+	TRANSFORM_NULL_EQUALS_OFF = 0,	/* Disabled */
+	TRANSFORM_NULL_EQUALS_WARN,		/* Issue a warning */
+	TRANSFORM_NULL_EQUALS_ERR,		/* Error out */
+	TRANSFORM_NULL_EQUALS_ON		/* Enabled */
+} TransformNullEquals;
+
+int transform_null_equals;
+
 #endif							/* PARSE_EXPR_H */
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 43ac5f5f11..cc4fdf2de8 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -767,3 +767,58 @@ NOTICE:  text search configuration "no_such_config" does not exist
 select func_with_bad_set();
 ERROR:  invalid value for parameter "default_text_search_config": "no_such_config"
 reset check_function_bodies;
+set transform_null_equals = off;
+select 1=null as transform_null_equals_off;
+ transform_null_equals_off 
+---------------------------
+ 
+(1 row)
+
+select null=null as transform_null_equals_off;
+ transform_null_equals_off 
+---------------------------
+ 
+(1 row)
+
+set transform_null_equals = warn;
+select 1=null as transform_null_equals_warn;
+WARNING:  = NULL can only produce a NULL
+LINE 1: select 1=null as transform_null_equals_warn;
+                ^
+ transform_null_equals_warn 
+----------------------------
+ 
+(1 row)
+
+select null=null as transform_null_equals_warn;
+WARNING:  = NULL can only produce a NULL
+LINE 1: select null=null as transform_null_equals_warn;
+                   ^
+ transform_null_equals_warn 
+----------------------------
+ 
+(1 row)
+
+set transform_null_equals = error;
+select 1=null as transform_null_equals_error;
+ERROR:  = NULL does not comply with the SQL specification
+LINE 1: select 1=null as transform_null_equals_error;
+                ^
+select null=null as transform_null_equals_error;
+ERROR:  = NULL does not comply with the SQL specification
+LINE 1: select null=null as transform_null_equals_error;
+                   ^
+set transform_null_equals = on;
+select 1=null as transform_null_equals_on;
+ transform_null_equals_on 
+--------------------------
+ f
+(1 row)
+
+select null=null as transform_null_equals_on;
+ transform_null_equals_on 
+--------------------------
+ t
+(1 row)
+
+reset transform_null_equals;
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index 23e5029780..24fa8c5308 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -288,3 +288,29 @@ set default_text_search_config = no_such_config;
 select func_with_bad_set();
 
 reset check_function_bodies;
+
+set transform_null_equals = off;
+
+select 1=null as transform_null_equals_off;
+
+select null=null as transform_null_equals_off;
+
+set transform_null_equals = warn;
+
+select 1=null as transform_null_equals_warn;
+
+select null=null as transform_null_equals_warn;
+
+set transform_null_equals = error;
+
+select 1=null as transform_null_equals_error;
+
+select null=null as transform_null_equals_error;
+
+set transform_null_equals = on;
+
+select 1=null as transform_null_equals_on;
+
+select null=null as transform_null_equals_on;
+
+reset transform_null_equals;
-- 
2.17.1