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

Commit c102d11

Browse files
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 c122a99 commit c102d11

File tree

4 files changed

+50
-23
lines changed

4 files changed

+50
-23
lines changed

src/backend/statistics/extended_stats.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,13 +1051,19 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
10511051
RangeTblEntry *rte = root->simple_rte_array[relid];
10521052
ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) clause;
10531053
Var *var;
1054+
Const *cst;
1055+
bool expronleft;
10541056

10551057
/* Only expressions with two arguments are considered compatible. */
10561058
if (list_length(expr->args) != 2)
10571059
return false;
10581060

10591061
/* Check if the expression has the right shape (one Var, one Const) */
1060-
if (!examine_clause_args(expr->args, &var, NULL, NULL))
1062+
if (!examine_clause_args(expr->args, &var, &cst, &expronleft))
1063+
return false;
1064+
1065+
/* We only support Var on left and non-null array constants */
1066+
if (!expronleft || cst->constisnull)
10611067
return false;
10621068

10631069
/*
@@ -1161,7 +1167,7 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
11611167
* statext_is_compatible_clause
11621168
* Determines if the clause is compatible with MCV lists.
11631169
*
1164-
* Currently, we only support three types of clauses:
1170+
* Currently, we only support the following types of clauses:
11651171
*
11661172
* (a) OpExprs of the form (Var op Const), or (Const op Var), where the op
11671173
* is one of ("=", "<", ">", ">=", "<=")
@@ -1170,6 +1176,9 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause,
11701176
*
11711177
* (c) combinations using AND/OR/NOT
11721178
*
1179+
* (d) ScalarArrayOpExprs of the form (Var op ANY (Const)) or
1180+
* (Var op ALL (Const))
1181+
*
11731182
* In the future, the range of supported clauses may be expanded to more
11741183
* complex cases, for example (Var op Var).
11751184
*/

src/backend/statistics/mcv.c

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,19 +1679,17 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
16791679
Datum *elem_values;
16801680
bool *elem_nulls;
16811681

1682-
/* ScalarArrayOpExpr has the Var always on the left */
1683-
Assert(varonleft);
1684-
1685-
if (!cst->constisnull)
1686-
{
1687-
arrayval = DatumGetArrayTypeP(cst->constvalue);
1688-
get_typlenbyvalalign(ARR_ELEMTYPE(arrayval),
1689-
&elmlen, &elmbyval, &elmalign);
1690-
deconstruct_array(arrayval,
1691-
ARR_ELEMTYPE(arrayval),
1692-
elmlen, elmbyval, elmalign,
1693-
&elem_values, &elem_nulls, &num_elems);
1694-
}
1682+
/* We expect Var on left and non-null constant on right */
1683+
if (!varonleft || cst->constisnull)
1684+
elog(ERROR, "incompatible clause");
1685+
1686+
arrayval = DatumGetArrayTypeP(cst->constvalue);
1687+
get_typlenbyvalalign(ARR_ELEMTYPE(arrayval),
1688+
&elmlen, &elmbyval, &elmalign);
1689+
deconstruct_array(arrayval,
1690+
ARR_ELEMTYPE(arrayval),
1691+
elmlen, elmbyval, elmalign,
1692+
&elem_values, &elem_nulls, &num_elems);
16951693

16961694
/* match the attribute to a dimension of the statistic */
16971695
idx = bms_member_index(keys, var->varattno);

src/test/regress/expected/stats_ext.out

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -933,7 +933,8 @@ CREATE TABLE mcv_lists (
933933
b VARCHAR,
934934
filler3 DATE,
935935
c INT,
936-
d TEXT
936+
d TEXT,
937+
ia INT[]
937938
)
938939
WITH (autovacuum_enabled = off);
939940
-- random data (no MCV list)
@@ -970,8 +971,9 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = 1 AND b =
970971
-- 100 distinct combinations, all in the MCV list
971972
TRUNCATE mcv_lists;
972973
DROP STATISTICS mcv_lists_stats;
973-
INSERT INTO mcv_lists (a, b, c, filler1)
974-
SELECT mod(i,100), mod(i,50), mod(i,25), i FROM generate_series(1,5000) s(i);
974+
INSERT INTO mcv_lists (a, b, c, ia, filler1)
975+
SELECT mod(i,100), mod(i,50), mod(i,25), array[mod(i,25)], i
976+
FROM generate_series(1,5000) s(i);
975977
ANALYZE mcv_lists;
976978
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = 1 AND b = ''1''');
977979
estimated | actual
@@ -1111,8 +1113,14 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY
11111113
1 | 100
11121114
(1 row)
11131115

1116+
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = ANY (ARRAY[4,5]) AND 4 = ANY(ia)');
1117+
estimated | actual
1118+
-----------+--------
1119+
4 | 50
1120+
(1 row)
1121+
11141122
-- create statistics
1115-
CREATE STATISTICS mcv_lists_stats (mcv) ON a, b, c FROM mcv_lists;
1123+
CREATE STATISTICS mcv_lists_stats (mcv) ON a, b, c, ia FROM mcv_lists;
11161124
ANALYZE mcv_lists;
11171125
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = 1 AND b = ''1''');
11181126
estimated | actual
@@ -1253,6 +1261,12 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = 1 OR b = '
12531261
343 | 200
12541262
(1 row)
12551263

1264+
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = ANY (ARRAY[4,5]) AND 4 = ANY(ia)');
1265+
estimated | actual
1266+
-----------+--------
1267+
4 | 50
1268+
(1 row)
1269+
12561270
-- check change of unrelated column type does not reset the MCV statistics
12571271
ALTER TABLE mcv_lists ALTER COLUMN d TYPE VARCHAR(64);
12581272
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
@@ -497,7 +497,8 @@ CREATE TABLE mcv_lists (
497497
b VARCHAR,
498498
filler3 DATE,
499499
c INT,
500-
d TEXT
500+
d TEXT,
501+
ia INT[]
501502
)
502503
WITH (autovacuum_enabled = off);
503504

@@ -524,8 +525,9 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = 1 AND b =
524525
TRUNCATE mcv_lists;
525526
DROP STATISTICS mcv_lists_stats;
526527

527-
INSERT INTO mcv_lists (a, b, c, filler1)
528-
SELECT mod(i,100), mod(i,50), mod(i,25), i FROM generate_series(1,5000) s(i);
528+
INSERT INTO mcv_lists (a, b, c, ia, filler1)
529+
SELECT mod(i,100), mod(i,50), mod(i,25), array[mod(i,25)], i
530+
FROM generate_series(1,5000) s(i);
529531

530532
ANALYZE mcv_lists;
531533

@@ -575,8 +577,10 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY
575577

576578
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])');
577579

580+
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = ANY (ARRAY[4,5]) AND 4 = ANY(ia)');
581+
578582
-- create statistics
579-
CREATE STATISTICS mcv_lists_stats (mcv) ON a, b, c FROM mcv_lists;
583+
CREATE STATISTICS mcv_lists_stats (mcv) ON a, b, c, ia FROM mcv_lists;
580584

581585
ANALYZE mcv_lists;
582586

@@ -627,6 +631,8 @@ SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a < ALL (ARRAY
627631
-- we can't use the statistic for OR clauses that are not fully covered (missing 'd' attribute)
628632
SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE a = 1 OR b = ''1'' OR c = 1 OR d IS NOT NULL');
629633

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

0 commit comments

Comments
 (0)