Обсуждение: duplicate function declaration in multirangetypes_selfuncs.c
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
> 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
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
Вложения
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
Вложения
> 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
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.
Вложения
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
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
> 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
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
> 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