Fix handling of NULLs in MCV items and constants

There were two issues in how the extended statistics handled NULL values
in opclauses. Firstly, the code was oblivious to the possibility that
Const may be NULL (constisnull=true) in which case the constvalue is
undefined. We need to treat this as a mismatch, and not call the proc.

Secondly, the MCV item itself may contain NULL values too - the code
already did check that, and updated the match bitmap accordingly, but
failed to ensure we won't call the operator procedure anyway. It did
work for AND-clauses, because in that case false in the bitmap stops
evaluation of further clauses. But for OR-clauses ir was not easy to
get incorrect estimates or even trigger a crash.

This fixes both issues by extending the existing check so that it looks
at constisnull too, and making sure it skips calling the procedure.

Discussion: https://postgr.es/m/8736jdhbhc.fsf%40ansel.ydns.eu

diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index a708a8f6740c4a0b6d9dbf0bdee042b965f52fa8..429cafd689b14eb62061b2b76671d89f420ac2ec 100644 (file)
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -1593,12 +1593,18 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses,
                                        MCVItem    *item = &mcvlist->items[i];
 
                                        /*
-                                        * For AND-lists, we can also mark NULL items as 'no
-                                        * match' (and then skip them). For OR-lists this is not
-                                        * possible.
+                                        * When the MCV item or the Const value is NULL we can treat
+                                        * this as a mismatch. We must not call the operator because
+                                        * of strictness.
                                         */
-                                       if ((!is_or) && item->isnull[idx])
-                                               matches[i] = false;
+                                       if (item->isnull[idx] || cst->constisnull)
+                                       {
+                                               /* we only care about AND, because OR can't change */
+                                               if (!is_or)
+                                                       matches[i] = false;
+
+                                               continue;
+                                       }
 
                                        /* skip MCV items that were already ruled out */
                                        if ((!is_or) && (matches[i] == false))

diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index 6a070a9649ddc4cd7b6e3781275d9c54dda6401d..94b8a8f8b85d55fb4c3d49ae188c461d5343202a 100644 (file)
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -619,6 +619,32 @@ SELECT m.*
      0 | {1,2,3} | {f,f,f} |         1 |              1
 (1 row)
 
+-- 2 distinct combinations with NULL values, all in the MCV list
+TRUNCATE mcv_lists;
+DROP STATISTICS mcv_lists_stats;
+INSERT INTO mcv_lists (a, b, c, d)
+     SELECT
+         (CASE WHEN mod(i,2) = 0 THEN NULL ELSE 0 END),
+         (CASE WHEN mod(i,2) = 0 THEN NULL ELSE 'x' END),
+         (CASE WHEN mod(i,2) = 0 THEN NULL ELSE 0 END),
+         (CASE WHEN mod(i,2) = 0 THEN NULL ELSE 'x' END)
+     FROM generate_series(1,5000) s(i);
+ANALYZE mcv_lists;
+SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE b = ''x'' OR d = ''x''');
+ estimated | actual 
+-----------+--------
+      3750 |   2500
+(1 row)
+
+-- create statistics
+CREATE STATISTICS mcv_lists_stats (mcv) ON b, d FROM mcv_lists;
+ANALYZE mcv_lists;
+SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE b = ''x'' OR d = ''x''');
+ estimated | actual 
+-----------+--------
+      2500 |   2500
+(1 row)
+
 -- mcv with arrays
 CREATE TABLE mcv_lists_arrays (
     a TEXT[],

diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index 7a77d20446e3d468839d974857c8790603e8a051..4bc1536727b46602a9d231248d19b24f54814464 100644 (file)
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -393,6 +393,29 @@ SELECT m.*
  WHERE s.stxname = 'mcv_lists_stats'
    AND d.stxoid = s.oid;
 
+-- 2 distinct combinations with NULL values, all in the MCV list
+TRUNCATE mcv_lists;
+DROP STATISTICS mcv_lists_stats;
+
+INSERT INTO mcv_lists (a, b, c, d)
+     SELECT
+         (CASE WHEN mod(i,2) = 0 THEN NULL ELSE 0 END),
+         (CASE WHEN mod(i,2) = 0 THEN NULL ELSE 'x' END),
+         (CASE WHEN mod(i,2) = 0 THEN NULL ELSE 0 END),
+         (CASE WHEN mod(i,2) = 0 THEN NULL ELSE 'x' END)
+     FROM generate_series(1,5000) s(i);
+
+ANALYZE mcv_lists;
+
+SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE b = ''x'' OR d = ''x''');
+
+-- create statistics
+CREATE STATISTICS mcv_lists_stats (mcv) ON b, d FROM mcv_lists;
+
+ANALYZE mcv_lists;
+
+SELECT * FROM check_estimated_rows('SELECT * FROM mcv_lists WHERE b = ''x'' OR d = ''x''');
+
 -- mcv with arrays
 CREATE TABLE mcv_lists_arrays (
     a TEXT[],