Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Remove redundant setting of tuplesort status

  1. [PATCH] Simple code cleanup in tuplesort.c.

    Xing Guo <higuoxing@gmail.com> — 2022-07-27T09:09:54Z

    Hi hackers,

The bounded heap sorting status flag is set twice in sort_bounded_heap()
and tuplesort_performsort(). This patch helps remove one of them.

Best Regards,
Xing

  2. Re: [PATCH] Simple code cleanup in tuplesort.c.

    Richard Guo <guofenglinux@gmail.com> — 2022-07-27T09:49:38Z

    On Wed, Jul 27, 2022 at 5:10 PM Xing Guo <higuoxing@gmail.com> wrote:

> The bounded heap sorting status flag is set twice in sort_bounded_heap()
> and tuplesort_performsort(). This patch helps remove one of them.
>

+1. Looks good to me.

Thanks
Richard

  3. Re: [PATCH] Simple code cleanup in tuplesort.c.

    Richard Guo <guofenglinux@gmail.com> — 2022-09-16T06:43:04Z

    On Wed, Jul 27, 2022 at 5:10 PM Xing Guo <higuoxing@gmail.com> wrote:

> The bounded heap sorting status flag is set twice in sort_bounded_heap()
> and tuplesort_performsort(). This patch helps remove one of them.
>

Revisiting this patch I think maybe it's better to remove the setting of
Tuplesort status from tuplesort_performsort() for the TSS_BOUNDED case.
Thus we keep the heap manipulation routines, make_bounded_heap and
sort_bounded_heap, consistent in that they update their status
accordingly inside the function.

Also, would you please add it to the CF to not lose track of it?

Thanks
Richard

  4. Re: [PATCH] Simple code cleanup in tuplesort.c.

    Xing Guo <higuoxing@gmail.com> — 2022-09-30T15:32:10Z

    Hi Richard,

Sorry for not responding for a long time, I missed the previous email
by accident :-)

I attached a newer patch based on your suggestions and created an
entry in cf manager.
https://commitfest.postgresql.org/40/3924/

Best Regards,
Xing Guo



On 9/16/22, Richard Guo <guofenglinux@gmail.com> wrote:
> On Wed, Jul 27, 2022 at 5:10 PM Xing Guo <higuoxing@gmail.com> wrote:
>
>> The bounded heap sorting status flag is set twice in sort_bounded_heap()
>> and tuplesort_performsort(). This patch helps remove one of them.
>>
>
> Revisiting this patch I think maybe it's better to remove the setting of
> Tuplesort status from tuplesort_performsort() for the TSS_BOUNDED case.
> Thus we keep the heap manipulation routines, make_bounded_heap and
> sort_bounded_heap, consistent in that they update their status
> accordingly inside the function.
>
> Also, would you please add it to the CF to not lose track of it?
>
> Thanks
> Richard
>


-- 
Best Regards,
Xing

  5. Re: [PATCH] Simple code cleanup in tuplesort.c.

    Cary Huang <cary.huang@highgo.ca> — 2022-11-25T21:41:14Z

    The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       not tested
Spec compliant:           not tested
Documentation:            not tested

Removing "state->status = TSS_SORTEDINMEM" should be fine as it is already set in sort_bounded_heap(state) few lines before.

Cary Huang
----------------
HighGo Software Canada
www.highgo.ca

  6. Re: [PATCH] Simple code cleanup in tuplesort.c.

    John Naylor <john.naylor@enterprisedb.com> — 2023-01-05T01:18:39Z

    On Fri, Sep 16, 2022 at 1:43 PM Richard Guo <guofenglinux@gmail.com> wrote:
>
>
> On Wed, Jul 27, 2022 at 5:10 PM Xing Guo <higuoxing@gmail.com> wrote:
>>
>> The bounded heap sorting status flag is set twice in sort_bounded_heap()
and tuplesort_performsort(). This patch helps remove one of them.
>
>
> Revisiting this patch I think maybe it's better to remove the setting of
> Tuplesort status from tuplesort_performsort() for the TSS_BOUNDED case.
> Thus we keep the heap manipulation routines, make_bounded_heap and
> sort_bounded_heap, consistent in that they update their status
> accordingly inside the function.

The label TSS_BUILDRUNS has a similar style and also the following comment,
so I will push this patch with a similar comment added unless someone wants
to make a case for doing otherwise.

* Note that mergeruns sets the correct state->status.

--
John Naylor
EDB: http://www.enterprisedb.com

  7. Re: [PATCH] Simple code cleanup in tuplesort.c.

    John Naylor <john.naylor@enterprisedb.com> — 2023-01-09T09:58:11Z

    On Thu, Jan 5, 2023 at 8:18 AM John Naylor <john.naylor@enterprisedb.com>
wrote:
>
> The label TSS_BUILDRUNS has a similar style and also the following
comment, so I will push this patch with a similar comment added unless
someone wants to make a case for doing otherwise.
>
> * Note that mergeruns sets the correct state->status.

This has been pushed, thanks. Note that both patches in this thread have
the same name. Adding a version number to the name is a good way to
distinguish them.

--
John Naylor
EDB: http://www.enterprisedb.com

  8. Re: [PATCH] Simple code cleanup in tuplesort.c.

    Xing Guo <higuoxing@gmail.com> — 2023-01-09T12:29:23Z

    On 1/9/23, John Naylor <john.naylor@enterprisedb.com> wrote:
> On Thu, Jan 5, 2023 at 8:18 AM John Naylor <john.naylor@enterprisedb.com>
> wrote:
>>
>> The label TSS_BUILDRUNS has a similar style and also the following
> comment, so I will push this patch with a similar comment added unless
> someone wants to make a case for doing otherwise.
>>
>> * Note that mergeruns sets the correct state->status.
>
> This has been pushed, thanks. Note that both patches in this thread have
> the same name. Adding a version number to the name is a good way to
> distinguish them.

Thank you John. This is my first patch, I'll keep it in mind that
adding a version number next time I sending the patch.

> --
> John Naylor
> EDB: http://www.enterprisedb.com
>


-- 
Best Regards,
Xing

  9. Re: [PATCH] Simple code cleanup in tuplesort.c.

    John Naylor <john.naylor@enterprisedb.com> — 2023-01-11T03:45:55Z

    On Mon, Jan 9, 2023 at 7:29 PM Xing Guo <higuoxing@gmail.com> wrote:
>
> Thank you John. This is my first patch, I'll keep it in mind that
> adding a version number next time I sending the patch.

Welcome to the community! You may also consider reviewing a patch from the
current commitfest, since we can always use additional help there.

--
John Naylor
EDB: http://www.enterprisedb.com