Horton

Thread

Commits

  1. Remove duplicate lines of code

  1. duplicate function declaration in multirangetypes_selfuncs.c

    Anton Voloshin <a.voloshin@postgrespro.ru> — 2023-04-21T10:32:01Z

    Hello, hackers,

we have a duplicate line, declaration of default_multirange_selectivity() in
src/backend/utils/adt/multirangetypes_selfuncs.c:

static double default_multirange_selectivity(Oid operator);
static double default_multirange_selectivity(Oid operator);

Affected branches: REL_14_STABLE and above.

Both lines come from the same commit:
 > commit 6df7a9698bb036610c1e8c6d375e1be38cb26d5f
 > Author: Alexander Korotkov <akorotkov@postgresql.org>
 > Date:   Sun Dec 20 07:20:33 2020
 >
 >     Multirange datatypes

No harm from this duplication, still, I suggest to clean it up for 
tidiness' sake.

-- 
Anton Voloshin
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru

  2. Re: duplicate function declaration in multirangetypes_selfuncs.c

    Daniel Gustafsson <daniel@yesql.se> — 2023-04-21T10:34:11Z

    > On 21 Apr 2023, at 12:32, Anton Voloshin <a.voloshin@postgrespro.ru> wrote:

> we have a duplicate line, declaration of default_multirange_selectivity() in
> src/backend/utils/adt/multirangetypes_selfuncs.c:
> 
> static double default_multirange_selectivity(Oid operator);
> static double default_multirange_selectivity(Oid operator);

Nice catch.

> No harm from this duplication, still, I suggest to clean it up for tidiness' sake.

+1

--
Daniel Gustafsson

  3. Re: duplicate function declaration in multirangetypes_selfuncs.c

    Pavel Borisov <pashkin.elfe@gmail.com> — 2023-04-21T10:45:16Z

    Hi!

On Fri, 21 Apr 2023 at 14:34, Daniel Gustafsson <daniel@yesql.se> wrote:
>
> > On 21 Apr 2023, at 12:32, Anton Voloshin <a.voloshin@postgrespro.ru> wrote:
>
> > we have a duplicate line, declaration of default_multirange_selectivity() in
> > src/backend/utils/adt/multirangetypes_selfuncs.c:
> >
> > static double default_multirange_selectivity(Oid operator);
> > static double default_multirange_selectivity(Oid operator);
>
> Nice catch.
>
> > No harm from this duplication, still, I suggest to clean it up for tidiness' sake.
>
> +1
>
The patch is attached. Anyone to commit?

Pavel Borisov
Supabase

  4. Re: duplicate function declaration in multirangetypes_selfuncs.c

    Anton Voloshin <a.voloshin@postgrespro.ru> — 2023-04-21T10:58:34Z

    On 21/04/2023 13:45, Pavel Borisov wrote:
> The patch is attached. Anyone to commit?

Speaking of duplicates, I just found another one:
 >                        break;
 >                        break;
in src/interfaces/ecpg/preproc/variable.c
(in all stable branches).

Sorry for missing it in the previous letter.

Additional patch attached. Or both could go in the same commit, it's up 
to committer.

-- 
Anton Voloshin
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru

  5. Re: duplicate function declaration in multirangetypes_selfuncs.c

    Daniel Gustafsson <daniel@yesql.se> — 2023-04-21T11:14:45Z

    > On 21 Apr 2023, at 12:58, Anton Voloshin <a.voloshin@postgrespro.ru> wrote:
> 
> On 21/04/2023 13:45, Pavel Borisov wrote:
>> The patch is attached. Anyone to commit?
> 
> Speaking of duplicates, I just found another one:
> >                        break;
> >                        break;
> in src/interfaces/ecpg/preproc/variable.c
> (in all stable branches).

Indeed, coming in via 086cf1458 it's over a decade old.

> Additional patch attached. Or both could go in the same commit, it's up to committer.

I'll take care of these in a bit (unless someone finds more, or objects)
backpatching them to their respective origins branches.

--
Daniel Gustafsson

  6. Re: duplicate function declaration in multirangetypes_selfuncs.c

    Pavel Borisov <pashkin.elfe@gmail.com> — 2023-04-21T11:21:24Z

    Hi!

On Fri, 21 Apr 2023 at 15:14, Daniel Gustafsson <daniel@yesql.se> wrote:
>
> > On 21 Apr 2023, at 12:58, Anton Voloshin <a.voloshin@postgrespro.ru> wrote:
> >
> > On 21/04/2023 13:45, Pavel Borisov wrote:
> >> The patch is attached. Anyone to commit?
> >
> > Speaking of duplicates, I just found another one:
> > >                        break;
> > >                        break;
> > in src/interfaces/ecpg/preproc/variable.c
> > (in all stable branches).
>
> Indeed, coming in via 086cf1458 it's over a decade old.
>
> > Additional patch attached. Or both could go in the same commit, it's up to committer.
>
> I'll take care of these in a bit (unless someone finds more, or objects)
> backpatching them to their respective origins branches.
>
> --
> Daniel Gustafsson
Technically patches 0001 and 0002 in the thread above don't form
patchset i.e. 0002 will not apply over 0001. Fixed this in v2.
(They could be merged into one but as they fix completely unrelated
things, I think a better way to commit them separately.)

Pavel.

  7. Re: duplicate function declaration in multirangetypes_selfuncs.c

    Anton Voloshin <a.voloshin@postgrespro.ru> — 2023-04-21T12:20:54Z

    On 21/04/2023 14:14, Daniel Gustafsson wrote:
> I'll take care of these in a bit (unless someone finds more, or objects)
> backpatching them to their respective origins branches

Thanks!

I went through master with
find . -name "*.[ch]" -exec bash -c 'echo {}; uniq -d {}' \;|sed -E 
'/^[[:space:]*]*$/d;'

and could not find any other obvious unintentional duplicates, except 
the two mentioned already. There seems to be some strange duplicates in 
snowball, but that's external and generated code and I could not figure 
out quickly whether those are intentional or not. Hopefully, they are 
harmless or intentional.

All other duplicated lines I've analyzed seem to be intentional.

Granted, I've mostly ignored lines without ';', also I could have missed 
something, but currently I'm not aware of any other unintentionally 
duplicated lines.

-- 
Anton Voloshin
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru

  8. Re: duplicate function declaration in multirangetypes_selfuncs.c

    Alexander Korotkov <aekorotkov@gmail.com> — 2023-04-23T11:58:42Z

    On Fri, Apr 21, 2023 at 2:21 PM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
> On Fri, 21 Apr 2023 at 15:14, Daniel Gustafsson <daniel@yesql.se> wrote:
> >
> > > On 21 Apr 2023, at 12:58, Anton Voloshin <a.voloshin@postgrespro.ru> wrote:
> > >
> > > On 21/04/2023 13:45, Pavel Borisov wrote:
> > >> The patch is attached. Anyone to commit?
> > >
> > > Speaking of duplicates, I just found another one:
> > > >                        break;
> > > >                        break;
> > > in src/interfaces/ecpg/preproc/variable.c
> > > (in all stable branches).
> >
> > Indeed, coming in via 086cf1458 it's over a decade old.
> >
> > > Additional patch attached. Or both could go in the same commit, it's up to committer.
> >
> > I'll take care of these in a bit (unless someone finds more, or objects)
> > backpatching them to their respective origins branches.
> >
> > --
> > Daniel Gustafsson
> Technically patches 0001 and 0002 in the thread above don't form
> patchset i.e. 0002 will not apply over 0001. Fixed this in v2.
> (They could be merged into one but as they fix completely unrelated
> things, I think a better way to commit them separately.)

I wonder if we should backpatch this.  On the one hand, this is not
critical, and we may skip backpatching.  On the other hand,
backpatching will evade unnecessary code differences between major
versions and potentially simplify further backpatching.

I would prefer backpathing.  Other opinions?

------
Regards,
Alexander Korotkov

  9. Re: duplicate function declaration in multirangetypes_selfuncs.c

    Daniel Gustafsson <daniel@yesql.se> — 2023-04-23T12:35:53Z

    > On 23 Apr 2023, at 13:59, Alexander Korotkov <aekorotkov@gmail.com> wrote:
> 
> On Fri, Apr 21, 2023 at 2:21 PM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
>>> On Fri, 21 Apr 2023 at 15:14, Daniel Gustafsson <daniel@yesql.se> wrote:
>>> 
>>>> On 21 Apr 2023, at 12:58, Anton Voloshin <a.voloshin@postgrespro.ru> wrote:
>>>> 
>>>> On 21/04/2023 13:45, Pavel Borisov wrote:
>>>>> The patch is attached. Anyone to commit?
>>>> 
>>>> Speaking of duplicates, I just found another one:
>>>>>                       break;
>>>>>                       break;
>>>> in src/interfaces/ecpg/preproc/variable.c
>>>> (in all stable branches).
>>> 
>>> Indeed, coming in via 086cf1458 it's over a decade old.
>>> 
>>>> Additional patch attached. Or both could go in the same commit, it's up to committer.
>>> 
>>> I'll take care of these in a bit (unless someone finds more, or objects)
>>> backpatching them to their respective origins branches.
>>> 
>>> --
>>> Daniel Gustafsson
>> Technically patches 0001 and 0002 in the thread above don't form
>> patchset i.e. 0002 will not apply over 0001. Fixed this in v2.
>> (They could be merged into one but as they fix completely unrelated
>> things, I think a better way to commit them separately.)
> 
> I wonder if we should backpatch this.  On the one hand, this is not
> critical, and we may skip backpatching.  On the other hand,
> backpatching will evade unnecessary code differences between major
> versions and potentially simplify further backpatching.
> 
> I would prefer backpathing.  Other opinions?

I had planned to backpatch these two fixes for just that reason, to avoid the risk for other backpatches not applying. 

./daniel

  10. Re: duplicate function declaration in multirangetypes_selfuncs.c

    Alexander Korotkov <aekorotkov@gmail.com> — 2023-04-24T01:35:21Z

    On Sun, Apr 23, 2023 at 3:36 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> > On 23 Apr 2023, at 13:59, Alexander Korotkov <aekorotkov@gmail.com> wrote:
> >
> > On Fri, Apr 21, 2023 at 2:21 PM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
> >>> On Fri, 21 Apr 2023 at 15:14, Daniel Gustafsson <daniel@yesql.se> wrote:
> >>>
> >>>> On 21 Apr 2023, at 12:58, Anton Voloshin <a.voloshin@postgrespro.ru> wrote:
> >>>>
> >>>> On 21/04/2023 13:45, Pavel Borisov wrote:
> >>>>> The patch is attached. Anyone to commit?
> >>>>
> >>>> Speaking of duplicates, I just found another one:
> >>>>>                       break;
> >>>>>                       break;
> >>>> in src/interfaces/ecpg/preproc/variable.c
> >>>> (in all stable branches).
> >>>
> >>> Indeed, coming in via 086cf1458 it's over a decade old.
> >>>
> >>>> Additional patch attached. Or both could go in the same commit, it's up to committer.
> >>>
> >>> I'll take care of these in a bit (unless someone finds more, or objects)
> >>> backpatching them to their respective origins branches.
> >>>
> >>> --
> >>> Daniel Gustafsson
> >> Technically patches 0001 and 0002 in the thread above don't form
> >> patchset i.e. 0002 will not apply over 0001. Fixed this in v2.
> >> (They could be merged into one but as they fix completely unrelated
> >> things, I think a better way to commit them separately.)
> >
> > I wonder if we should backpatch this.  On the one hand, this is not
> > critical, and we may skip backpatching.  On the other hand,
> > backpatching will evade unnecessary code differences between major
> > versions and potentially simplify further backpatching.
> >
> > I would prefer backpathing.  Other opinions?
>
> I had planned to backpatch these two fixes for just that reason, to avoid the risk for other backpatches not applying.

OK. I'm good with this plan.

------
Regards,
Alexander Korotkov

  11. Re: duplicate function declaration in multirangetypes_selfuncs.c

    Daniel Gustafsson <daniel@yesql.se> — 2023-04-24T09:43:18Z

    > On 24 Apr 2023, at 03:35, Alexander Korotkov <aekorotkov@gmail.com> wrote:
> On Sun, Apr 23, 2023 at 3:36 PM Daniel Gustafsson <daniel@yesql.se> wrote:

>> I had planned to backpatch these two fixes for just that reason, to avoid the risk for other backpatches not applying.
> 
> OK. I'm good with this plan.

Done.

--
Daniel Gustafsson