public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ilia Evdokimov <[email protected]>
To: Richard Guo <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Remove vardata parameters from eqjoinsel_inner
Date: Thu, 27 Mar 2025 14:00:30 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAMbWs49ybFS2UPhm4veuTPJw8ZTsrjjXYQgE9Pi5neC2XvOrbg@mail.gmail.com>
References: <[email protected]>
<CAMbWs49ybFS2UPhm4veuTPJw8ZTsrjjXYQgE9Pi5neC2XvOrbg@mail.gmail.com>
On 27.03.2025 10:48, Richard Guo wrote:
> I'm wondering whether we should also remove parameter vardata1 from
> eqjoinsel_semi. vardata2 is still needed though to clamp nd2 to be
> not more than the rel's row estimate.
>
> Thanks
> Richard
Indeed, the parameter vardata1 in eqjoinsel_semi() is currently unused
and could logically be removed. However, simply leaving a single
parameter named vardata2 would appear strange and unintuitive, as it
implicitly suggests the existence of a corresponding "first" parameter.
I suggest renaming vardata2 to something more descriptive, such as
rhs_vardata, clearly indicating its role related specifically to the
right side of the join condition.
I attached v2 patch with changes.
Any thoughts?
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.
Attachments:
[text/x-patch] v2-0001-Remove-unused-vardata-parameters-in-eqjoinsel.patch (3.3K, ../[email protected]/3-v2-0001-Remove-unused-vardata-parameters-in-eqjoinsel.patch)
download | inline diff:
From 6ec99d712d7fda3cc18a8700318158f54bae4c55 Mon Sep 17 00:00:00 2001
From: Evdokimov Ilia <[email protected]>
Date: Thu, 27 Mar 2025 13:59:02 +0300
Subject: [PATCH v2] Remove unused vardata parameters in eqjoinsel_inner and
eqjoinsel_semi
---
src/backend/utils/adt/selfuncs.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 5b35debc8ff..6d428d65d64 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -150,14 +150,13 @@ get_index_stats_hook_type get_index_stats_hook = NULL;
static double eqsel_internal(PG_FUNCTION_ARGS, bool negate);
static double eqjoinsel_inner(Oid opfuncoid, Oid collation,
- VariableStatData *vardata1, VariableStatData *vardata2,
double nd1, double nd2,
bool isdefault1, bool isdefault2,
AttStatsSlot *sslot1, AttStatsSlot *sslot2,
Form_pg_statistic stats1, Form_pg_statistic stats2,
bool have_mcvs1, bool have_mcvs2);
static double eqjoinsel_semi(Oid opfuncoid, Oid collation,
- VariableStatData *vardata1, VariableStatData *vardata2,
+ VariableStatData *rhs_vardata,
double nd1, double nd2,
bool isdefault1, bool isdefault2,
AttStatsSlot *sslot1, AttStatsSlot *sslot2,
@@ -2348,7 +2347,6 @@ eqjoinsel(PG_FUNCTION_ARGS)
/* We need to compute the inner-join selectivity in all cases */
selec_inner = eqjoinsel_inner(opfuncoid, collation,
- &vardata1, &vardata2,
nd1, nd2,
isdefault1, isdefault2,
&sslot1, &sslot2,
@@ -2375,7 +2373,7 @@ eqjoinsel(PG_FUNCTION_ARGS)
if (!join_is_reversed)
selec = eqjoinsel_semi(opfuncoid, collation,
- &vardata1, &vardata2,
+ &vardata2,
nd1, nd2,
isdefault1, isdefault2,
&sslot1, &sslot2,
@@ -2388,7 +2386,7 @@ eqjoinsel(PG_FUNCTION_ARGS)
Oid commopfuncoid = OidIsValid(commop) ? get_opcode(commop) : InvalidOid;
selec = eqjoinsel_semi(commopfuncoid, collation,
- &vardata2, &vardata1,
+ &vardata1,
nd2, nd1,
isdefault2, isdefault1,
&sslot2, &sslot1,
@@ -2436,7 +2434,6 @@ eqjoinsel(PG_FUNCTION_ARGS)
*/
static double
eqjoinsel_inner(Oid opfuncoid, Oid collation,
- VariableStatData *vardata1, VariableStatData *vardata2,
double nd1, double nd2,
bool isdefault1, bool isdefault2,
AttStatsSlot *sslot1, AttStatsSlot *sslot2,
@@ -2633,7 +2630,7 @@ eqjoinsel_inner(Oid opfuncoid, Oid collation,
*/
static double
eqjoinsel_semi(Oid opfuncoid, Oid collation,
- VariableStatData *vardata1, VariableStatData *vardata2,
+ VariableStatData *rhs_vardata,
double nd1, double nd2,
bool isdefault1, bool isdefault2,
AttStatsSlot *sslot1, AttStatsSlot *sslot2,
@@ -2662,11 +2659,11 @@ eqjoinsel_semi(Oid opfuncoid, Oid collation,
* great, maybe, but it didn't come out of nowhere either. This is most
* helpful when the inner relation is empty and consequently has no stats.
*/
- if (vardata2->rel)
+ if (rhs_vardata->rel)
{
- if (nd2 >= vardata2->rel->rows)
+ if (nd2 >= rhs_vardata->rel->rows)
{
- nd2 = vardata2->rel->rows;
+ nd2 = rhs_vardata->rel->rows;
isdefault2 = false;
}
}
--
2.34.1
view thread (5+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Remove vardata parameters from eqjoinsel_inner
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox