Re: [HACKERS] parser dilemma

From: Zoltan Boszormenyi <zb(at)cybertec(dot)at>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Martijn van Oosterhout <kleptog(at)svana(dot)org>, List pgsql-patches <pgsql-patches(at)postgresql(dot)org>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, "Florian G(dot) Pflug" <fgp(at)phlo(dot)org>, Bruce Momjian <bruce(at)momjian(dot)us>
Subject: Re: [HACKERS] parser dilemma
Date: 2007-04-25 08:31:56
Message-ID: 462F11FC.3030502@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Tom Lane írta:
> Andrew Dunstan <andrew(at)dunslane(dot)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/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message ITAGAKI Takahiro 2007-04-25 09:16:39 Load distributed checkpoint V4.1
Previous Message ITAGAKI Takahiro 2007-04-25 08:27:15 autovacuum does not start in HEAD

Browse pgsql-patches by date

  From Date Subject
Next Message ITAGAKI Takahiro 2007-04-25 09:16:39 Load distributed checkpoint V4.1
Previous Message ITAGAKI Takahiro 2007-04-25 08:27:15 autovacuum does not start in HEAD