duplicate function declaration in multirangetypes_selfuncs.c

Started by Anton Voloshinover 2 years ago11 messages
#1Anton Voloshin
a.voloshin@postgrespro.ru

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

#2Daniel Gustafsson
daniel@yesql.se
In reply to: Anton Voloshin (#1)
Re: duplicate function declaration in multirangetypes_selfuncs.c

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

#3Pavel Borisov
pashkin.elfe@gmail.com
In reply to: Daniel Gustafsson (#2)
1 attachment(s)
Re: duplicate function declaration in multirangetypes_selfuncs.c

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

Attachments:

0001-Remove-double-declaration-of-default_multirange_sele.patchapplication/octet-stream; name=0001-Remove-double-declaration-of-default_multirange_sele.patchDownload
From e61e0f9851cbc2ab25e77654dbca3f562ddb4925 Mon Sep 17 00:00:00 2001
From: Pavel Borisov <pashkin.elfe@gmail.com>
Date: Fri, 21 Apr 2023 14:42:12 +0400
Subject: [PATCH] Remove double declaration of default_multirange_selectivity()

---
 src/backend/utils/adt/multirangetypes_selfuncs.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/backend/utils/adt/multirangetypes_selfuncs.c b/src/backend/utils/adt/multirangetypes_selfuncs.c
index e9326f8342a..cefc4710fd4 100644
--- a/src/backend/utils/adt/multirangetypes_selfuncs.c
+++ b/src/backend/utils/adt/multirangetypes_selfuncs.c
@@ -35,7 +35,6 @@ static double calc_multirangesel(TypeCacheEntry *typcache,
 								 VariableStatData *vardata,
 								 const MultirangeType *constval, Oid operator);
 static double default_multirange_selectivity(Oid operator);
-static double default_multirange_selectivity(Oid operator);
 static double calc_hist_selectivity(TypeCacheEntry *typcache,
 									VariableStatData *vardata,
 									const MultirangeType *constval,
-- 
2.37.1 (Apple Git-137.1)

#4Anton Voloshin
a.voloshin@postgrespro.ru
In reply to: Pavel Borisov (#3)
1 attachment(s)
Re: duplicate function declaration in multirangetypes_selfuncs.c

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

Attachments:

0002-Remove-duplicate-break-in-find_struct_member-s-switc.patchtext/x-patch; charset=UTF-8; name=0002-Remove-duplicate-break-in-find_struct_member-s-switc.patchDownload
From 05c9979de0596f9bbe89304f54e20b759205b57b Mon Sep 17 00:00:00 2001
From: Anton Voloshin <a.voloshin@postgrespro.ru>
Date: Fri, 21 Apr 2023 13:55:05 +0300
Subject: [PATCH] Remove duplicate break in find_struct_member's switch

---
 src/backend/utils/adt/multirangetypes_selfuncs.c | 1 -
 src/interfaces/ecpg/preproc/variable.c           | 1 -
 2 files changed, 2 deletions(-)

diff --git a/src/backend/utils/adt/multirangetypes_selfuncs.c b/src/backend/utils/adt/multirangetypes_selfuncs.c
index e9326f8342a..cefc4710fd4 100644
--- a/src/backend/utils/adt/multirangetypes_selfuncs.c
+++ b/src/backend/utils/adt/multirangetypes_selfuncs.c
@@ -35,7 +35,6 @@ static double calc_multirangesel(TypeCacheEntry *typcache,
 								 VariableStatData *vardata,
 								 const MultirangeType *constval, Oid operator);
 static double default_multirange_selectivity(Oid operator);
-static double default_multirange_selectivity(Oid operator);
 static double calc_hist_selectivity(TypeCacheEntry *typcache,
 									VariableStatData *vardata,
 									const MultirangeType *constval,
diff --git a/src/interfaces/ecpg/preproc/variable.c b/src/interfaces/ecpg/preproc/variable.c
index 2a2b9531187..b23ed5edf46 100644
--- a/src/interfaces/ecpg/preproc/variable.c
+++ b/src/interfaces/ecpg/preproc/variable.c
@@ -105,7 +105,6 @@ find_struct_member(char *name, char *str, struct ECPGstruct_member *members, int
 						else
 							return find_struct_member(name, ++end, members->type->u.members, brace_level);
 						break;
-						break;
 					case '.':
 						if (members->type->type == ECPGt_array)
 							return find_struct_member(name, end, members->type->u.element->u.members, brace_level);
-- 
2.40.0

#5Daniel Gustafsson
daniel@yesql.se
In reply to: Anton Voloshin (#4)
Re: duplicate function declaration in multirangetypes_selfuncs.c

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

#6Pavel Borisov
pashkin.elfe@gmail.com
In reply to: Daniel Gustafsson (#5)
2 attachment(s)
Re: duplicate function declaration in multirangetypes_selfuncs.c

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.

Attachments:

v2-0002-Remove-duplicate-break-in-find_struct_member-s-sw.patchapplication/octet-stream; name=v2-0002-Remove-duplicate-break-in-find_struct_member-s-sw.patchDownload
From 7ef831fcc6d55190473b979c40cad4d8e512b0fd Mon Sep 17 00:00:00 2001
From: Pavel Borisov <pashkin.elfe@gmail.com>
Date: Fri, 21 Apr 2023 15:15:36 +0400
Subject: [PATCH v2 2/2] Remove duplicate break in find_struct_member's switch

---
 src/interfaces/ecpg/preproc/variable.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/interfaces/ecpg/preproc/variable.c b/src/interfaces/ecpg/preproc/variable.c
index 2a2b9531187..b23ed5edf46 100644
--- a/src/interfaces/ecpg/preproc/variable.c
+++ b/src/interfaces/ecpg/preproc/variable.c
@@ -105,7 +105,6 @@ find_struct_member(char *name, char *str, struct ECPGstruct_member *members, int
 						else
 							return find_struct_member(name, ++end, members->type->u.members, brace_level);
 						break;
-						break;
 					case '.':
 						if (members->type->type == ECPGt_array)
 							return find_struct_member(name, end, members->type->u.element->u.members, brace_level);
-- 
2.37.1 (Apple Git-137.1)

v2-0001-Remove-double-declaration-of-default_multirange_s.patchapplication/octet-stream; name=v2-0001-Remove-double-declaration-of-default_multirange_s.patchDownload
From e61e0f9851cbc2ab25e77654dbca3f562ddb4925 Mon Sep 17 00:00:00 2001
From: Pavel Borisov <pashkin.elfe@gmail.com>
Date: Fri, 21 Apr 2023 14:42:12 +0400
Subject: [PATCH v2 1/2] Remove double declaration of
 default_multirange_selectivity()

---
 src/backend/utils/adt/multirangetypes_selfuncs.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/backend/utils/adt/multirangetypes_selfuncs.c b/src/backend/utils/adt/multirangetypes_selfuncs.c
index e9326f8342a..cefc4710fd4 100644
--- a/src/backend/utils/adt/multirangetypes_selfuncs.c
+++ b/src/backend/utils/adt/multirangetypes_selfuncs.c
@@ -35,7 +35,6 @@ static double calc_multirangesel(TypeCacheEntry *typcache,
 								 VariableStatData *vardata,
 								 const MultirangeType *constval, Oid operator);
 static double default_multirange_selectivity(Oid operator);
-static double default_multirange_selectivity(Oid operator);
 static double calc_hist_selectivity(TypeCacheEntry *typcache,
 									VariableStatData *vardata,
 									const MultirangeType *constval,
-- 
2.37.1 (Apple Git-137.1)

#7Anton Voloshin
a.voloshin@postgrespro.ru
In reply to: Daniel Gustafsson (#5)
Re: duplicate function declaration in multirangetypes_selfuncs.c

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

#8Alexander Korotkov
aekorotkov@gmail.com
In reply to: Pavel Borisov (#6)
Re: duplicate function declaration in multirangetypes_selfuncs.c

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

#9Daniel Gustafsson
daniel@yesql.se
In reply to: Alexander Korotkov (#8)
Re: duplicate function declaration in multirangetypes_selfuncs.c

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

#10Alexander Korotkov
aekorotkov@gmail.com
In reply to: Daniel Gustafsson (#9)
Re: duplicate function declaration in multirangetypes_selfuncs.c

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

#11Daniel Gustafsson
daniel@yesql.se
In reply to: Alexander Korotkov (#10)
Re: duplicate function declaration in multirangetypes_selfuncs.c

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