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

On Fri, Mar 25, 2016 at 12:39 PM, Tom Lane wrote: > I wrote: >> Robert Haas writes: >>> On Wed, Mar 23, 2016 at 12:27 PM, Tom Lane wrote: We could resolve both of these issues by changing the semantics of OprUpdate so that

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

I wrote: > Robert Haas writes: >> On Wed, Mar 23, 2016 at 12:27 PM, Tom Lane wrote: >>> We could resolve both of these issues by changing the semantics of >>> OprUpdate so that it unconditionally does a CommandCounterIncrement >>> after each update that

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

Robert Haas writes: > On Wed, Mar 23, 2016 at 12:27 PM, Tom Lane wrote: >> I'm also a bit dubious of the assumption in RemoveOperatorById that an >> operator can't be its own negator. Yeah, that should not be the case, >> but if it is the case the

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

On Wed, Mar 23, 2016 at 12:27 PM, Tom Lane wrote: > Robert Haas writes: >> I did not find this version very clear. It wasn't consistent about >> using ObjectIdGetDatum() where needed, but the bigger problem was that >> I found the logic unnecessarily

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

Robert Haas writes: > I did not find this version very clear. It wasn't consistent about > using ObjectIdGetDatum() where needed, but the bigger problem was that > I found the logic unnecessarily convoluted. I rewrote it - I believe > more straightforwardly - as attached.

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

On Tue, Mar 22, 2016 at 9:25 PM, Tomas Vondra wrote: > OK, the new code seems more comprehensible to me. I did not find this version very clear. It wasn't consistent about using ObjectIdGetDatum() where needed, but the bigger problem was that I found the logic

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

Hi, On 03/23/2016 12:50 AM, Roma Sokolov wrote: Hi, Tomas, thanks for review and comments! I have to admit I find the existing code a bit convoluted, particularly the part that deals with the (commId == negId) case. And the patch does not really improve the situation, quite the contrary.

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

Hi, Tomas, thanks for review and comments! > I have to admit I find the existing code a bit convoluted, particularly the > part that deals with the (commId == negId) case. And the patch does not > really improve the situation, quite the contrary. > > Perhaps it’s time to get rid of this

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

Hi, On 03/01/2016 05:08 PM, Roma Sokolov wrote: On 27 Feb 2016, at 03:46, Euler Taveira wrote: Because it is not a common practice to test catalog dependency on separate tests (AFAICS initial catalogs are tested with oidjoins.sql). Also, your test case is too huge for

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

> On 27 Feb 2016, at 03:46, Euler Taveira wrote: > Because it is not a common practice to test catalog dependency on > separate tests (AFAICS initial catalogs are tested with oidjoins.sql). > Also, your test case is too huge for such a small use case. It seems oidjoins.sql

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

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 !=

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

On Sat, Feb 27, 2016 at 12:46 AM, Roma Sokolov wrote: > Should this patch be added to CommitFest? Yes please. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

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

On 26-02-2016 17:51, Roma Sokolov wrote: > Fixed the patch to be more descriptive and to avoid repeating same > computation over and over again. See v2 of the patch attached. > Oh, much better. > Why do you think that? Should I remove them or maybe send as separate > patch? > Because it is not

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

Thanks for comments, Euler! > ... is hard to understand. Instead, you could separate the conditional > expression into a variable. Fixed the patch to be more descriptive and to avoid repeating same computation over and over again. See v2 of the patch attached. > I don't think those are

Re: [HACKERS] [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

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

Hello, hackers! As was mentioned in that message [1], and earlier in [2], DROPping OPERATOR with negator or commutator doesn't update respective fields (oprnegate and oprcom) on them. While this issue is probably not very frequent, this behaviour leaves database in inconsistent state and prevents