On Fri, Sep 8, 2023 at 2:42 AM Robert Haas wrote:
> Committed.
Thanks for pushing it!
Thanks
Richard
On Thu, Sep 7, 2023 at 5:32 AM Aleksander Alekseev
wrote:
> v2 LGTM.
Committed.
--
Robert Haas
EDB: http://www.enterprisedb.com
Hi,
>> I agree. I don't think the patch submitter is obliged to try to write
>> a good commit message, but people who contribute regularly or are
>> posting large stacks of complex patches are probably well-advised to
>> try. It makes life easier for committers and even for reviewers trying
>> to
On Wed, Sep 6, 2023 at 8:50 PM Robert Haas wrote:
> On Wed, Sep 6, 2023 at 2:45 AM Andy Fan wrote:
> > In my opinion, we can do some stuff to improve the ROI.
> > - Authors should do as much as possible, mainly a better commit
> > message. As for this patch, the commit message is " Adjustmen
On Wed, Sep 6, 2023 at 8:50 PM Robert Haas wrote:
> On Wed, Sep 6, 2023 at 2:45 AM Andy Fan wrote:
> > I guess the *valuable* sometimes means the effort we pay is greater
> > than the benefit we get, As for this patch, the benefit is not huge (it
> > is possible the compiler already does that)
On Wed, Sep 6, 2023 at 2:45 AM Andy Fan wrote:
> I guess the *valuable* sometimes means the effort we pay is greater
> than the benefit we get, As for this patch, the benefit is not huge (it
> is possible the compiler already does that). and the most effort we pay
> should be committer's attenti
>
> I guess the *valuable* sometimes means the effort we pay is greater
> than the benefit we get, As for this patch, the benefit is not huge (it
> is possible the compiler already does that). and the most effort we pay
> should be committer's attention, who needs to grab the patch, write the
> c
On Wed, Sep 6, 2023 at 2:45 PM Andy Fan
wrote:uld have a different conversation...
>
> I like this consultation, so +1 from me :)
>
s/consultation/conclusion.
--
Best Regards
Andy Fan
On Wed, Sep 6, 2023 at 4:06 AM Robert Haas wrote:
> On Tue, Sep 5, 2023 at 12:05 PM Aleksander Alekseev
> wrote:
> > Now when we continue reviewing the patch, could you please elaborate a
> > bit on why you think it's worth committing?
>
> Well, why not? The test he's proposing to move earlier d
On Tue, Sep 5, 2023 at 12:05 PM Aleksander Alekseev
wrote:
> Now when we continue reviewing the patch, could you please elaborate a
> bit on why you think it's worth committing?
Well, why not? The test he's proposing to move earlier doesn't involve
calling a function, so it should be cheaper than
Hi,
> > I think this patch is a good idea and should be committed.
>
> No problem, changing status back to "Needs review".
Now when we continue reviewing the patch, could you please elaborate a
bit on why you think it's worth committing?
--
Best regards,
Aleksander Alekseev
Hi Robert,
> I don't think that's the correct conclusion. You said here that you
> didn't think the patch was valuable. Then you said the same thing over
> there. You agreeing with yourself is not a consensus.
The word "consensus" was a poor choice for sure. The fact that I
suggested to reject th
On Mon, Sep 4, 2023 at 10:17 AM Aleksander Alekseev
wrote:
> During the triage of the patches submitted for the September CF a
> consensus was reached [1] to mark this patch as Rejected.
I don't think that's the correct conclusion. You said here that you
didn't think the patch was valuable. Then
Hi,
>> I see the reasoning behind the proposed change, but I'm not convinced
>> that there will be any measurable performance improvements. Firstly,
>> compare_path_costs() is rather cheap. Secondly, require_parallel_safe
>> is `false` in most of the cases. Last but not least, one should prove
>>
On Tue, Jul 11, 2023 at 8:16 PM Aleksander Alekseev <
aleksan...@timescale.com> wrote:
> Hi,
>
> > The check for parallel_safe should be even cheaper than cost comparison
> > so I think it's better to do that first. The attached patch does this
> > and also updates the comment to mention the requ
Hi,
> The check for parallel_safe should be even cheaper than cost comparison
> so I think it's better to do that first. The attached patch does this
> and also updates the comment to mention the requirement about being
> parallel-safe.
The patch was marked as "Needs review" so I decided to take
16 matches
Mail list logo
