Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit f84ff0c

Browse files
committed
Don't read MCV stats needlessly in eqjoinsel().
eqjoinsel() currently makes use of MCV stats only when we have such stats for both sides of the clause. As coded, though, it would fetch those stats even when they're present for just one side. This can be a bit expensive with high statistics targets, leading to wasted effort in common cases such as joining a unique column to a non-unique column. So it seems worth the trouble to do a quick pre-check to confirm that both sides have MCVs before fetching either. Also, tweak the API spec for get_attstatsslot() to document the method we're using here. David Geier, Tomas Vondra, Tom Lane Discussion: https://postgr.es/m/b9846ca0-5f1c-9b26-5881-aad3f42b07f0@gmail.com
1 parent d8678ab commit f84ff0c

File tree

2 files changed

+22
-2
lines changed

2 files changed

+22
-2
lines changed

src/backend/utils/adt/selfuncs.c

+18-2
Original file line numberDiff line numberDiff line change
@@ -2261,6 +2261,7 @@ eqjoinsel(PG_FUNCTION_ARGS)
22612261
Form_pg_statistic stats2 = NULL;
22622262
bool have_mcvs1 = false;
22632263
bool have_mcvs2 = false;
2264+
bool get_mcv_stats;
22642265
bool join_is_reversed;
22652266
RelOptInfo *inner_rel;
22662267

@@ -2275,11 +2276,25 @@ eqjoinsel(PG_FUNCTION_ARGS)
22752276
memset(&sslot1, 0, sizeof(sslot1));
22762277
memset(&sslot2, 0, sizeof(sslot2));
22772278

2279+
/*
2280+
* There is no use in fetching one side's MCVs if we lack MCVs for the
2281+
* other side, so do a quick check to verify that both stats exist.
2282+
*/
2283+
get_mcv_stats = (HeapTupleIsValid(vardata1.statsTuple) &&
2284+
HeapTupleIsValid(vardata2.statsTuple) &&
2285+
get_attstatsslot(&sslot1, vardata1.statsTuple,
2286+
STATISTIC_KIND_MCV, InvalidOid,
2287+
0) &&
2288+
get_attstatsslot(&sslot2, vardata2.statsTuple,
2289+
STATISTIC_KIND_MCV, InvalidOid,
2290+
0));
2291+
22782292
if (HeapTupleIsValid(vardata1.statsTuple))
22792293
{
22802294
/* note we allow use of nullfrac regardless of security check */
22812295
stats1 = (Form_pg_statistic) GETSTRUCT(vardata1.statsTuple);
2282-
if (statistic_proc_security_check(&vardata1, opfuncoid))
2296+
if (get_mcv_stats &&
2297+
statistic_proc_security_check(&vardata1, opfuncoid))
22832298
have_mcvs1 = get_attstatsslot(&sslot1, vardata1.statsTuple,
22842299
STATISTIC_KIND_MCV, InvalidOid,
22852300
ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS);
@@ -2289,7 +2304,8 @@ eqjoinsel(PG_FUNCTION_ARGS)
22892304
{
22902305
/* note we allow use of nullfrac regardless of security check */
22912306
stats2 = (Form_pg_statistic) GETSTRUCT(vardata2.statsTuple);
2292-
if (statistic_proc_security_check(&vardata2, opfuncoid))
2307+
if (get_mcv_stats &&
2308+
statistic_proc_security_check(&vardata2, opfuncoid))
22932309
have_mcvs2 = get_attstatsslot(&sslot2, vardata2.statsTuple,
22942310
STATISTIC_KIND_MCV, InvalidOid,
22952311
ATTSTATSSLOT_VALUES | ATTSTATSSLOT_NUMBERS);

src/backend/utils/cache/lsyscache.c

+4
Original file line numberDiff line numberDiff line change
@@ -3183,6 +3183,10 @@ get_attavgwidth(Oid relid, AttrNumber attnum)
31833183
*
31843184
* If it's desirable to call free_attstatsslot when get_attstatsslot might
31853185
* not have been called, memset'ing sslot to zeroes will allow that.
3186+
*
3187+
* Passing flags=0 can be useful to quickly check if the requested slot type
3188+
* exists. In this case no arrays are extracted, so free_attstatsslot need
3189+
* not be called.
31863190
*/
31873191
bool
31883192
get_attstatsslot(AttStatsSlot *sslot, HeapTuple statstuple,

0 commit comments

Comments
 (0)