On 26-02-2016 12:46, Roma Sokolov wrote:
> Regression tests are added to check DROP OPERATOR behaves as intended 
> (including
> case with self-commutator and unlikely case with operator being both negator 
> and
> commutator).
> 
I don't think those are mandatory.

> Should this patch be added to CommitFest?
> 
Why not?

I didn't test your patch but

+  if (isDelete ? (t->oprcom == baseId || t->oprnegate == baseId)
+          : !OidIsValid(t->oprcom) || !OidIsValid(t->oprnegate))

... is hard to understand. Instead, you could separate the conditional
expression into a variable.

+ if (isDelete ? t->oprnegate == baseId : !OidIsValid(t->oprnegate))

It could be separate into a variable to be readable (or at least deserve
a comment).

(isDelete ? InvalidOid : ObjectIdGetDatum(baseId))

... and this one too. It is used in 4 places in that function.


-- 
   Euler Taveira                   Timbira - http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to