From f9b45278ee55e03d363b45d8bce518e7695fb5f4 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 13 Nov 2025 12:59:21 -0500 Subject: [PATCH v5 4/5] Share more work between eqjoinsel_inner and eqjoinsel_semi. Originally, only one of eqjoinsel_inner and eqjoinsel_semi was invoked per eqjoinsel call, so the fact that they duplicated a good deal of work was irrelevant to performance. But since commit a314c3407, the semi/antijoin case calls both, and that is really expensive if there are a lot of MCVs to match. Refactor so that we can re-use eqjoinsel_inner's matching results except in the (uncommon) case where eqjoinsel_semi clamps the RHS MCV list size because it's less than the expected number of rows to be fetched from the RHS rel. This doesn't seem to create any performance penalty for non-semijoin cases. While at it, we can avoid doing fmgr_info twice too. I considered also avoiding duplicate InitFunctionCallInfoData calls, but desisted: that wouldn't save very much, and in my tests it looks like there may be some performance advantage if fcinfo is a local variable. --- src/backend/utils/adt/selfuncs.c | 115 +++++++++++++++++++------------ 1 file changed, 72 insertions(+), 43 deletions(-) diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index 53653d2d05b..b4ed12a3737 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -148,14 +148,15 @@ get_relation_stats_hook_type get_relation_stats_hook = NULL; 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, +static double eqjoinsel_inner(FmgrInfo *eqproc, 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, + bool have_mcvs1, bool have_mcvs2, + bool *hasmatch1, bool *hasmatch2); +static double eqjoinsel_semi(FmgrInfo *eqproc, Oid collation, bool op_is_reversed, VariableStatData *vardata1, VariableStatData *vardata2, double nd1, double nd2, @@ -163,8 +164,9 @@ static double eqjoinsel_semi(Oid opfuncoid, Oid collation, AttStatsSlot *sslot1, AttStatsSlot *sslot2, Form_pg_statistic stats1, Form_pg_statistic stats2, bool have_mcvs1, bool have_mcvs2, + bool *hasmatch1, bool *hasmatch2, RelOptInfo *inner_rel); -static void eqjoinsel_find_matches(Oid opfuncoid, Oid collation, +static void eqjoinsel_find_matches(FmgrInfo *eqproc, Oid collation, bool op_is_reversed, AttStatsSlot *sslot1, AttStatsSlot *sslot2, int nvalues1, int nvalues2, @@ -2311,12 +2313,15 @@ eqjoinsel(PG_FUNCTION_ARGS) bool isdefault1; bool isdefault2; Oid opfuncoid; + FmgrInfo eqproc; AttStatsSlot sslot1; AttStatsSlot sslot2; Form_pg_statistic stats1 = NULL; Form_pg_statistic stats2 = NULL; bool have_mcvs1 = false; bool have_mcvs2 = false; + bool *hasmatch1 = NULL; + bool *hasmatch2 = NULL; bool get_mcv_stats; bool join_is_reversed; RelOptInfo *inner_rel; @@ -2367,14 +2372,25 @@ eqjoinsel(PG_FUNCTION_ARGS) ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS); } + /* Prepare info usable by both eqjoinsel_inner and eqjoinsel_semi */ + if (have_mcvs1 && have_mcvs2) + { + fmgr_info(opfuncoid, &eqproc); + hasmatch1 = (bool *) palloc0(sslot1.nvalues * sizeof(bool)); + hasmatch2 = (bool *) palloc0(sslot2.nvalues * sizeof(bool)); + } + else + memset(&eqproc, 0, sizeof(eqproc)); /* silence uninit-var warnings */ + /* We need to compute the inner-join selectivity in all cases */ - selec_inner = eqjoinsel_inner(opfuncoid, collation, + selec_inner = eqjoinsel_inner(&eqproc, collation, &vardata1, &vardata2, nd1, nd2, isdefault1, isdefault2, &sslot1, &sslot2, stats1, stats2, - have_mcvs1, have_mcvs2); + have_mcvs1, have_mcvs2, + hasmatch1, hasmatch2); switch (sjinfo->jointype) { @@ -2395,7 +2411,7 @@ eqjoinsel(PG_FUNCTION_ARGS) inner_rel = find_join_input_rel(root, sjinfo->min_righthand); if (!join_is_reversed) - selec = eqjoinsel_semi(opfuncoid, collation, + selec = eqjoinsel_semi(&eqproc, collation, false, &vardata1, &vardata2, nd1, nd2, @@ -2403,9 +2419,10 @@ eqjoinsel(PG_FUNCTION_ARGS) &sslot1, &sslot2, stats1, stats2, have_mcvs1, have_mcvs2, + hasmatch1, hasmatch2, inner_rel); else - selec = eqjoinsel_semi(opfuncoid, collation, + selec = eqjoinsel_semi(&eqproc, collation, true, &vardata2, &vardata1, nd2, nd1, @@ -2413,6 +2430,7 @@ eqjoinsel(PG_FUNCTION_ARGS) &sslot2, &sslot1, stats2, stats1, have_mcvs2, have_mcvs1, + hasmatch2, hasmatch1, inner_rel); /* @@ -2441,6 +2459,11 @@ eqjoinsel(PG_FUNCTION_ARGS) ReleaseVariableStats(vardata1); ReleaseVariableStats(vardata2); + if (hasmatch1) + pfree(hasmatch1); + if (hasmatch2) + pfree(hasmatch2); + CLAMP_PROBABILITY(selec); PG_RETURN_FLOAT8((float8) selec); @@ -2449,17 +2472,22 @@ eqjoinsel(PG_FUNCTION_ARGS) /* * eqjoinsel_inner --- eqjoinsel for normal inner join * + * In addition to computing the selectivity estimate, this will fill + * the hasmatch1[] and hasmatch2[] arrays (if have_mcvs1 && have_mcvs2). + * We may be able to re-use that data in eqjoinsel_semi. + * * We also use this for LEFT/FULL outer joins; it's not presently clear * that it's worth trying to distinguish them here. */ static double -eqjoinsel_inner(Oid opfuncoid, Oid collation, +eqjoinsel_inner(FmgrInfo *eqproc, 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) + bool have_mcvs1, bool have_mcvs2, + bool *hasmatch1, bool *hasmatch2) { double selec; @@ -2477,8 +2505,6 @@ eqjoinsel_inner(Oid opfuncoid, Oid collation, * results", Technical Report 1018, Computer Science Dept., University * of Wisconsin, Madison, March 1991 (available from ftp.cs.wisc.edu). */ - bool *hasmatch1; - bool *hasmatch2; double nullfrac1 = stats1->stanullfrac; double nullfrac2 = stats2->stanullfrac; double matchprodfreq, @@ -2493,11 +2519,8 @@ eqjoinsel_inner(Oid opfuncoid, Oid collation, int i, nmatches; - /* Construct the match arrays */ - hasmatch1 = (bool *) palloc0(sslot1->nvalues * sizeof(bool)); - hasmatch2 = (bool *) palloc0(sslot2->nvalues * sizeof(bool)); - - eqjoinsel_find_matches(opfuncoid, collation, + /* Fill the match arrays */ + eqjoinsel_find_matches(eqproc, collation, false, sslot1, sslot2, sslot1->nvalues, sslot2->nvalues, @@ -2526,8 +2549,6 @@ eqjoinsel_inner(Oid opfuncoid, Oid collation, } CLAMP_PROBABILITY(matchfreq2); CLAMP_PROBABILITY(unmatchfreq2); - pfree(hasmatch1); - pfree(hasmatch2); /* * Compute total frequency of non-null values that are not in the MCV @@ -2607,12 +2628,13 @@ eqjoinsel_inner(Oid opfuncoid, Oid collation, * eqjoinsel_semi --- eqjoinsel for semi join * * (Also used for anti join, which we are supposed to estimate the same way.) - * Caller has ensured that vardata1 is the LHS variable; however, opfuncoid + * Caller has ensured that vardata1 is the LHS variable; however, eqproc * is for the original join operator, which might now need to have the inputs - * swapped in order to apply correctly. + * swapped in order to apply correctly. Also, if have_mcvs1 && have_mcvs2 + * then hasmatch1[] and hasmatch2[] were filled by eqjoinsel_inner. */ static double -eqjoinsel_semi(Oid opfuncoid, Oid collation, +eqjoinsel_semi(FmgrInfo *eqproc, Oid collation, bool op_is_reversed, VariableStatData *vardata1, VariableStatData *vardata2, double nd1, double nd2, @@ -2620,6 +2642,7 @@ eqjoinsel_semi(Oid opfuncoid, Oid collation, AttStatsSlot *sslot1, AttStatsSlot *sslot2, Form_pg_statistic stats1, Form_pg_statistic stats2, bool have_mcvs1, bool have_mcvs2, + bool *hasmatch1, bool *hasmatch2, RelOptInfo *inner_rel) { double selec; @@ -2667,8 +2690,6 @@ eqjoinsel_semi(Oid opfuncoid, Oid collation, * lists. We still have to estimate for the remaining population, but * in a skewed distribution this gives us a big leg up in accuracy. */ - bool *hasmatch1; - bool *hasmatch2; double nullfrac1 = stats1->stanullfrac; double matchprodfreq, matchfreq1, @@ -2687,16 +2708,29 @@ eqjoinsel_semi(Oid opfuncoid, Oid collation, */ clamped_nvalues2 = Min(sslot2->nvalues, nd2); - /* Construct the match arrays */ - hasmatch1 = (bool *) palloc0(sslot1->nvalues * sizeof(bool)); - hasmatch2 = (bool *) palloc0(clamped_nvalues2 * sizeof(bool)); - - eqjoinsel_find_matches(opfuncoid, collation, - op_is_reversed, - sslot1, sslot2, - sslot1->nvalues, clamped_nvalues2, - hasmatch1, hasmatch2, - &nmatches, &matchprodfreq); + /* + * If we did not set clamped_nvalues2 to less than sslot2->nvalues, + * then the hasmatch1[] and hasmatch2[] match flags computed by + * eqjoinsel_inner are still perfectly applicable, so we need not + * re-do the matching work. Note that it does not matter if + * op_is_reversed: we'd get the same answers. + * + * If we did clamp, then a different set of sslot2 values is to be + * compared, so we have to re-do the matching. + */ + if (clamped_nvalues2 != sslot2->nvalues) + { + /* Must re-zero the arrays */ + memset(hasmatch1, 0, sslot1->nvalues * sizeof(bool)); + memset(hasmatch2, 0, clamped_nvalues2 * sizeof(bool)); + /* Re-fill the match arrays */ + eqjoinsel_find_matches(eqproc, collation, + op_is_reversed, + sslot1, sslot2, + sslot1->nvalues, clamped_nvalues2, + hasmatch1, hasmatch2, + &nmatches, &matchprodfreq); + } /* Sum up frequencies of matched MCVs */ matchfreq1 = 0.0; @@ -2706,8 +2740,6 @@ eqjoinsel_semi(Oid opfuncoid, Oid collation, matchfreq1 += sslot1->numbers[i]; } CLAMP_PROBABILITY(matchfreq1); - pfree(hasmatch1); - pfree(hasmatch2); /* * Now we need to estimate the fraction of relation 1 that has at @@ -2765,9 +2797,9 @@ eqjoinsel_semi(Oid opfuncoid, Oid collation, * Identify matching MCVs for eqjoinsel_inner or eqjoinsel_semi. * * Inputs: - * opfuncoid: OID of equality function to use (might be reversed) + * eqproc: FmgrInfo for equality function to use (might be reversed) * collation: OID of collation to use - * op_is_reversed: indicates that opfuncoid compares right type to left type + * op_is_reversed: indicates that eqproc compares right type to left type * sslot1, sslot2: MCV values for the lefthand and righthand inputs * nvalues1, nvalues2: number of values to be considered (can be less than * sslotN->nvalues, but not more) @@ -2784,7 +2816,7 @@ eqjoinsel_semi(Oid opfuncoid, Oid collation, * math wouldn't add up... */ static void -eqjoinsel_find_matches(Oid opfuncoid, Oid collation, +eqjoinsel_find_matches(FmgrInfo *eqproc, Oid collation, bool op_is_reversed, AttStatsSlot *sslot1, AttStatsSlot *sslot2, int nvalues1, int nvalues2, @@ -2792,18 +2824,15 @@ eqjoinsel_find_matches(Oid opfuncoid, Oid collation, int *p_nmatches, double *p_matchprodfreq) { LOCAL_FCINFO(fcinfo, 2); - FmgrInfo eqproc; double matchprodfreq = 0.0; int nmatches = 0; - fmgr_info(opfuncoid, &eqproc); - /* * Save a few cycles by setting up the fcinfo struct just once. Using * FunctionCallInvoke directly also avoids failure if the eqproc returns * NULL, though really equality functions should never do that. */ - InitFunctionCallInfoData(*fcinfo, &eqproc, 2, collation, + InitFunctionCallInfoData(*fcinfo, eqproc, 2, collation, NULL, NULL); fcinfo->args[0].isnull = false; fcinfo->args[1].isnull = false; -- 2.43.7