diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2020-09-17 19:38:05 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2020-09-17 19:38:05 -0400 |
commit | 1ed6b895634ce0dc5fd4bd040e87252b32182cba (patch) | |
tree | d0f22d227e7df8ca5139baf4eba578de052f6a82 /src | |
parent | 76f412ab310554acb970a0b73c8d1f37f35548c6 (diff) | |
download | postgresql-1ed6b895634ce0dc5fd4bd040e87252b32182cba.tar.gz postgresql-1ed6b895634ce0dc5fd4bd040e87252b32182cba.zip |
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
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/catalog/namespace.c | 7 | ||||
-rw-r--r-- | src/backend/catalog/pg_operator.c | 4 | ||||
-rw-r--r-- | src/backend/commands/operatorcmds.c | 14 | ||||
-rw-r--r-- | src/backend/nodes/print.c | 1 | ||||
-rw-r--r-- | src/backend/parser/gram.y | 13 | ||||
-rw-r--r-- | src/backend/parser/parse_expr.c | 38 | ||||
-rw-r--r-- | src/backend/parser/parse_oper.c | 128 | ||||
-rw-r--r-- | src/backend/utils/adt/ruleutils.c | 39 | ||||
-rw-r--r-- | src/bin/pg_dump/pg_dump.c | 8 | ||||
-rw-r--r-- | src/bin/pg_upgrade/check.c | 106 | ||||
-rw-r--r-- | src/bin/psql/describe.c | 4 | ||||
-rw-r--r-- | src/include/catalog/catversion.h | 2 | ||||
-rw-r--r-- | src/include/catalog/pg_operator.h | 6 | ||||
-rw-r--r-- | src/include/parser/parse_oper.h | 2 | ||||
-rw-r--r-- | src/test/regress/expected/create_operator.out | 50 | ||||
-rw-r--r-- | src/test/regress/expected/opr_sanity.out | 17 | ||||
-rw-r--r-- | src/test/regress/sql/create_operator.sql | 36 | ||||
-rw-r--r-- | src/test/regress/sql/opr_sanity.sql | 14 | ||||
-rw-r--r-- | src/tutorial/complex.source | 2 | ||||
-rw-r--r-- | src/tutorial/syscat.source | 24 |
20 files changed, 242 insertions, 273 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 */ diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 320820165b7..dce6af09c9b 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -12650,6 +12650,11 @@ dumpOpr(Archive *fout, OprInfo *oprinfo) oprcanmerge = PQgetvalue(res, 0, i_oprcanmerge); oprcanhash = PQgetvalue(res, 0, i_oprcanhash); + /* In PG14 upwards postfix operator support does not exist anymore. */ + if (strcmp(oprkind, "r") == 0) + pg_log_warning("postfix operators are not supported anymore (operator \"%s\")", + oprcode); + oprregproc = convertRegProcReference(fout, oprcode); if (oprregproc) { @@ -12662,7 +12667,8 @@ dumpOpr(Archive *fout, OprInfo *oprinfo) /* * right unary means there's a left arg and left unary means there's a - * right arg + * right arg. (Although the "r" case is dead code for PG14 and later, + * continue to support it in case we're dumping from an old server.) */ if (strcmp(oprkind, "r") == 0 || strcmp(oprkind, "b") == 0) diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 00aef855dc0..2f7aa632c52 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -22,6 +22,7 @@ static void check_is_install_user(ClusterInfo *cluster); static void check_proper_datallowconn(ClusterInfo *cluster); static void check_for_prepared_transactions(ClusterInfo *cluster); static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster); +static void check_for_user_defined_postfix_ops(ClusterInfo *cluster); static void check_for_tables_with_oids(ClusterInfo *cluster); static void check_for_reg_data_type_usage(ClusterInfo *cluster); static void check_for_jsonb_9_4_usage(ClusterInfo *cluster); @@ -101,6 +102,13 @@ check_and_dump_old_cluster(bool live_check) check_for_isn_and_int8_passing_mismatch(&old_cluster); /* + * Pre-PG 14 allowed user defined postfix operators, which are not + * supported anymore. Verify there are none, iff applicable. + */ + if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1300) + check_for_user_defined_postfix_ops(&old_cluster); + + /* * Pre-PG 12 allowed tables to be declared WITH OIDS, which is not * supported anymore. Verify there are none, iff applicable. */ @@ -896,6 +904,104 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster) check_ok(); } +/* + * Verify that no user defined postfix operators exist. + */ +static void +check_for_user_defined_postfix_ops(ClusterInfo *cluster) +{ + int dbnum; + FILE *script = NULL; + bool found = false; + char output_path[MAXPGPATH]; + + prep_status("Checking for user-defined postfix operators"); + + snprintf(output_path, sizeof(output_path), + "postfix_ops.txt"); + + /* Find any user defined postfix operators */ + for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++) + { + PGresult *res; + bool db_used = false; + int ntups; + int rowno; + int i_oproid, + i_oprnsp, + i_oprname, + i_typnsp, + i_typname; + DbInfo *active_db = &cluster->dbarr.dbs[dbnum]; + PGconn *conn = connectToServer(cluster, active_db->db_name); + + /* + * The query below hardcodes FirstNormalObjectId as 16384 rather than + * interpolating that C #define into the query because, if that + * #define is ever changed, the cutoff we want to use is the value + * used by pre-version 14 servers, not that of some future version. + */ + res = executeQueryOrDie(conn, + "SELECT o.oid AS oproid, " + " n.nspname AS oprnsp, " + " o.oprname, " + " tn.nspname AS typnsp, " + " t.typname " + "FROM pg_catalog.pg_operator o, " + " pg_catalog.pg_namespace n, " + " pg_catalog.pg_type t, " + " pg_catalog.pg_namespace tn " + "WHERE o.oprnamespace = n.oid AND " + " o.oprleft = t.oid AND " + " t.typnamespace = tn.oid AND " + " o.oprright = 0 AND " + " o.oid >= 16384"); + ntups = PQntuples(res); + i_oproid = PQfnumber(res, "oproid"); + i_oprnsp = PQfnumber(res, "oprnsp"); + i_oprname = PQfnumber(res, "oprname"); + i_typnsp = PQfnumber(res, "typnsp"); + i_typname = PQfnumber(res, "typname"); + for (rowno = 0; rowno < ntups; rowno++) + { + found = true; + if (script == NULL && + (script = fopen_priv(output_path, "w")) == NULL) + pg_fatal("could not open file \"%s\": %s\n", + output_path, strerror(errno)); + if (!db_used) + { + fprintf(script, "In database: %s\n", active_db->db_name); + db_used = true; + } + fprintf(script, " (oid=%s) %s.%s (%s.%s, NONE)\n", + PQgetvalue(res, rowno, i_oproid), + PQgetvalue(res, rowno, i_oprnsp), + PQgetvalue(res, rowno, i_oprname), + PQgetvalue(res, rowno, i_typnsp), + PQgetvalue(res, rowno, i_typname)); + } + + PQclear(res); + + PQfinish(conn); + } + + if (script) + fclose(script); + + if (found) + { + pg_log(PG_REPORT, "fatal\n"); + pg_fatal("Your installation contains user-defined postfix operators, which are not\n" + "supported anymore. Consider dropping the postfix operators and replacing\n" + "them with prefix operators or function calls.\n" + "A list of user-defined postfix operators is in the file:\n" + " %s\n\n", output_path); + } + else + check_ok(); +} /* * Verify that no tables are declared WITH OIDS. diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index f22d907b1f9..58de433fd30 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -799,6 +799,10 @@ describeOperators(const char *pattern, bool verbose, bool showSystem) * anyway, for now, because (1) third-party modules may still be following * the old convention, and (2) we'd need to do it anyway when talking to a * pre-9.1 server. + * + * The support for postfix operators in this query is dead code as of + * Postgres 14, but we need to keep it for as long as we support talking + * to pre-v14 servers. */ printfPQExpBuffer(&buf, diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 35893599708..365552635b8 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202009171 +#define CATALOG_VERSION_NO 202009172 #endif diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h index 1daa2638520..62a7dbf23f6 100644 --- a/src/include/catalog/pg_operator.h +++ b/src/include/catalog/pg_operator.h @@ -41,7 +41,7 @@ CATALOG(pg_operator,2617,OperatorRelationId) /* operator owner */ Oid oprowner BKI_DEFAULT(PGUID); - /* 'l', 'r', or 'b' */ + /* 'l' for prefix or 'b' for infix */ char oprkind BKI_DEFAULT(b); /* can be used in merge join? */ @@ -50,10 +50,10 @@ CATALOG(pg_operator,2617,OperatorRelationId) /* can be used in hash join? */ bool oprcanhash BKI_DEFAULT(f); - /* left arg type, or 0 if 'l' oprkind */ + /* left arg type, or 0 if prefix operator */ Oid oprleft BKI_LOOKUP(pg_type); - /* right arg type, or 0 if 'r' oprkind */ + /* right arg type */ Oid oprright BKI_LOOKUP(pg_type); /* result datatype */ diff --git a/src/include/parser/parse_oper.h b/src/include/parser/parse_oper.h index bcd861e43ac..09695a2765c 100644 --- a/src/include/parser/parse_oper.h +++ b/src/include/parser/parse_oper.h @@ -31,8 +31,6 @@ extern Oid LookupOperWithArgs(ObjectWithArgs *oper, bool noError); /* NB: the selected operator may require coercion of the input types! */ extern Operator oper(ParseState *pstate, List *op, Oid arg1, Oid arg2, bool noError, int location); -extern Operator right_oper(ParseState *pstate, List *op, Oid arg, - bool noError, int location); extern Operator left_oper(ParseState *pstate, List *op, Oid arg, bool noError, int location); diff --git a/src/test/regress/expected/create_operator.out b/src/test/regress/expected/create_operator.out index 9e4d4e93fb7..53032775914 100644 --- a/src/test/regress/expected/create_operator.out +++ b/src/test/regress/expected/create_operator.out @@ -15,17 +15,15 @@ CREATE OPERATOR <% ( negator = >=% ); CREATE OPERATOR @#@ ( - rightarg = int8, -- left unary - procedure = factorial -); -CREATE OPERATOR #@# ( - leftarg = int8, -- right unary + rightarg = int8, -- prefix procedure = factorial ); CREATE OPERATOR #%# ( - leftarg = int8, -- right unary + leftarg = int8, -- fail, postfix is no longer supported procedure = factorial ); +ERROR: operator right argument type must be specified +DETAIL: Postfix operators are not supported. -- Test operator created above SELECT point '(1,2)' <% widget '(0,0,3)' AS t, point '(1,2)' <% widget '(0,0,1)' AS f; @@ -35,11 +33,22 @@ SELECT point '(1,2)' <% widget '(0,0,3)' AS t, (1 row) -- Test comments -COMMENT ON OPERATOR ###### (int4, NONE) IS 'bad right unary'; -ERROR: operator does not exist: integer ###### --- => is disallowed now +COMMENT ON OPERATOR ###### (NONE, int4) IS 'bad prefix'; +ERROR: operator does not exist: ###### integer +COMMENT ON OPERATOR ###### (int4, NONE) IS 'bad postfix'; +ERROR: postfix operators are not supported +COMMENT ON OPERATOR ###### (int4, int8) IS 'bad infix'; +ERROR: operator does not exist: integer ###### bigint +-- Check that DROP on a nonexistent op behaves sanely, too +DROP OPERATOR ###### (NONE, int4); +ERROR: operator does not exist: ###### integer +DROP OPERATOR ###### (int4, NONE); +ERROR: postfix operators are not supported +DROP OPERATOR ###### (int4, int8); +ERROR: operator does not exist: integer ###### bigint +-- => is disallowed as an operator name now CREATE OPERATOR => ( - leftarg = int8, -- right unary + rightarg = int8, procedure = factorial ); ERROR: syntax error at or near "=>" @@ -49,15 +58,20 @@ LINE 1: CREATE OPERATOR => ( -- (=> is tested elsewhere) -- this is legal because ! is not allowed in sql ops CREATE OPERATOR !=- ( - leftarg = int8, -- right unary + rightarg = int8, procedure = factorial ); -SELECT 2 !=-; +SELECT !=- 10; ?column? ---------- - 2 + 3628800 (1 row) +-- postfix operators don't work anymore +SELECT 10 !=-; +ERROR: syntax error at or near ";" +LINE 1: SELECT 10 !=-; + ^ -- make sure lexer returns != as <> even in edge cases SELECT 2 !=/**/ 1, 2 !=/**/ 2; ?column? | ?column? @@ -127,7 +141,7 @@ GRANT USAGE ON SCHEMA schema_op1 TO PUBLIC; REVOKE USAGE ON SCHEMA schema_op1 FROM regress_rol_op1; SET ROLE regress_rol_op1; CREATE OPERATOR schema_op1.#*# ( - leftarg = int8, -- right unary + rightarg = int8, procedure = factorial ); ERROR: permission denied for schema schema_op1 @@ -167,19 +181,19 @@ CREATE OPERATOR === ( ROLLBACK; -- Should fail. Invalid attribute CREATE OPERATOR #@%# ( - leftarg = int8, -- right unary + rightarg = int8, procedure = factorial, invalid_att = int8 ); WARNING: operator attribute "invalid_att" not recognized --- Should fail. At least leftarg or rightarg should be mandatorily specified +-- Should fail. At least rightarg should be mandatorily specified CREATE OPERATOR #@%# ( procedure = factorial ); -ERROR: at least one of leftarg or rightarg must be specified +ERROR: operator argument types must be specified -- Should fail. Procedure should be mandatorily specified CREATE OPERATOR #@%# ( - leftarg = int8 + rightarg = int8 ); ERROR: operator function must be specified -- Should fail. CREATE OPERATOR requires USAGE on TYPE diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out index 1b3c146e4cc..7825a765cd7 100644 --- a/src/test/regress/expected/opr_sanity.out +++ b/src/test/regress/expected/opr_sanity.out @@ -1066,7 +1066,7 @@ WHERE condefault AND -- Look for illegal values in pg_operator fields. SELECT p1.oid, p1.oprname FROM pg_operator as p1 -WHERE (p1.oprkind != 'b' AND p1.oprkind != 'l' AND p1.oprkind != 'r') OR +WHERE (p1.oprkind != 'b' AND p1.oprkind != 'l') OR p1.oprresult = 0 OR p1.oprcode = 0; oid | oprname -----+--------- @@ -1077,8 +1077,7 @@ SELECT p1.oid, p1.oprname FROM pg_operator as p1 WHERE (p1.oprleft = 0 and p1.oprkind != 'l') OR (p1.oprleft != 0 and p1.oprkind = 'l') OR - (p1.oprright = 0 and p1.oprkind != 'r') OR - (p1.oprright != 0 and p1.oprkind = 'r'); + p1.oprright = 0; oid | oprname -----+--------- (0 rows) @@ -1285,18 +1284,6 @@ WHERE p1.oprcode = p2.oid AND -----+---------+-----+--------- (0 rows) -SELECT p1.oid, p1.oprname, p2.oid, p2.proname -FROM pg_operator AS p1, pg_proc AS p2 -WHERE p1.oprcode = p2.oid AND - p1.oprkind = 'r' AND - (p2.pronargs != 1 - OR NOT binary_coercible(p2.prorettype, p1.oprresult) - OR NOT binary_coercible(p1.oprleft, p2.proargtypes[0]) - OR p1.oprright != 0); - oid | oprname | oid | proname ------+---------+-----+--------- -(0 rows) - -- If the operator is mergejoinable or hashjoinable, its underlying function -- should not be volatile. SELECT p1.oid, p1.oprname, p2.oid, p2.proname diff --git a/src/test/regress/sql/create_operator.sql b/src/test/regress/sql/create_operator.sql index c32da8c066a..4ff2c0ff216 100644 --- a/src/test/regress/sql/create_operator.sql +++ b/src/test/regress/sql/create_operator.sql @@ -18,17 +18,12 @@ CREATE OPERATOR <% ( ); CREATE OPERATOR @#@ ( - rightarg = int8, -- left unary - procedure = factorial -); - -CREATE OPERATOR #@# ( - leftarg = int8, -- right unary + rightarg = int8, -- prefix procedure = factorial ); CREATE OPERATOR #%# ( - leftarg = int8, -- right unary + leftarg = int8, -- fail, postfix is no longer supported procedure = factorial ); @@ -37,11 +32,18 @@ SELECT point '(1,2)' <% widget '(0,0,3)' AS t, point '(1,2)' <% widget '(0,0,1)' AS f; -- Test comments -COMMENT ON OPERATOR ###### (int4, NONE) IS 'bad right unary'; +COMMENT ON OPERATOR ###### (NONE, int4) IS 'bad prefix'; +COMMENT ON OPERATOR ###### (int4, NONE) IS 'bad postfix'; +COMMENT ON OPERATOR ###### (int4, int8) IS 'bad infix'; + +-- Check that DROP on a nonexistent op behaves sanely, too +DROP OPERATOR ###### (NONE, int4); +DROP OPERATOR ###### (int4, NONE); +DROP OPERATOR ###### (int4, int8); --- => is disallowed now +-- => is disallowed as an operator name now CREATE OPERATOR => ( - leftarg = int8, -- right unary + rightarg = int8, procedure = factorial ); @@ -50,10 +52,12 @@ CREATE OPERATOR => ( -- this is legal because ! is not allowed in sql ops CREATE OPERATOR !=- ( - leftarg = int8, -- right unary + rightarg = int8, procedure = factorial ); -SELECT 2 !=-; +SELECT !=- 10; +-- postfix operators don't work anymore +SELECT 10 !=-; -- make sure lexer returns != as <> even in edge cases SELECT 2 !=/**/ 1, 2 !=/**/ 2; SELECT 2 !=-- comment to be removed by psql @@ -84,7 +88,7 @@ GRANT USAGE ON SCHEMA schema_op1 TO PUBLIC; REVOKE USAGE ON SCHEMA schema_op1 FROM regress_rol_op1; SET ROLE regress_rol_op1; CREATE OPERATOR schema_op1.#*# ( - leftarg = int8, -- right unary + rightarg = int8, procedure = factorial ); ROLLBACK; @@ -128,19 +132,19 @@ ROLLBACK; -- Should fail. Invalid attribute CREATE OPERATOR #@%# ( - leftarg = int8, -- right unary + rightarg = int8, procedure = factorial, 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 = factorial ); -- Should fail. Procedure should be mandatorily specified CREATE OPERATOR #@%# ( - leftarg = int8 + rightarg = int8 ); -- Should fail. CREATE OPERATOR requires USAGE on TYPE diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql index 7a9180b0815..307aab1deb7 100644 --- a/src/test/regress/sql/opr_sanity.sql +++ b/src/test/regress/sql/opr_sanity.sql @@ -571,7 +571,7 @@ WHERE condefault AND SELECT p1.oid, p1.oprname FROM pg_operator as p1 -WHERE (p1.oprkind != 'b' AND p1.oprkind != 'l' AND p1.oprkind != 'r') OR +WHERE (p1.oprkind != 'b' AND p1.oprkind != 'l') OR p1.oprresult = 0 OR p1.oprcode = 0; -- Look for missing or unwanted operand types @@ -580,8 +580,7 @@ SELECT p1.oid, p1.oprname FROM pg_operator as p1 WHERE (p1.oprleft = 0 and p1.oprkind != 'l') OR (p1.oprleft != 0 and p1.oprkind = 'l') OR - (p1.oprright = 0 and p1.oprkind != 'r') OR - (p1.oprright != 0 and p1.oprkind = 'r'); + p1.oprright = 0; -- Look for conflicting operator definitions (same names and input datatypes). @@ -715,15 +714,6 @@ WHERE p1.oprcode = p2.oid AND OR NOT binary_coercible(p1.oprright, p2.proargtypes[0]) OR p1.oprleft != 0); -SELECT p1.oid, p1.oprname, p2.oid, p2.proname -FROM pg_operator AS p1, pg_proc AS p2 -WHERE p1.oprcode = p2.oid AND - p1.oprkind = 'r' AND - (p2.pronargs != 1 - OR NOT binary_coercible(p2.prorettype, p1.oprresult) - OR NOT binary_coercible(p1.oprleft, p2.proargtypes[0]) - OR p1.oprright != 0); - -- If the operator is mergejoinable or hashjoinable, its underlying function -- should not be volatile. diff --git a/src/tutorial/complex.source b/src/tutorial/complex.source index 03559267016..d849ec0d4b7 100644 --- a/src/tutorial/complex.source +++ b/src/tutorial/complex.source @@ -111,7 +111,7 @@ CREATE FUNCTION complex_add(complex, complex) LANGUAGE C IMMUTABLE STRICT; -- we can now define the operator. We show a binary operator here but you --- can also define unary operators by omitting either of leftarg or rightarg. +-- can also define a prefix operator by omitting the leftarg. CREATE OPERATOR + ( leftarg = complex, rightarg = complex, diff --git a/src/tutorial/syscat.source b/src/tutorial/syscat.source index 3a1767f97be..8a04d6a961f 100644 --- a/src/tutorial/syscat.source +++ b/src/tutorial/syscat.source @@ -96,36 +96,22 @@ SELECT n.nspname, r.rolname, format_type(t.oid, null) as typname -- --- lists all left unary operators +-- lists all prefix operators -- -SELECT n.nspname, o.oprname AS left_unary, +SELECT n.nspname, o.oprname AS prefix_op, format_type(right_type.oid, null) AS operand, format_type(result.oid, null) AS return_type FROM pg_namespace n, pg_operator o, pg_type right_type, pg_type result WHERE o.oprnamespace = n.oid - and o.oprkind = 'l' -- left unary + and o.oprkind = 'l' -- prefix ("left unary") and o.oprright = right_type.oid and o.oprresult = result.oid ORDER BY nspname, operand; -- --- lists all right unary operators --- -SELECT n.nspname, o.oprname AS right_unary, - format_type(left_type.oid, null) AS operand, - format_type(result.oid, null) AS return_type - FROM pg_namespace n, pg_operator o, - pg_type left_type, pg_type result - WHERE o.oprnamespace = n.oid - and o.oprkind = 'r' -- right unary - and o.oprleft = left_type.oid - and o.oprresult = result.oid - ORDER BY nspname, operand; - --- --- lists all binary operators +-- lists all infix operators -- SELECT n.nspname, o.oprname AS binary_op, format_type(left_type.oid, null) AS left_opr, @@ -134,7 +120,7 @@ SELECT n.nspname, o.oprname AS binary_op, FROM pg_namespace n, pg_operator o, pg_type left_type, pg_type right_type, pg_type result WHERE o.oprnamespace = n.oid - and o.oprkind = 'b' -- binary + and o.oprkind = 'b' -- infix ("binary") and o.oprleft = left_type.oid and o.oprright = right_type.oid and o.oprresult = result.oid |