Remove support for postfix (right-unary) operators.

This feature has been a thorn in our sides for a long time, causing
many grammatical ambiguity problems.  It doesn't seem worth the
pain to continue to support it, so remove it.

There are some follow-on improvements we can make in the grammar,
but this commit only removes the bare minimum number of productions,
plus assorted backend support code.

Note that pg_dump and psql continue to have full support, since
they may be used against older servers.  However, pg_dump warns
about postfix operators.  There is also a check in pg_upgrade.

Documentation-wise, I (tgl) largely removed the "left unary"
terminology in favor of saying "prefix operator", which is
a more standard and IMO less confusing term.

I included a catversion bump, although no initial catalog data
changes here, to mark the boundary at which oprkind = 'r'
stopped being valid in pg_operator.

Mark Dilger, based on work by myself and Robert Haas;
review by John Naylor

Discussion: https://postgr.es/m/38ca86db-42ab-9b48-2902-337a0d6b8311@2ndquadrant.com

8 files changed, 59 insertions, 185 deletions

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 0152e3869ab..391a9b225db 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -1473,8 +1473,7 @@ FunctionIsVisible(Oid funcid)
  *		Given a possibly-qualified operator name and exact input datatypes,
  *		look up the operator.  Returns InvalidOid if not found.
  *
- * Pass oprleft = InvalidOid for a prefix op, oprright = InvalidOid for
- * a postfix op.
+ * Pass oprleft = InvalidOid for a prefix op.
  *
  * If the operator name is not schema-qualified, it is sought in the current
  * namespace search path.  If the name is schema-qualified and the given
@@ -1580,8 +1579,8 @@ OpernameGetOprid(List *names, Oid oprleft, Oid oprright)
  * namespace case, we arrange for entries in earlier namespaces to mask
  * identical entries in later namespaces.
  *
- * The returned items always have two args[] entries --- one or the other
- * will be InvalidOid for a prefix or postfix oprkind.  nargs is 2, too.
+ * The returned items always have two args[] entries --- the first will be
+ * InvalidOid for a prefix oprkind.  nargs is always 2, too.
  */
 FuncCandidateList
 OpernameGetCandidates(List *names, char oprkind, bool missing_schema_ok)
diff --git a/src/backend/catalog/pg_operator.c b/src/backend/catalog/pg_operator.c
index f7c07c9b5b8..904cb8ef820 100644
--- a/src/backend/catalog/pg_operator.c
+++ b/src/backend/catalog/pg_operator.c
@@ -245,7 +245,7 @@ OperatorShellMake(const char *operatorName,
 	values[Anum_pg_operator_oprname - 1] = NameGetDatum(&oname);
 	values[Anum_pg_operator_oprnamespace - 1] = ObjectIdGetDatum(operatorNamespace);
 	values[Anum_pg_operator_oprowner - 1] = ObjectIdGetDatum(GetUserId());
-	values[Anum_pg_operator_oprkind - 1] = CharGetDatum(leftTypeId ? (rightTypeId ? 'b' : 'r') : 'l');
+	values[Anum_pg_operator_oprkind - 1] = CharGetDatum(leftTypeId ? 'b' : 'l');
 	values[Anum_pg_operator_oprcanmerge - 1] = BoolGetDatum(false);
 	values[Anum_pg_operator_oprcanhash - 1] = BoolGetDatum(false);
 	values[Anum_pg_operator_oprleft - 1] = ObjectIdGetDatum(leftTypeId);
@@ -494,7 +494,7 @@ OperatorCreate(const char *operatorName,
 	values[Anum_pg_operator_oprname - 1] = NameGetDatum(&oname);
 	values[Anum_pg_operator_oprnamespace - 1] = ObjectIdGetDatum(operatorNamespace);
 	values[Anum_pg_operator_oprowner - 1] = ObjectIdGetDatum(GetUserId());
-	values[Anum_pg_operator_oprkind - 1] = CharGetDatum(leftTypeId ? (rightTypeId ? 'b' : 'r') : 'l');
+	values[Anum_pg_operator_oprkind - 1] = CharGetDatum(leftTypeId ? 'b' : 'l');
 	values[Anum_pg_operator_oprcanmerge - 1] = BoolGetDatum(canMerge);
 	values[Anum_pg_operator_oprcanhash - 1] = BoolGetDatum(canHash);
 	values[Anum_pg_operator_oprleft - 1] = ObjectIdGetDatum(leftTypeId);
diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c
index bf23937849c..a791e99092d 100644
--- a/src/backend/commands/operatorcmds.c
+++ b/src/backend/commands/operatorcmds.c
@@ -168,10 +168,22 @@ DefineOperator(List *names, List *parameters)
 	if (typeName2)
 		typeId2 = typenameTypeId(NULL, typeName2);
 
+	/*
+	 * If only the right argument is missing, the user is likely trying to
+	 * create a postfix operator, so give them a hint about why that does not
+	 * work.  But if both arguments are missing, do not mention postfix
+	 * operators, as the user most likely simply neglected to mention the
+	 * arguments.
+	 */
 	if (!OidIsValid(typeId1) && !OidIsValid(typeId2))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
-				 errmsg("at least one of leftarg or rightarg must be specified")));
+				 errmsg("operator argument types must be specified")));
+	if (!OidIsValid(typeId2))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+				 errmsg("operator right argument type must be specified"),
+				 errdetail("Postfix operators are not supported.")));
 
 	if (typeName1)
 	{
diff --git a/src/backend/nodes/print.c b/src/backend/nodes/print.c
index 42476724d88..970a2d43840 100644
--- a/src/backend/nodes/print.c
+++ b/src/backend/nodes/print.c
@@ -394,7 +394,6 @@ print_expr(const Node *expr, const List *rtable)
 		}
 		else
 		{
-			/* we print prefix and postfix ops the same... */
 			printf("%s ", ((opname != NULL) ? opname : "(invalid operator)"));
 			print_expr(get_leftop((const Expr *) e), rtable);
 		}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 4558c02f6a6..b16ffb9bf7f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -741,10 +741,9 @@ 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
+ * between ESCAPE and Op.  We can safely assign the same priority to
  * various unreserved keywords as needed to resolve ambiguities (this can't
  * have any bad effects since obviously the keywords will still behave the
  * same as if they weren't keywords).  We need to do this:
@@ -12993,8 +12992,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); }
@@ -13408,8 +13405,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);
@@ -14665,11 +14660,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/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index f69976cc8c9..d24420c5831 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -57,7 +57,7 @@ bool		Transform_null_equals = false;
 #define PREC_GROUP_NOT_LIKE		9	/* NOT LIKE/ILIKE/SIMILAR */
 #define PREC_GROUP_NOT_BETWEEN	10	/* NOT BETWEEN */
 #define PREC_GROUP_NOT_IN		11	/* NOT IN */
-#define PREC_GROUP_POSTFIX_OP	12	/* generic postfix operators */
+#define PREC_GROUP_ANY_ALL		12	/* ANY/ALL */
 #define PREC_GROUP_INFIX_OP		13	/* generic infix operators */
 #define PREC_GROUP_PREFIX_OP	14	/* generic prefix operators */
 
@@ -71,7 +71,7 @@ bool		Transform_null_equals = false;
  * 4. LIKE ILIKE SIMILAR
  * 5. BETWEEN
  * 6. IN
- * 7. generic postfix Op
+ * 7. ANY ALL
  * 8. generic Op, including <= => <>
  * 9. generic prefix Op
  * 10. IS tests (NullTest, BooleanTest, etc)
@@ -1031,7 +1031,7 @@ transformAExprOpAny(ParseState *pstate, A_Expr *a)
 	Node	   *rexpr = a->rexpr;
 
 	if (operator_precedence_warning)
-		emit_precedence_warnings(pstate, PREC_GROUP_POSTFIX_OP,
+		emit_precedence_warnings(pstate, PREC_GROUP_ANY_ALL,
 								 strVal(llast(a->name)),
 								 lexpr, NULL,
 								 a->location);
@@ -1054,7 +1054,7 @@ transformAExprOpAll(ParseState *pstate, A_Expr *a)
 	Node	   *rexpr = a->rexpr;
 
 	if (operator_precedence_warning)
-		emit_precedence_warnings(pstate, PREC_GROUP_POSTFIX_OP,
+		emit_precedence_warnings(pstate, PREC_GROUP_ANY_ALL,
 								 strVal(llast(a->name)),
 								 lexpr, NULL,
 								 a->location);
@@ -2019,7 +2019,7 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
 										 sublink->testexpr, NULL,
 										 sublink->location);
 			else
-				emit_precedence_warnings(pstate, PREC_GROUP_POSTFIX_OP,
+				emit_precedence_warnings(pstate, PREC_GROUP_ANY_ALL,
 										 strVal(llast(sublink->operName)),
 										 sublink->testexpr, NULL,
 										 sublink->location);
@@ -3244,28 +3244,11 @@ operator_precedence_group(Node *node, const char **nodename)
 				group = PREC_GROUP_PREFIX_OP;
 			}
 		}
-		else if (aexpr->kind == AEXPR_OP &&
-				 aexpr->lexpr != NULL &&
-				 aexpr->rexpr == NULL)
-		{
-			/* postfix operator */
-			if (list_length(aexpr->name) == 1)
-			{
-				*nodename = strVal(linitial(aexpr->name));
-				group = PREC_GROUP_POSTFIX_OP;
-			}
-			else
-			{
-				/* schema-qualified operator syntax */
-				*nodename = "OPERATOR()";
-				group = PREC_GROUP_POSTFIX_OP;
-			}
-		}
 		else if (aexpr->kind == AEXPR_OP_ANY ||
 				 aexpr->kind == AEXPR_OP_ALL)
 		{
 			*nodename = strVal(llast(aexpr->name));
-			group = PREC_GROUP_POSTFIX_OP;
+			group = PREC_GROUP_ANY_ALL;
 		}
 		else if (aexpr->kind == AEXPR_DISTINCT ||
 				 aexpr->kind == AEXPR_NOT_DISTINCT)
@@ -3356,7 +3339,7 @@ operator_precedence_group(Node *node, const char **nodename)
 			else
 			{
 				*nodename = strVal(llast(s->operName));
-				group = PREC_GROUP_POSTFIX_OP;
+				group = PREC_GROUP_ANY_ALL;
 			}
 		}
 	}
@@ -3432,9 +3415,8 @@ emit_precedence_warnings(ParseState *pstate,
 	 * Complain if left child, which should be same or higher precedence
 	 * according to current rules, used to be lower precedence.
 	 *
-	 * Exception to precedence rules: if left child is IN or NOT IN or a
-	 * postfix operator, the grouping is syntactically forced regardless of
-	 * precedence.
+	 * Exception to precedence rules: if left child is IN or NOT IN the
+	 * grouping is syntactically forced regardless of precedence.
 	 */
 	cgroup = operator_precedence_group(lchild, &copname);
 	if (cgroup > 0)
@@ -3442,7 +3424,7 @@ emit_precedence_warnings(ParseState *pstate,
 		if (oldprecedence_l[cgroup] < oldprecedence_r[opgroup] &&
 			cgroup != PREC_GROUP_IN &&
 			cgroup != PREC_GROUP_NOT_IN &&
-			cgroup != PREC_GROUP_POSTFIX_OP &&
+			cgroup != PREC_GROUP_ANY_ALL &&
 			cgroup != PREC_GROUP_POSTFIX_IS)
 			ereport(WARNING,
 					(errmsg("operator precedence change: %s is now lower precedence than %s",
diff --git a/src/backend/parser/parse_oper.c b/src/backend/parser/parse_oper.c
index 2749974f638..6613a3a8f87 100644
--- a/src/backend/parser/parse_oper.c
+++ b/src/backend/parser/parse_oper.c
@@ -52,7 +52,7 @@ typedef struct OprCacheKey
 {
 	char		oprname[NAMEDATALEN];
 	Oid			left_arg;		/* Left input OID, or 0 if prefix op */
-	Oid			right_arg;		/* Right input OID, or 0 if postfix op */
+	Oid			right_arg;		/* Right input OID */
 	Oid			search_path[MAX_CACHED_PATH_LEN];
 } OprCacheKey;
 
@@ -88,8 +88,7 @@ static void InvalidateOprCacheCallBack(Datum arg, int cacheid, uint32 hashvalue)
  *		Given a possibly-qualified operator name and exact input datatypes,
  *		look up the operator.
  *
- * Pass oprleft = InvalidOid for a prefix op, oprright = InvalidOid for
- * a postfix op.
+ * Pass oprleft = InvalidOid for a prefix op.
  *
  * If the operator name is not schema-qualified, it is sought in the current
  * namespace search path.
@@ -115,10 +114,16 @@ LookupOperName(ParseState *pstate, List *opername, Oid oprleft, Oid oprright,
 
 		if (!OidIsValid(oprleft))
 			oprkind = 'l';
-		else if (!OidIsValid(oprright))
-			oprkind = 'r';
-		else
+		else if (OidIsValid(oprright))
 			oprkind = 'b';
+		else
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_SYNTAX_ERROR),
+					 errmsg("postfix operators are not supported"),
+					 parser_errposition(pstate, location)));
+			oprkind = 0;		/* keep compiler quiet */
+		}
 
 		ereport(ERROR,
 				(errcode(ERRCODE_UNDEFINED_FUNCTION),
@@ -507,85 +512,6 @@ compatible_oper_opid(List *op, Oid arg1, Oid arg2, bool noError)
 }
 
 
-/* right_oper() -- search for a unary right operator (postfix operator)
- * Given operator name and type of arg, return oper struct.
- *
- * IMPORTANT: the returned operator (if any) is only promised to be
- * coercion-compatible with the input datatype.  Do not use this if
- * you need an exact- or binary-compatible match.
- *
- * If no matching operator found, return NULL if noError is true,
- * raise an error if it is false.  pstate and location are used only to report
- * the error position; pass NULL/-1 if not available.
- *
- * NOTE: on success, the returned object is a syscache entry.  The caller
- * must ReleaseSysCache() the entry when done with it.
- */
-Operator
-right_oper(ParseState *pstate, List *op, Oid arg, bool noError, int location)
-{
-	Oid			operOid;
-	OprCacheKey key;
-	bool		key_ok;
-	FuncDetailCode fdresult = FUNCDETAIL_NOTFOUND;
-	HeapTuple	tup = NULL;
-
-	/*
-	 * Try to find the mapping in the lookaside cache.
-	 */
-	key_ok = make_oper_cache_key(pstate, &key, op, arg, InvalidOid, location);
-
-	if (key_ok)
-	{
-		operOid = find_oper_cache_entry(&key);
-		if (OidIsValid(operOid))
-		{
-			tup = SearchSysCache1(OPEROID, ObjectIdGetDatum(operOid));
-			if (HeapTupleIsValid(tup))
-				return (Operator) tup;
-		}
-	}
-
-	/*
-	 * First try for an "exact" match.
-	 */
-	operOid = OpernameGetOprid(op, arg, InvalidOid);
-	if (!OidIsValid(operOid))
-	{
-		/*
-		 * Otherwise, search for the most suitable candidate.
-		 */
-		FuncCandidateList clist;
-
-		/* Get postfix operators of given name */
-		clist = OpernameGetCandidates(op, 'r', false);
-
-		/* No operators found? Then fail... */
-		if (clist != NULL)
-		{
-			/*
-			 * We must run oper_select_candidate even if only one candidate,
-			 * otherwise we may falsely return a non-type-compatible operator.
-			 */
-			fdresult = oper_select_candidate(1, &arg, clist, &operOid);
-		}
-	}
-
-	if (OidIsValid(operOid))
-		tup = SearchSysCache1(OPEROID, ObjectIdGetDatum(operOid));
-
-	if (HeapTupleIsValid(tup))
-	{
-		if (key_ok)
-			make_oper_cache_entry(&key, operOid);
-	}
-	else if (!noError)
-		op_error(pstate, op, 'r', arg, InvalidOid, fdresult, location);
-
-	return (Operator) tup;
-}
-
-
 /* left_oper() -- search for a unary left operator (prefix operator)
  * Given operator name and type of arg, return oper struct.
  *
@@ -696,8 +622,7 @@ op_signature_string(List *op, char oprkind, Oid arg1, Oid arg2)
 
 	appendStringInfoString(&argbuf, NameListToString(op));
 
-	if (oprkind != 'r')
-		appendStringInfo(&argbuf, " %s", format_type_be(arg2));
+	appendStringInfo(&argbuf, " %s", format_type_be(arg2));
 
 	return argbuf.data;			/* return palloc'd string buffer */
 }
@@ -758,17 +683,16 @@ make_op(ParseState *pstate, List *opname, Node *ltree, Node *rtree,
 	Oid			rettype;
 	OpExpr	   *result;
 
-	/* Select the operator */
+	/* Check it's not a postfix operator */
 	if (rtree == NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("postfix operators are not supported")));
+
+	/* Select the operator */
+	if (ltree == NULL)
 	{
-		/* right operator */
-		ltypeId = exprType(ltree);
-		rtypeId = InvalidOid;
-		tup = right_oper(pstate, opname, ltypeId, false, location);
-	}
-	else if (ltree == NULL)
-	{
-		/* left operator */
+		/* prefix operator */
 		rtypeId = exprType(rtree);
 		ltypeId = InvalidOid;
 		tup = left_oper(pstate, opname, rtypeId, false, location);
@@ -795,17 +719,9 @@ make_op(ParseState *pstate, List *opname, Node *ltree, Node *rtree,
 				 parser_errposition(pstate, location)));
 
 	/* Do typecasting and build the expression tree */
-	if (rtree == NULL)
-	{
-		/* right operator */
-		args = list_make1(ltree);
-		actual_arg_types[0] = ltypeId;
-		declared_arg_types[0] = opform->oprleft;
-		nargs = 1;
-	}
-	else if (ltree == NULL)
+	if (ltree == NULL)
 	{
-		/* left operator */
+		/* prefix operator */
 		args = list_make1(rtree);
 		actual_arg_types[0] = rtypeId;
 		declared_arg_types[0] = opform->oprright;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 60dd80c23c8..15877e37a60 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -9198,35 +9198,14 @@ get_oper_expr(OpExpr *expr, deparse_context *context)
 	}
 	else
 	{
-		/* unary operator --- but which side? */
+		/* prefix operator */
 		Node	   *arg = (Node *) linitial(args);
-		HeapTuple	tp;
-		Form_pg_operator optup;
-
-		tp = SearchSysCache1(OPEROID, ObjectIdGetDatum(opno));
-		if (!HeapTupleIsValid(tp))
-			elog(ERROR, "cache lookup failed for operator %u", opno);
-		optup = (Form_pg_operator) GETSTRUCT(tp);
-		switch (optup->oprkind)
-		{
-			case 'l':
-				appendStringInfo(buf, "%s ",
-								 generate_operator_name(opno,
-														InvalidOid,
-														exprType(arg)));
-				get_rule_expr_paren(arg, context, true, (Node *) expr);
-				break;
-			case 'r':
-				get_rule_expr_paren(arg, context, true, (Node *) expr);
-				appendStringInfo(buf, " %s",
-								 generate_operator_name(opno,
-														exprType(arg),
-														InvalidOid));
-				break;
-			default:
-				elog(ERROR, "bogus oprkind: %d", optup->oprkind);
-		}
-		ReleaseSysCache(tp);
+
+		appendStringInfo(buf, "%s ",
+						 generate_operator_name(opno,
+												InvalidOid,
+												exprType(arg)));
+		get_rule_expr_paren(arg, context, true, (Node *) expr);
 	}
 	if (!PRETTY_PAREN(context))
 		appendStringInfoChar(buf, ')');
@@ -11087,10 +11066,6 @@ generate_operator_name(Oid operid, Oid arg1, Oid arg2)
 			p_result = left_oper(NULL, list_make1(makeString(oprname)), arg2,
 								 true, -1);
 			break;
-		case 'r':
-			p_result = right_oper(NULL, list_make1(makeString(oprname)), arg1,
-								  true, -1);
-			break;
 		default:
 			elog(ERROR, "unrecognized oprkind: %d", operform->oprkind);
 			p_result = NULL;	/* keep compiler quiet */