Re: [HACKERS] parser dilemma

Tom Lane írta:
> Andrew Dunstan <andrew@dunslane.net> writes:
>
>> Tom Lane wrote:
>>
>>> So I think attaching a precedence to the GENERATED keyword is dangerous.
>>>
>
>
>> Especially when we have a good workaround which would just require use
>> of ()  around certain postfix-operator expressions.
>>
>
> Yeah, I'm leaning to the idea that removing postfix operators from
> b_expr is the least bad solution.
>
> One thing that would have to be looked at is the rules in ruleutils.c
> for suppressing "unnecessary" parentheses when reverse-listing
> parsetrees.  It might be safest to just never suppress them around a
> postfix operator.
>
>             regards, tom lane
>

You mean something like this?

-----------------------------------------------------
***************
*** 4138,4157 ****
        Oid                     opno = expr->opno;
        List       *args = expr->args;

-       if (!PRETTY_PAREN(context))
-               appendStringInfoChar(buf, '(');
        if (list_length(args) == 2)
        {
                /* binary operator */
                Node       *arg1 = (Node *) linitial(args);
                Node       *arg2 = (Node *) lsecond(args);

                get_rule_expr_paren(arg1, context, true, (Node *) expr);
                appendStringInfo(buf, " %s ",

generate_operator_name(opno,

exprType(arg1),

exprType(arg2)));
                get_rule_expr_paren(arg2, context, true, (Node *) expr);
        }
        else
        {
--- 4095,4118 ----
        Oid                     opno = expr->opno;
        List       *args = expr->args;

        if (list_length(args) == 2)
        {
                /* binary operator */
                Node       *arg1 = (Node *) linitial(args);
                Node       *arg2 = (Node *) lsecond(args);

+               if (!PRETTY_PAREN(context))
+                       appendStringInfoChar(buf, '(');
+
                get_rule_expr_paren(arg1, context, true, (Node *) expr);
                appendStringInfo(buf, " %s ",

generate_operator_name(opno,

exprType(arg1),

exprType(arg2)));
                get_rule_expr_paren(arg2, context, true, (Node *) expr);
+
+               if (!PRETTY_PAREN(context))
+                       appendStringInfoChar(buf, ')');
        }
        else
        {
***************
*** 4169,4194 ****
                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);
        }
-       if (!PRETTY_PAREN(context))
-               appendStringInfoChar(buf, ')');
  }

  /*
--- 4130,4159 ----
                switch (optup->oprkind)
                {
                        case 'l':
+                               if (!PRETTY_PAREN(context))
+                                       appendStringInfoChar(buf, '(');
                                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, ')');
                                break;
                        case 'r':
+                               appendStringInfoChar(buf, '(');
                                get_rule_expr_paren(arg, context, true,
(Node *) expr);
                                appendStringInfo(buf, " %s",

generate_operator_name(opno,

exprType(arg),

InvalidOid));
+                               appendStringInfoChar(buf, ')');
                                break;
                        default:
                                elog(ERROR, "bogus oprkind: %d",
optup->oprkind);
                }
                ReleaseSysCache(tp);
        }
  }

  /*
-----------------------------------------------------

It seems to work:

-----------------------------------------------------
$ psql
Welcome to psql 8.3devel, the PostgreSQL interactive terminal.

Type:  \copyright for distribution terms
       \h for help with SQL commands
       \? for help with psql commands
       \g or terminate with semicolon to execute query
       \q to quit

zozo=# create table t1 (i integer default 5! generated always as
identity primary key, t text not null unique);
ERROR:  syntax error at or near "always"
LINE 1: create table t1 (i integer default 5! generated always as id...
                                                        ^
zozo=# create table t1 (i integer default (5!) generated always as
identity primary key, t text not null unique);
NOTICE:  CREATE TABLE will create implicit sequence "t1_i_seq" for
serial column "t1.i"
ERROR:  multiple default values specified for column "i" of table "t1"
zozo=# create table t1 (i integer default (5!) primary key, t text not
null unique);
NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "t1_pkey"
for table "t1"
NOTICE:  CREATE TABLE / UNIQUE will create implicit index "t1_t_key" for
table "t1"
CREATE TABLE
zozo=# \d t1
                  Table "public.t1"
 Column |  Type   |            Modifiers
--------+---------+----------------------------------
 i      | integer | not null default ((5)::bigint !)
 t      | text    | not null
Indexes:
    "t1_pkey" PRIMARY KEY, btree (i)
    "t1_t_key" UNIQUE, btree (t)

zozo=# \q
$ pg_dump
...
--
-- Name: t1; Type: TABLE; Schema: public; Owner: zozo; Tablespace:
--

CREATE TABLE t1 (
    i integer DEFAULT ((5)::bigint !) NOT NULL,
    t text NOT NULL
);

...

--
-- PostgreSQL database dump complete
--
-----------------------------------------------------

So, if I send the patch again (POSTFIXOP deleted from b_expr)
with the above added, will it get accepted?

Best regards,
Zoltán Böszörményi

--
----------------------------------
Zoltán Böszörményi
Cybertec Geschwinde & Schönig GmbH
http://www.postgresql.at/