Skip to content

Commit 94da732

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 d7714c1 commit 94da732

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, bool inh,
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
@@ -1837,7 +1837,8 @@ CREATE TABLE mcv_lists (
18371837
b VARCHAR,
18381838
filler3 DATE,
18391839
c INT,
1840-
d TEXT
1840+
d TEXT,
1841+
ia INT[]
18411842
)
18421843
WITH (autovacuum_enabled = off);
18431844
-- random data (no MCV list)
@@ -1907,8 +1908,9 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE mod(a,7) = 1 A
19071908
-- 100 distinct combinations, all in the MCV list
19081909
TRUNCATE mcv_lists;
19091910
DROP STATISTICS mcv_lists_stats;
1910-
INSERT INTO mcv_lists (a, b, c, filler1)
1911-
SELECT mod(i,100), mod(i,50), mod(i,25), i FROM generate_series(1,5000) s(i);
1911+
INSERT INTO mcv_lists (a, b, c, ia, filler1)
1912+
SELECT mod(i,100), mod(i,50), mod(i,25), array[mod(i,25)], i
1913+
FROM generate_series(1,5000) s(i);
19121914
ANALYZE mcv_lists;
19131915
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = 1 AND b = ''1''');
19141916
estimated | actual
@@ -2048,8 +2050,14 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY
20482050
1 | 100
20492051
(1 row)
20502052

2053+
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = ANY (ARRAY[4,5]) AND 4 = ANY(ia)');
2054+
estimated | actual
2055+
-----------+--------
2056+
4 | 50
2057+
(1 row)
2058+
20512059
-- create statistics
2052-
CREATE STATISTICS mcv_lists_stats (mcv) ON a, b, c FROM mcv_lists;
2060+
CREATE STATISTICS mcv_lists_stats (mcv) ON a, b, c, ia FROM mcv_lists;
20532061
ANALYZE mcv_lists;
20542062
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = 1 AND b = ''1''');
20552063
estimated | actual
@@ -2195,6 +2203,12 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY
21952203
100 | 100
21962204
(1 row)
21972205

2206+
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = ANY (ARRAY[4,5]) AND 4 = ANY(ia)');
2207+
estimated | actual
2208+
-----------+--------
2209+
4 | 50
2210+
(1 row)
2211+
21982212
-- check change of unrelated column type does not reset the MCV statistics
21992213
ALTER TABLE mcv_lists ALTER COLUMN d TYPE VARCHAR(64);
22002214
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
@@ -909,7 +909,8 @@ CREATE TABLE mcv_lists (
909909
b VARCHAR,
910910
filler3 DATE,
911911
c INT,
912-
d TEXT
912+
d TEXT,
913+
ia INT[]
913914
)
914915
WITH (autovacuum_enabled = off);
915916

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

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

964966
ANALYZE mcv_lists;
965967

@@ -1009,8 +1011,10 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY
10091011

10101012
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])');
10111013

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

10151019
ANALYZE mcv_lists;
10161020

@@ -1062,6 +1066,8 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY
10621066

10631067
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])');
10641068

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

0 commit comments

Comments
 (0)