Skip to content

Commit ea6c916

Browse files
tglsfdctglsfdc
committed
Fix non-bulletproof ScalarArrayOpExpr code for extended statistics.
statext_is_compatible_clause_internal() checked that the arguments of a ScalarArrayOpExpr are one Var and one Const, but it would allow cases where the Const was on the left. Subsequent uses of the clause are not expecting that and would suffer assertion failures or core dumps. mcv.c also had not bothered to cope with the case of a NULL array constant, which seems really unacceptably sloppy of somebody. (Although our tools failed us there too, since AFAIK neither Coverity nor any compiler warned of the obvious use-of-uninitialized-variable condition.) It seems best to handle that by having statext_is_compatible_clause_internal() reject it. Noted while fixing bug #17570. Back-patch to v13 where the extended stats code grew some awareness of ScalarArrayOpExpr.
1 parent fff3f33 commit ea6c916

File tree

4 files changed

+47
-24
lines changed

4 files changed

+47
-24
lines changed

‎src/backend/statistics/extended_stats.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,8 +1331,8 @@ choose_best_statistics(List *stats, char requiredkind,
13311331
*
13321332
* (c) combinations using AND/OR/NOT
13331333
*
1334-
* (d) ScalarArrayOpExprs of the form (Var/Expr op ANY (array)) or (Var/Expr
1335-
* op ALL (array))
1334+
* (d) ScalarArrayOpExprs of the form (Var/Expr op ANY (Const)) or
1335+
* (Var/Expr op ALL (Const))
13361336
*
13371337
* In the future, the range of supported clauses may be expanded to more
13381338
* complex cases, for example (Var op Var).
@@ -1452,13 +1452,19 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
14521452
RangeTblEntry *rte = root->simple_rte_array[relid];
14531453
ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) clause;
14541454
Node *clause_expr;
1455+
Const *cst;
1456+
bool expronleft;
14551457

14561458
/* Only expressions with two arguments are considered compatible. */
14571459
if (list_length(expr->args) != 2)
14581460
return false;
14591461

14601462
/* Check if the expression has the right shape (one Var, one Const) */
1461-
if (!examine_opclause_args(expr->args, &clause_expr, NULL, NULL))
1463+
if (!examine_opclause_args(expr->args, &clause_expr, &cst, &expronleft))
1464+
return false;
1465+
1466+
/* We only support Var on left and non-null array constants */
1467+
if (!expronleft || cst->constisnull)
14621468
return false;
14631469

14641470
/*

‎src/backend/statistics/mcv.c

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1746,20 +1746,17 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
17461746
if (!examine_opclause_args(expr->args, &clause_expr, &cst, &expronleft))
17471747
elog(ERROR, "incompatible clause");
17481748

1749-
/* ScalarArrayOpExpr has the Var always on the left */
1750-
Assert(expronleft);
1749+
/* We expect Var on left and non-null constant on right */
1750+
if (!expronleft || cst->constisnull)
1751+
elog(ERROR, "incompatible clause");
17511752

1752-
/* XXX what if (cst->constisnull == NULL)? */
1753-
if (!cst->constisnull)
1754-
{
1755-
arrayval = DatumGetArrayTypeP(cst->constvalue);
1756-
get_typlenbyvalalign(ARR_ELEMTYPE(arrayval),
1757-
&elmlen, &elmbyval, &elmalign);
1758-
deconstruct_array(arrayval,
1759-
ARR_ELEMTYPE(arrayval),
1760-
elmlen, elmbyval, elmalign,
1761-
&elem_values, &elem_nulls, &num_elems);
1762-
}
1753+
arrayval = DatumGetArrayTypeP(cst->constvalue);
1754+
get_typlenbyvalalign(ARR_ELEMTYPE(arrayval),
1755+
&elmlen, &elmbyval, &elmalign);
1756+
deconstruct_array(arrayval,
1757+
ARR_ELEMTYPE(arrayval),
1758+
elmlen, elmbyval, elmalign,
1759+
&elem_values, &elem_nulls, &num_elems);
17631760

17641761
/* match the attribute/expression to a dimension of the statistic */
17651762
idx = mcv_match_expression(clause_expr, keys, exprs, &collid);

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1822,7 +1822,8 @@ CREATE TABLE mcv_lists (
18221822
b VARCHAR,
18231823
filler3 DATE,
18241824
c INT,
1825-
d TEXT
1825+
d TEXT,
1826+
ia INT[]
18261827
)
18271828
WITH (autovacuum_enabled = off);
18281829
-- random data (no MCV list)
@@ -1892,8 +1893,9 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE mod(a,7) = 1 A
18921893
-- 100 distinct combinations, all in the MCV list
18931894
TRUNCATE mcv_lists;
18941895
DROP STATISTICS mcv_lists_stats;
1895-
INSERT INTO mcv_lists (a, b, c, filler1)
1896-
SELECT mod(i,100), mod(i,50), mod(i,25), i FROM generate_series(1,5000) s(i);
1896+
INSERT INTO mcv_lists (a, b, c, ia, filler1)
1897+
SELECT mod(i,100), mod(i,50), mod(i,25), array[mod(i,25)], i
1898+
FROM generate_series(1,5000) s(i);
18971899
ANALYZE mcv_lists;
18981900
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = 1 AND b = ''1''');
18991901
estimated | actual
@@ -2033,8 +2035,14 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY
20332035
1 | 100
20342036
(1 row)
20352037

2038+
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = ANY (ARRAY[4,5]) AND 4 = ANY(ia)');
2039+
estimated | actual
2040+
-----------+--------
2041+
4 | 50
2042+
(1 row)
2043+
20362044
-- create statistics
2037-
CREATE STATISTICS mcv_lists_stats (mcv) ON a, b, c FROM mcv_lists;
2045+
CREATE STATISTICS mcv_lists_stats (mcv) ON a, b, c, ia FROM mcv_lists;
20382046
ANALYZE mcv_lists;
20392047
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = 1 AND b = ''1''');
20402048
estimated | actual
@@ -2180,6 +2188,12 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY
21802188
100 | 100
21812189
(1 row)
21822190

2191+
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = ANY (ARRAY[4,5]) AND 4 = ANY(ia)');
2192+
estimated | actual
2193+
-----------+--------
2194+
4 | 50
2195+
(1 row)
2196+
21832197
-- check change of unrelated column type does not reset the MCV statistics
21842198
ALTER TABLE mcv_lists ALTER COLUMN d TYPE VARCHAR(64);
21852199
SELECT d.stxdmcv IS NOT NULL

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,8 @@ CREATE TABLE mcv_lists (
908908
b VARCHAR,
909909
filler3 DATE,
910910
c INT,
911-
d TEXT
911+
d TEXT,
912+
ia INT[]
912913
)
913914
WITH (autovacuum_enabled = off);
914915

@@ -957,8 +958,9 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE mod(a,7) = 1 A
957958
TRUNCATE mcv_lists;
958959
DROP STATISTICS mcv_lists_stats;
959960

960-
INSERT INTO mcv_lists (a, b, c, filler1)
961-
SELECT mod(i,100), mod(i,50), mod(i,25), i FROM generate_series(1,5000) s(i);
961+
INSERT INTO mcv_lists (a, b, c, ia, filler1)
962+
SELECT mod(i,100), mod(i,50), mod(i,25), array[mod(i,25)], i
963+
FROM generate_series(1,5000) s(i);
962964

963965
ANALYZE mcv_lists;
964966

@@ -1008,8 +1010,10 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY
10081010

10091011
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY[4, 5]) AND b IN (''1'', ''2'', NULL, ''3'') AND c > ANY (ARRAY[1, 2, NULL, 3])');
10101012

1013+
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = ANY (ARRAY[4,5]) AND 4 = ANY(ia)');
1014+
10111015
-- create statistics
1012-
CREATE STATISTICS mcv_lists_stats (mcv) ON a, b, c FROM mcv_lists;
1016+
CREATE STATISTICS mcv_lists_stats (mcv) ON a, b, c, ia FROM mcv_lists;
10131017

10141018
ANALYZE mcv_lists;
10151019

@@ -1061,6 +1065,8 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY
10611065

10621066
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY[4, 5]) AND b IN (''1'', ''2'', NULL, ''3'') AND c > ANY (ARRAY[1, 2, NULL, 3])');
10631067

1068+
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = ANY (ARRAY[4,5]) AND 4 = ANY(ia)');
1069+
10641070
-- check change of unrelated column type does not reset the MCV statistics
10651071
ALTER TABLE mcv_lists ALTER COLUMN d TYPE VARCHAR(64);
10661072

0 commit comments

Comments
 (0)