| From: | Yury Zhuravlev <u(dot)zhuravlev(at)postgrespro(dot)ru> |
|---|---|
| To: | <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: [PATCH] fix DROP OPERATOR to reset links to itself on commutator and negator |
| Date: | 2016-02-27 18:59:29 |
| Message-ID: | 8d4eaf35-091c-4171-84fb-e67283f4848f@postgrespro.ru |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Roma Sokolov wrote:
> See v2 of the patch attached.
Hello.
I have a stylistic comments. Sometimes you forget a space:
+ replaces[Anum_pg_operator_oprcom - 1] =true;
or use tab insted space:
+ if (OidIsValid(op->oprnegate) ||
+ (OidIsValid(op->oprcom) && operOid != op->oprcom))
+ OperatorUpd(operOid,
+ operOid == op->oprcom ? InvalidOid : op->oprcom,
+ op->oprnegate,
+ true);
And I think if you make this logic into a separate function,
it is possible to simplify the code. OperatorUpd function is too complex.
Also better to add comments to the tests.
The rest seems good.
PS I here thought it would be possible to print operators that have been
changed?
--
Yury Zhuravlev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Masahiko Sawada | 2016-02-27 19:04:37 | Re: Support for N synchronous standby servers - take 2 |
| Previous Message | Álvaro Hernández Tortosa | 2016-02-27 18:51:27 | Re: PostgreSQL extension API? Documentation? |