Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit518442c

Browse files
committed
Fix handling of clauses incompatible with extended statistics
Handling of incompatible clauses while applying extended statistics wasa bit confused - while handling a mix of compatible and incompatibleclauses it sometimes incorrectly treated the incompatible clauses ascompatible, resulting in a crash.Fixed by reworking the code applying the selected statistics object tomake it easier to understand, and adding a proper compatibility check.Reported-by: David RowleyDiscussion:https://postgr.es/m/CAApHDvpYT10-nkSp8xXe-nbO3jmoaRyRFHbzh-RWMfAJynqgpQ%40mail.gmail.com
1 parent7ab96cf commit518442c

File tree

4 files changed

+100
-28
lines changed

4 files changed

+100
-28
lines changed

‎src/backend/statistics/extended_stats.c

Lines changed: 58 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1255,6 +1255,10 @@ choose_best_statistics(List *stats, char requiredkind,
12551255
/*
12561256
* Collect attributes and expressions in remaining (unestimated)
12571257
* clauses fully covered by this statistic object.
1258+
*
1259+
* We know already estimated clauses have both clause_attnums and
1260+
* clause_exprs set to NULL. We leave the pointers NULL if already
1261+
* estimated, or we reset them to NULL after estimating the clause.
12581262
*/
12591263
for (i=0;i<nclauses;i++)
12601264
{
@@ -1758,39 +1762,65 @@ statext_mcv_clauselist_selectivity(PlannerInfo *root, List *clauses, int varReli
17581762
/* record which clauses are simple (single column or expression) */
17591763
simple_clauses=NULL;
17601764

1761-
listidx=0;
1765+
listidx=-1;
17621766
foreach(l,clauses)
17631767
{
1768+
/* Increment the index before we decide if to skip the clause. */
1769+
listidx++;
1770+
17641771
/*
1765-
* If the clause is not already estimated and is compatible with
1766-
* the selected statistics object (all attributes and expressions
1767-
* covered), mark it as estimated and add it to the list to
1768-
* estimate.
1772+
* Ignore clauses from which we did not extract any attnums or
1773+
* expressions (this needs to be consistent with what we do in
1774+
* choose_best_statistics).
1775+
*
1776+
* This also eliminates already estimated clauses - both those
1777+
* estimated before and during applying extended statistics.
1778+
*
1779+
* XXX This check is needed because both bms_is_subset and
1780+
* stat_covers_expressions return true for empty attnums and
1781+
* expressions.
17691782
*/
1770-
if (!bms_is_member(listidx,*estimatedclauses)&&
1771-
bms_is_subset(list_attnums[listidx],stat->keys)&&
1772-
stat_covers_expressions(stat,list_exprs[listidx],NULL))
1773-
{
1774-
/* record simple clauses (single column or expression) */
1775-
if ((list_attnums[listidx]==NULL&&
1776-
list_length(list_exprs[listidx])==1)||
1777-
(list_exprs[listidx]==NIL&&
1778-
bms_membership(list_attnums[listidx])==BMS_SINGLETON))
1779-
simple_clauses=bms_add_member(simple_clauses,
1780-
list_length(stat_clauses));
1781-
1782-
/* add clause to list and mark as estimated */
1783-
stat_clauses=lappend(stat_clauses, (Node*)lfirst(l));
1784-
*estimatedclauses=bms_add_member(*estimatedclauses,listidx);
1785-
1786-
bms_free(list_attnums[listidx]);
1787-
list_attnums[listidx]=NULL;
1788-
1789-
list_free(list_exprs[listidx]);
1790-
list_exprs[listidx]=NULL;
1791-
}
1783+
if (!list_attnums[listidx]&& !list_exprs[listidx])
1784+
continue;
17921785

1793-
listidx++;
1786+
/*
1787+
* The clause was not estimated yet, and we've extracted either
1788+
* attnums of expressions from it. Ignore it if it's not fully
1789+
* covered by the chosen statistics.
1790+
*
1791+
* We need to check both attributes and expressions, and reject
1792+
* if either is not covered.
1793+
*/
1794+
if (!bms_is_subset(list_attnums[listidx],stat->keys)||
1795+
!stat_covers_expressions(stat,list_exprs[listidx],NULL))
1796+
continue;
1797+
1798+
/*
1799+
* Now we know the clause is compatible (we have either atttnums
1800+
* or expressions extracted from it), and was not estimated yet.
1801+
*/
1802+
1803+
/* record simple clauses (single column or expression) */
1804+
if ((list_attnums[listidx]==NULL&&
1805+
list_length(list_exprs[listidx])==1)||
1806+
(list_exprs[listidx]==NIL&&
1807+
bms_membership(list_attnums[listidx])==BMS_SINGLETON))
1808+
simple_clauses=bms_add_member(simple_clauses,
1809+
list_length(stat_clauses));
1810+
1811+
/* add clause to list and mark it as estimated */
1812+
stat_clauses=lappend(stat_clauses, (Node*)lfirst(l));
1813+
*estimatedclauses=bms_add_member(*estimatedclauses,listidx);
1814+
1815+
/*
1816+
* Reset the pointers, so that choose_best_statistics knows this
1817+
* clause was estimated and does not consider it again.
1818+
*/
1819+
bms_free(list_attnums[listidx]);
1820+
list_attnums[listidx]=NULL;
1821+
1822+
list_free(list_exprs[listidx]);
1823+
list_exprs[listidx]=NULL;
17941824
}
17951825

17961826
if (is_or)

‎src/backend/statistics/mcv.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1575,6 +1575,8 @@ mcv_match_expression(Node *expr, Bitmapset *keys, List *exprs, Oid *collid)
15751575
(idx <=bms_num_members(keys)+list_length(exprs)));
15761576
}
15771577

1578+
Assert((idx >=0)&& (idx<bms_num_members(keys)+list_length(exprs)));
1579+
15781580
returnidx;
15791581
}
15801582

@@ -1654,6 +1656,8 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
16541656
/* match the attribute/expression to a dimension of the statistic */
16551657
idx=mcv_match_expression(clause_expr,keys,exprs,&collid);
16561658

1659+
Assert((idx >=0)&& (idx<bms_num_members(keys)+list_length(exprs)));
1660+
16571661
/*
16581662
* Walk through the MCV items and evaluate the current clause. We
16591663
* can skip items that were already ruled out, and terminate if

‎src/test/regress/expected/stats_ext.out

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2938,6 +2938,25 @@ SELECT * FROM check_estimated_rows('SELECT * FROM expr_stats WHERE a = 0 AND (b
29382938
(1 row)
29392939

29402940
DROP TABLE expr_stats;
2941+
-- test handling of a mix of compatible and incompatible expressions
2942+
CREATE TABLE expr_stats_incompatible_test (
2943+
c0 double precision,
2944+
c1 boolean NOT NULL
2945+
);
2946+
CREATE STATISTICS expr_stat_comp_1 ON c0, c1 FROM expr_stats_incompatible_test;
2947+
INSERT INTO expr_stats_incompatible_test VALUES (1234,false), (5678,true);
2948+
ANALYZE expr_stats_incompatible_test;
2949+
SELECT c0 FROM ONLY expr_stats_incompatible_test WHERE
2950+
(
2951+
upper('x') LIKE ('x'||('[0,1]'::int4range))
2952+
AND
2953+
(c0 IN (0, 1) OR c1)
2954+
);
2955+
c0
2956+
----
2957+
(0 rows)
2958+
2959+
DROP TABLE expr_stats_incompatible_test;
29412960
-- Permission tests. Users should not be able to see specific data values in
29422961
-- the extended statistics, if they lack permission to see those values in
29432962
-- the underlying table.

‎src/test/regress/sql/stats_ext.sql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,6 +1470,25 @@ SELECT * FROM check_estimated_rows('SELECT * FROM expr_stats WHERE a = 0 AND (b
14701470

14711471
DROPTABLE expr_stats;
14721472

1473+
-- test handling of a mix of compatible and incompatible expressions
1474+
CREATETABLEexpr_stats_incompatible_test (
1475+
c0double precision,
1476+
c1booleanNOT NULL
1477+
);
1478+
1479+
CREATE STATISTICS expr_stat_comp_1ON c0, c1FROM expr_stats_incompatible_test;
1480+
1481+
INSERT INTO expr_stats_incompatible_testVALUES (1234,false), (5678,true);
1482+
ANALYZE expr_stats_incompatible_test;
1483+
1484+
SELECT c0FROM ONLY expr_stats_incompatible_testWHERE
1485+
(
1486+
upper('x')LIKE ('x'||('[0,1]'::int4range))
1487+
AND
1488+
(c0IN (0,1)OR c1)
1489+
);
1490+
1491+
DROPTABLE expr_stats_incompatible_test;
14731492

14741493
-- Permission tests. Users should not be able to see specific data values in
14751494
-- the extended statistics, if they lack permission to see those values in

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp