From 73619159fc4886dc35ecd933f1f54f695e4d61e5 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Mon, 15 Jul 2019 02:30:50 +0200 Subject: [PATCH 4/5] Refactor / simplify evaluation of MCV bitmap --- src/backend/statistics/mcv.c | 145 ++++++++++++++++------------------- 1 file changed, 65 insertions(+), 80 deletions(-) diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c index 77daa647e2..9697f3d101 100644 --- a/src/backend/statistics/mcv.c +++ b/src/backend/statistics/mcv.c @@ -1504,6 +1504,37 @@ pg_mcv_list_send(PG_FUNCTION_ARGS) return byteasend(fcinfo); } +/* + * mcv_result_merge + * Compute new value in result bitmap for AND/OR clauses. + * + * Compute new value for bitmap item, considering whether it's used for + * clauses connected by AND/OR. + */ +static bool +mcv_result_merge(bool value, bool is_or, bool match) +{ + return (is_or) ? (value || match) : (value && match); +} + +/* + * mcv_result_is_final + * Decide if the current bitmap value may change. + * + * When processing a list of clauses, the bitmap item may get set to a value + * such that additional clauses can't change it. For example, when processing + * a list of clauses connected to AND, as soon as the item gets set to 'false' + * then it'll remain like that. Similarly clauses connected by OR and 'true'. + * + * Returns true when the value in the bitmap can't change no matter how the + * remaining clauses are evaluated. + */ +static bool +mcv_result_is_final(bool value, bool is_or) +{ + return (is_or) ? value : (!value); +} + /* * mcv_get_match_bitmap * Evaluate clauses using the MCV list, and update the match bitmap. @@ -1589,27 +1620,27 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses, */ for (i = 0; i < mcvlist->nitems; i++) { - bool mismatch = false; + bool match = true; MCVItem *item = &mcvlist->items[i]; /* - * When the MCV item or the Const value is NULL we can treat - * this as a mismatch. We must not call the operator proc - * because of strictness. + * Handle NULL constants and NULL values in the MCV item by + * simply considering this to be a mismatch. + * + * We must not call the opproc, because it might be strict. */ - if (item->isnull[idx] || cst->constisnull) + if (cst->constisnull || item->isnull[idx]) { - /* we only care about AND, because OR can't change */ - if (!is_or) - matches[i] = false; - + matches[i] = mcv_result_merge(matches[i], is_or, false); continue; } - /* skip MCV items that were already ruled out */ - if ((!is_or) && (matches[i] == false)) - continue; - else if (is_or && (matches[i] == true)) + /* + * Skip MCV items that can't change result in the bitmap. + * Once the value gets false for AND-lists, or true for + * OR-lists, we don't need to look at more clauses. + */ + if (mcv_result_is_final(matches[i], is_or)) continue; switch (oprrest) @@ -1622,10 +1653,10 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses, * it does not matter whether it's (var op const) * or (const op var). */ - mismatch = !DatumGetBool(FunctionCall2Coll(&opproc, - var->varcollid, - cst->constvalue, - item->values[idx])); + match = DatumGetBool(FunctionCall2Coll(&opproc, + var->varcollid, + cst->constvalue, + item->values[idx])); break; @@ -1640,39 +1671,21 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses, * bucket, because there's no overlap). */ if (isgt) - mismatch = !DatumGetBool(FunctionCall2Coll(&opproc, - var->varcollid, - cst->constvalue, - item->values[idx])); + match = DatumGetBool(FunctionCall2Coll(&opproc, + var->varcollid, + cst->constvalue, + item->values[idx])); else - mismatch = !DatumGetBool(FunctionCall2Coll(&opproc, - var->varcollid, - item->values[idx], - cst->constvalue)); + match = DatumGetBool(FunctionCall2Coll(&opproc, + var->varcollid, + item->values[idx], + cst->constvalue)); break; } - /* - * XXX The conditions on matches[i] are not needed, as we - * skip MCV items that can't become true/false, depending - * on the current flag. See beginning of the loop over MCV - * items. - */ - - if ((is_or) && (!mismatch)) - { - /* OR - was not a match before, matches now */ - matches[i] = true; - continue; - } - else if ((!is_or) && mismatch) - { - /* AND - was a match before, does not match anymore */ - matches[i] = false; - continue; - } - + /* update the match bitmap with the result */ + matches[i] = mcv_result_merge(matches[i], is_or, match); } } } @@ -1707,10 +1720,7 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses, } /* now, update the match bitmap, depending on OR/AND type */ - if (is_or) - matches[i] = Max(matches[i], match); - else - matches[i] = Min(matches[i], match); + matches[i] = mcv_result_merge(matches[i], is_or, match); } } else if (is_orclause(clause) || is_andclause(clause)) @@ -1737,13 +1747,7 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses, * condition when merging the results. */ for (i = 0; i < mcvlist->nitems; i++) - { - /* Is this OR or AND clause? */ - if (is_or) - matches[i] = Max(matches[i], bool_matches[i]); - else - matches[i] = Min(matches[i], bool_matches[i]); - } + matches[i] = mcv_result_merge(matches[i], is_or, bool_matches[i]); pfree(bool_matches); } @@ -1767,25 +1771,11 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses, /* * Merge the bitmap produced by mcv_get_match_bitmap into the - * current one. + * current one. We're handling a NOT clause, so invert the result + * before merging it into the global bitmap. */ for (i = 0; i < mcvlist->nitems; i++) - { - /* - * When handling a NOT clause, we need to invert the result - * before merging it into the global result. - */ - if (not_matches[i] == false) - not_matches[i] = true; - else - not_matches[i] = false; - - /* Is this OR or AND clause? */ - if (is_or) - matches[i] = Max(matches[i], not_matches[i]); - else - matches[i] = Min(matches[i], not_matches[i]); - } + matches[i] = mcv_result_merge(matches[i], is_or, !not_matches[i]); pfree(not_matches); } @@ -1814,17 +1804,12 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses, if (!item->isnull[idx] && DatumGetBool(item->values[idx])) match = true; - /* now, update the match bitmap, depending on OR/AND type */ - if (is_or) - matches[i] = Max(matches[i], match); - else - matches[i] = Min(matches[i], match); + /* update the result bitmap */ + matches[i] = mcv_result_merge(matches[i], is_or, match); } } else - { elog(ERROR, "unknown clause type: %d", clause->type); - } } return matches; -- 2.20.1