From bc4291a8307fa299d96e99a0a98348c451a3eab6 Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Tue, 21 Jul 2020 14:18:48 -0700
Subject: [PATCH v3 1/3] Removing postfix operator support.

Removing postfix operator support from the grammar; disallowing
the creation of postfix operators through the CREATE OPERATOR
command; and removing the factorial postfix operator.

This accomplishes nothing useful in itself, but sets up
for grammar changes to allow keywords to be used as bare column
aliases without the use of an explicit "AS".
---
 .../postgres_fdw/expected/postgres_fdw.out    |  8 +++---
 contrib/postgres_fdw/sql/postgres_fdw.sql     |  2 +-
 src/backend/commands/operatorcmds.c           | 18 ++++++++++++-
 src/backend/parser/gram.y                     | 12 +--------
 src/include/catalog/pg_operator.dat           |  3 ---
 src/include/catalog/pg_proc.dat               |  2 +-
 src/test/regress/expected/create_operator.out | 20 +++++++++-----
 src/test/regress/expected/numeric.out         | 27 ++++++++++++++++---
 src/test/regress/sql/create_operator.sql      |  2 +-
 src/test/regress/sql/numeric.sql              | 16 ++++++++---
 10 files changed, 73 insertions(+), 37 deletions(-)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 90db550b92..9728c252ba 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -653,12 +653,12 @@ EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c1 = -c1;          -- Op
    Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (("C 1" = (- "C 1")))
 (3 rows)
 
-EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE 1 = c1!;           -- OpExpr(r)
-                                                QUERY PLAN                                                
-----------------------------------------------------------------------------------------------------------
+EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE 1 = !!c1;          -- OpExpr(r)
+                                                QUERY PLAN                                                 
+-----------------------------------------------------------------------------------------------------------
  Foreign Scan on public.ft1 t1
    Output: c1, c2, c3, c4, c5, c6, c7, c8
-   Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE ((1::numeric = ("C 1" !)))
+   Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE ((1::numeric = (!! "C 1")))
 (3 rows)
 
 EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE (c1 IS NOT NULL) IS DISTINCT FROM (c1 IS NOT NULL); -- DistinctExpr
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 83971665e3..a8e58a8f24 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -307,7 +307,7 @@ EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c1 IS NULL;        -- Nu
 EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c1 IS NOT NULL;    -- NullTest
 EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE round(abs(c1), 0) = 1; -- FuncExpr
 EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c1 = -c1;          -- OpExpr(l)
-EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE 1 = c1!;           -- OpExpr(r)
+EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE 1 = !!c1;          -- OpExpr(r)
 EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE (c1 IS NOT NULL) IS DISTINCT FROM (c1 IS NOT NULL); -- DistinctExpr
 EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c1 = ANY(ARRAY[c2, 1, c1 + 0]); -- ScalarArrayOpExpr
 EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c1 = (ARRAY[c1,c2,3])[1]; -- SubscriptingRef
diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c
index 0a53e9b93e..cb7ffda4bd 100644
--- a/src/backend/commands/operatorcmds.c
+++ b/src/backend/commands/operatorcmds.c
@@ -168,10 +168,26 @@ DefineOperator(List *names, List *parameters)
 	if (typeName2)
 		typeId2 = typenameTypeId(NULL, typeName2);
 
+	/*
+	 * If neither argument is specified, do not mention postfix operator
+	 * discontinuation, as the user is unlikely to have meant to create a
+	 * postfix operator.  It is more likely they simply neglected to mention
+	 * the args.
+	 */
 	if (!OidIsValid(typeId1) && !OidIsValid(typeId2))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
-				 errmsg("at least one of leftarg or rightarg must be specified")));
+				 errmsg("at least rightarg must be specified")));
+
+	/*
+	 * But if only the right arg is missing, they probably do intend to create
+	 * a postfix operator, so give them a hint about why that no longer works.
+	 */
+	if (!OidIsValid(typeId2))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+				 errmsg("rightarg must be specified"),
+				 errhint("postfix operator support has been discontinued")));
 
 	if (typeName1)
 	{
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index dbb47d4982..18c2c3c2f0 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -741,7 +741,6 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %nonassoc	'<' '>' '=' LESS_EQUALS GREATER_EQUALS NOT_EQUALS
 %nonassoc	BETWEEN IN_P LIKE ILIKE SIMILAR NOT_LA
 %nonassoc	ESCAPE			/* ESCAPE must be just above LIKE/ILIKE/SIMILAR */
-%left		POSTFIXOP		/* dummy for postfix Op rules */
 /*
  * To support target_el without AS, we must give IDENT an explicit priority
  * between POSTFIXOP and Op.  We can safely assign the same priority to
@@ -12989,9 +12988,6 @@ a_expr:		c_expr									{ $$ = $1; }
 				{ $$ = (Node *) makeA_Expr(AEXPR_OP, $2, $1, $3, @2); }
 			| qual_Op a_expr					%prec Op
 				{ $$ = (Node *) makeA_Expr(AEXPR_OP, $1, NULL, $2, @1); }
-			| a_expr qual_Op					%prec POSTFIXOP
-				{ $$ = (Node *) makeA_Expr(AEXPR_OP, $2, $1, NULL, @2); }
-
 			| a_expr AND a_expr
 				{ $$ = makeAndExpr($1, $3, @2); }
 			| a_expr OR a_expr
@@ -13404,8 +13400,6 @@ b_expr:		c_expr
 				{ $$ = (Node *) makeA_Expr(AEXPR_OP, $2, $1, $3, @2); }
 			| qual_Op b_expr					%prec Op
 				{ $$ = (Node *) makeA_Expr(AEXPR_OP, $1, NULL, $2, @1); }
-			| b_expr qual_Op					%prec POSTFIXOP
-				{ $$ = (Node *) makeA_Expr(AEXPR_OP, $2, $1, NULL, @2); }
 			| b_expr IS DISTINCT FROM b_expr		%prec IS
 				{
 					$$ = (Node *) makeSimpleA_Expr(AEXPR_DISTINCT, "=", $1, $5, @2);
@@ -14661,11 +14655,7 @@ target_el:	a_expr AS ColLabel
 				}
 			/*
 			 * We support omitting AS only for column labels that aren't
-			 * any known keyword.  There is an ambiguity against postfix
-			 * operators: is "a ! b" an infix expression, or a postfix
-			 * expression and a column label?  We prefer to resolve this
-			 * as an infix expression, which we accomplish by assigning
-			 * IDENT a precedence higher than POSTFIXOP.
+			 * any known keyword.
 			 */
 			| a_expr IDENT
 				{
diff --git a/src/include/catalog/pg_operator.dat b/src/include/catalog/pg_operator.dat
index 5b0e063655..9b83bddf2e 100644
--- a/src/include/catalog/pg_operator.dat
+++ b/src/include/catalog/pg_operator.dat
@@ -218,9 +218,6 @@
   oprname => '>=', oprleft => 'xid8', oprright => 'xid8', oprresult => 'bool',
   oprcom => '<=(xid8,xid8)', oprnegate => '<(xid8,xid8)', oprcode => 'xid8ge',
   oprrest => 'scalargesel', oprjoin => 'scalargejoinsel' },
-{ oid => '388', descr => 'factorial',
-  oprname => '!', oprkind => 'r', oprleft => 'int8', oprright => '0',
-  oprresult => 'numeric', oprcode => 'numeric_fac' },
 { oid => '389', descr => 'deprecated, use ! instead',
   oprname => '!!', oprkind => 'l', oprleft => '0', oprright => 'int8',
   oprresult => 'numeric', oprcode => 'numeric_fac' },
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 4b5af32440..88d59fb96d 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -327,7 +327,7 @@
 { oid => '110', descr => 'I/O',
   proname => 'unknownout', prorettype => 'cstring', proargtypes => 'unknown',
   prosrc => 'unknownout' },
-{ oid => '111',
+{ oid => '111', descr => 'factorial',
   proname => 'numeric_fac', prorettype => 'numeric', proargtypes => 'int8',
   prosrc => 'numeric_fac' },
 
diff --git a/src/test/regress/expected/create_operator.out b/src/test/regress/expected/create_operator.out
index 54e8b79159..ed77a6616e 100644
--- a/src/test/regress/expected/create_operator.out
+++ b/src/test/regress/expected/create_operator.out
@@ -22,10 +22,14 @@ CREATE OPERATOR #@# (
    leftarg = int8,		-- right unary
    procedure = numeric_fac
 );
+ERROR:  rightarg must be specified
+HINT:  postfix operator support has been discontinued
 CREATE OPERATOR #%# (
    leftarg = int8,		-- right unary
    procedure = numeric_fac
 );
+ERROR:  rightarg must be specified
+HINT:  postfix operator support has been discontinued
 -- Test operator created above
 SELECT point '(1,2)' <% widget '(0,0,3)' AS t,
        point '(1,2)' <% widget '(0,0,1)' AS f;
@@ -52,12 +56,12 @@ CREATE OPERATOR !=- (
    leftarg = int8,		-- right unary
    procedure = numeric_fac
 );
+ERROR:  rightarg must be specified
+HINT:  postfix operator support has been discontinued
 SELECT 2 !=-;
- ?column? 
-----------
-        2
-(1 row)
-
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT 2 !=-;
+                    ^
 -- make sure lexer returns != as <> even in edge cases
 SELECT 2 !=/**/ 1, 2 !=/**/ 2;
  ?column? | ?column? 
@@ -172,11 +176,13 @@ CREATE OPERATOR #@%# (
    invalid_att = int8
 );
 WARNING:  operator attribute "invalid_att" not recognized
--- Should fail. At least leftarg or rightarg should be mandatorily specified
+ERROR:  rightarg must be specified
+HINT:  postfix operator support has been discontinued
+-- Should fail. At least rightarg should be mandatorily specified
 CREATE OPERATOR #@%# (
    procedure = numeric_fac
 );
-ERROR:  at least one of leftarg or rightarg must be specified
+ERROR:  at least rightarg must be specified
 -- Should fail. Procedure should be mandatorily specified
 CREATE OPERATOR #@%# (
    leftarg = int8
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index 81a0c5d40f..8b2e29779d 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -2318,7 +2318,7 @@ ERROR:  value overflows numeric format
 --
 -- Tests for factorial
 --
-SELECT 4!;
+SELECT !!4;
  ?column? 
 ----------
        24
@@ -2336,19 +2336,38 @@ SELECT factorial(15);
  1307674368000
 (1 row)
 
-SELECT 100000!;
+SELECT !!100000;
 ERROR:  value overflows numeric format
-SELECT 0!;
+SELECT !!0;
  ?column? 
 ----------
         1
 (1 row)
 
-SELECT -4!;
+SELECT !!(-4);
 ERROR:  factorial of a negative number is undefined
 SELECT factorial(-4);
 ERROR:  factorial of a negative number is undefined
 --
+-- Postfix operator support has been removed.
+--
+SELECT -5!;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT -5!;
+                  ^
+SELECT -0!;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT -0!;
+                  ^
+SELECT 0!;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT 0!;
+                 ^
+SELECT 100!;
+ERROR:  syntax error at or near ";"
+LINE 1: SELECT 100!;
+                   ^
+--
 -- Tests for pg_lsn()
 --
 SELECT pg_lsn(23783416::numeric);
diff --git a/src/test/regress/sql/create_operator.sql b/src/test/regress/sql/create_operator.sql
index 8b6fd0bb43..eb2335e0ef 100644
--- a/src/test/regress/sql/create_operator.sql
+++ b/src/test/regress/sql/create_operator.sql
@@ -133,7 +133,7 @@ CREATE OPERATOR #@%# (
    invalid_att = int8
 );
 
--- Should fail. At least leftarg or rightarg should be mandatorily specified
+-- Should fail. At least rightarg should be mandatorily specified
 CREATE OPERATOR #@%# (
    procedure = numeric_fac
 );
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index 5dc80f686f..f8ce69422e 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -1115,14 +1115,22 @@ SELECT lcm(9999 * (10::numeric)^131068 + (10::numeric^131068 - 1), 2); -- overfl
 --
 -- Tests for factorial
 --
-SELECT 4!;
+SELECT !!4;
 SELECT !!3;
 SELECT factorial(15);
-SELECT 100000!;
-SELECT 0!;
-SELECT -4!;
+SELECT !!100000;
+SELECT !!0;
+SELECT !!(-4);
 SELECT factorial(-4);
 
+--
+-- Postfix operator support has been removed.
+--
+SELECT -5!;
+SELECT -0!;
+SELECT 0!;
+SELECT 100!;
+
 --
 -- Tests for pg_lsn()
 --
-- 
2.21.1 (Apple Git-122.3)

