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 it unconditionally does a CommandCounterIncrement
after ea
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 it performs. IMO that would be a lot simpl
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 deletion will fail outright.
> So what? We've
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 convoluted. I rewrote it - I believe
>> more
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. How does this look?
I
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 unnecessarily convoluted. I rewrot
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. Perh
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 optimi
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 such a small use case.
I
> 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 is automatically gener
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->o
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:
http://www.postgresql.org/mailpref/pgsql-hackers
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
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 mandato
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
15 matches
Mail list logo
