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.
From 6ec99d712d7fda3cc18a8700318158f54bae4c55 Mon Sep 17 00:00:00 2001
From: Evdokimov Ilia <ilya.evdoki...@tantorlabs.com>
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
