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

Commit 7d24f6a

Browse files
committed
Simplify bitmap updates in multivariate MCV code
When evaluating clauses on a multivariate MCV list, we build a bitmap tracking how the clauses match each item of the MCV list. When updating the bitmap we need to consider the current value (tracking how the item matches preceding clauses), match for the current clause and whether the clauses are connected by AND or OR. Until now the logic was copied on every place updating the bitmap, which was not quite readable. So just move it to a separate function and call it where needed. Backpatch to 12, where the code was introduced. While not a bugfix, this should make maintenance and future backpatches easier. Discussion: https://postgr.es/m/8736jdhbhc.fsf%40ansel.ydns.eu
1 parent e4deae7 commit 7d24f6a

File tree

1 file changed

+47
-76
lines changed
  • src/backend/statistics

1 file changed

+47
-76
lines changed

src/backend/statistics/mcv.c

+47-76
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,24 @@ static SortItem **build_column_frequencies(SortItem *groups, int ngroups,
8383
static int count_distinct_groups(int numrows, SortItem *items,
8484
MultiSortSupport mss);
8585

86+
/*
87+
* Compute new value for bitmap item, considering whether it's used for
88+
* clauses connected by AND/OR.
89+
*/
90+
#define RESULT_MERGE(value, is_or, match) \
91+
((is_or) ? ((value) || (match)) : ((value) && (match)))
92+
93+
/*
94+
* When processing a list of clauses, the bitmap item may get set to a value
95+
* such that additional clauses can't change it. For example, when processing
96+
* a list of clauses connected to AND, as soon as the item gets set to 'false'
97+
* then it'll remain like that. Similarly clauses connected by OR and 'true'.
98+
*
99+
* Returns true when the value in the bitmap can't change no matter how the
100+
* remaining clauses are evaluated.
101+
*/
102+
#define RESULT_IS_FINAL(value, is_or) ((is_or) ? (value) : (!(value)))
103+
86104
/*
87105
* get_mincount_for_mcv_list
88106
* Determine the minimum number of times a value needs to appear in
@@ -1589,7 +1607,7 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
15891607
*/
15901608
for (i = 0; i < mcvlist->nitems; i++)
15911609
{
1592-
bool mismatch = false;
1610+
bool match = true;
15931611
MCVItem *item = &mcvlist->items[i];
15941612

15951613
/*
@@ -1599,17 +1617,16 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
15991617
*/
16001618
if (item->isnull[idx] || cst->constisnull)
16011619
{
1602-
/* we only care about AND, because OR can't change */
1603-
if (!is_or)
1604-
matches[i] = false;
1605-
1620+
matches[i] = RESULT_MERGE(matches[i], is_or, false);
16061621
continue;
16071622
}
16081623

1609-
/* skip MCV items that were already ruled out */
1610-
if ((!is_or) && (matches[i] == false))
1611-
continue;
1612-
else if (is_or && (matches[i] == true))
1624+
/*
1625+
* Skip MCV items that can't change result in the bitmap.
1626+
* Once the value gets false for AND-lists, or true for
1627+
* OR-lists, we don't need to look at more clauses.
1628+
*/
1629+
if (RESULT_IS_FINAL(matches[i], is_or))
16131630
continue;
16141631

16151632
switch (oprrest)
@@ -1622,10 +1639,10 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
16221639
* it does not matter whether it's (var op const)
16231640
* or (const op var).
16241641
*/
1625-
mismatch = !DatumGetBool(FunctionCall2Coll(&opproc,
1626-
DEFAULT_COLLATION_OID,
1627-
cst->constvalue,
1628-
item->values[idx]));
1642+
match = DatumGetBool(FunctionCall2Coll(&opproc,
1643+
DEFAULT_COLLATION_OID,
1644+
cst->constvalue,
1645+
item->values[idx]));
16291646

16301647
break;
16311648

@@ -1640,39 +1657,21 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
16401657
* bucket, because there's no overlap).
16411658
*/
16421659
if (isgt)
1643-
mismatch = !DatumGetBool(FunctionCall2Coll(&opproc,
1644-
DEFAULT_COLLATION_OID,
1645-
cst->constvalue,
1646-
item->values[idx]));
1660+
match = DatumGetBool(FunctionCall2Coll(&opproc,
1661+
DEFAULT_COLLATION_OID,
1662+
cst->constvalue,
1663+
item->values[idx]));
16471664
else
1648-
mismatch = !DatumGetBool(FunctionCall2Coll(&opproc,
1649-
DEFAULT_COLLATION_OID,
1650-
item->values[idx],
1651-
cst->constvalue));
1665+
match = DatumGetBool(FunctionCall2Coll(&opproc,
1666+
DEFAULT_COLLATION_OID,
1667+
item->values[idx],
1668+
cst->constvalue));
16521669

16531670
break;
16541671
}
16551672

1656-
/*
1657-
* XXX The conditions on matches[i] are not needed, as we
1658-
* skip MCV items that can't become true/false, depending
1659-
* on the current flag. See beginning of the loop over MCV
1660-
* items.
1661-
*/
1662-
1663-
if ((is_or) && (!mismatch))
1664-
{
1665-
/* OR - was not a match before, matches now */
1666-
matches[i] = true;
1667-
continue;
1668-
}
1669-
else if ((!is_or) && mismatch)
1670-
{
1671-
/* AND - was a match before, does not match anymore */
1672-
matches[i] = false;
1673-
continue;
1674-
}
1675-
1673+
/* update the match bitmap with the result */
1674+
matches[i] = RESULT_MERGE(matches[i], is_or, match);
16761675
}
16771676
}
16781677
}
@@ -1707,10 +1706,7 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
17071706
}
17081707

17091708
/* now, update the match bitmap, depending on OR/AND type */
1710-
if (is_or)
1711-
matches[i] = Max(matches[i], match);
1712-
else
1713-
matches[i] = Min(matches[i], match);
1709+
matches[i] = RESULT_MERGE(matches[i], is_or, match);
17141710
}
17151711
}
17161712
else if (is_orclause(clause) || is_andclause(clause))
@@ -1737,13 +1733,7 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
17371733
* condition when merging the results.
17381734
*/
17391735
for (i = 0; i < mcvlist->nitems; i++)
1740-
{
1741-
/* Is this OR or AND clause? */
1742-
if (is_or)
1743-
matches[i] = Max(matches[i], bool_matches[i]);
1744-
else
1745-
matches[i] = Min(matches[i], bool_matches[i]);
1746-
}
1736+
matches[i] = RESULT_MERGE(matches[i], is_or, bool_matches[i]);
17471737

17481738
pfree(bool_matches);
17491739
}
@@ -1767,25 +1757,11 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
17671757

17681758
/*
17691759
* Merge the bitmap produced by mcv_get_match_bitmap into the
1770-
* current one.
1760+
* current one. We're handling a NOT clause, so invert the result
1761+
* before merging it into the global bitmap.
17711762
*/
17721763
for (i = 0; i < mcvlist->nitems; i++)
1773-
{
1774-
/*
1775-
* When handling a NOT clause, we need to invert the result
1776-
* before merging it into the global result.
1777-
*/
1778-
if (not_matches[i] == false)
1779-
not_matches[i] = true;
1780-
else
1781-
not_matches[i] = false;
1782-
1783-
/* Is this OR or AND clause? */
1784-
if (is_or)
1785-
matches[i] = Max(matches[i], not_matches[i]);
1786-
else
1787-
matches[i] = Min(matches[i], not_matches[i]);
1788-
}
1764+
matches[i] = RESULT_MERGE(matches[i], is_or, !not_matches[i]);
17891765

17901766
pfree(not_matches);
17911767
}
@@ -1814,17 +1790,12 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
18141790
if (!item->isnull[idx] && DatumGetBool(item->values[idx]))
18151791
match = true;
18161792

1817-
/* now, update the match bitmap, depending on OR/AND type */
1818-
if (is_or)
1819-
matches[i] = Max(matches[i], match);
1820-
else
1821-
matches[i] = Min(matches[i], match);
1793+
/* update the result bitmap */
1794+
matches[i] = RESULT_MERGE(matches[i], is_or, match);
18221795
}
18231796
}
18241797
else
1825-
{
18261798
elog(ERROR, "unknown clause type: %d", clause->type);
1827-
}
18281799
}
18291800

18301801
return matches;

0 commit comments

Comments
 (0)