Re: [PATCH] fix DROP OPERATOR to reset links to itself on commutator and negator

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,
Suporte24x7 e Treinamento