From bc4291a8307fa299d96e99a0a98348c451a3eab6 Mon Sep 17 00:00:00 2001 From: Mark Dilger 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)