From: | Zhihong Yu <zyu(at)yugabyte(dot)com> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Daniel Gustafsson <daniel(at)yesql(dot)se>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: A qsort template |
Date: | 2021-06-16 22:05:24 |
Message-ID: | CALNJ-vRzf+TfoY_sKsPCb4+3Z4EcdRmZnM69CJkFEV7fdo8+Dg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Jun 16, 2021 at 2:54 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> Hi Zhihong,
>
> On Thu, Jun 17, 2021 at 8:13 AM Zhihong Yu <zyu(at)yugabyte(dot)com> wrote:
> > In 0001-Add-bsearch-and-unique-templates-to-sort_template.h.patch :
> >
> > - const ST_ELEMENT_TYPE *
> ST_SORT_PROTO_ARG);
> > + const ST_ELEMENT_TYPE
> *ST_SORT_PROTO_ARG);
> >
> > It seems there is no real change in the line above. Better keep the
> original formation.
>
> Hmm, well it was only recently damaged by commit def5b065, and that's
> because I'd forgotten to put ST_ELEMENT_TYPE into typedefs.list, and I
> was correcting that in this patch. (That file is used by
> pg_bsd_indent to decide if an identifier is a type or a variable,
> which affects whether '*' is formatted like a unary operator/type
> syntax or a binary operator.)
>
> > * - ST_COMPARE_ARG_TYPE - type of extra argument
> > *
> > + * To say that the comparator returns a type other than int, use:
> > + *
> > + * - ST_COMPARE_TYPE - an integer type
> >
> > Since the ST_COMPARE_TYPE is meant to designate the type of the return
> value, maybe ST_COMPARE_RET_TYPE would be better name.
> > It also goes with ST_COMPARE_ARG_TYPE preceding this.
>
> Good idea, will do.
>
> > - ST_POINTER_TYPE *a = (ST_POINTER_TYPE *) data,
> > - *pa,
> > - *pb,
> > - *pc,
> > - *pd,
> > - *pl,
> > - *pm,
> > - *pn;
> > + ST_POINTER_TYPE *a = (ST_POINTER_TYPE *) data;
> > + ST_POINTER_TYPE *pa;
> >
> > There doesn't seem to be material change for the above hunk.
>
> In master, you can't write #define ST_ELEMENT_TYPE some_type *, which
> seems like it would be quite useful. You can use pointers as element
> types, but only with a typedef name due to C parsing rules. some_type
> **a, *pa, ... declares some_type *pa, but we want some_type **pa. I
> don't want to have to introduce extra typedefs. The change fixes that
> problem by not using C's squirrelly variable declaration list syntax.
>
> > + while (left <= right)
> > + {
> > + size_t mid = (left + right) / 2;
> >
> > The computation for midpoint should be left + (right-left)/2.
>
> Right, my way can overflow. Will fix. Thanks!
>
Hi,
Thanks for giving me background on typedefs.
The relevant changes look fine to me.
Cheers
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2021-06-16 22:13:16 | Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints |
Previous Message | John Naylor | 2021-06-16 22:02:32 | Re: a path towards replacing GEQO with something better |