Reject incorrect range_bounds_histograms in stats import functions

pg_restore_attribute_stats() and pg_restore_extended_stats()
(expressions) can handle a STATISTIC_KIND_BOUNDS_HISTOGRAM value, but
did not check its shape when importing, especially regarding:
- Empty ranges.
- Unsorted elements.

These properties are respected by ANALYZE in compute_range_stats(), when
computing a histogram for a [multi]range, and by the planner when
reading the data from the stats catalogs.  The effects of importing data
with these properties depend on the compilation options:
- A assertion would be hit, if enabled.
- A non-sensical value that would make the load of the stats fail.
- A failure is hit if an empty range is loaded.

While the owner of the table or the one with MAINTAIN rights is
responsible for the stats data inserted, buggy data makes little sense
if their load is going to fail.  This commit adds a validation step to
match what compute_range_stats() expects.

This issue would be unlikely hit in practice, so no backpatch is done.

Author: Ewan Young <kdbase.hack@gmail.com>
Discussion: https://postgr.es/m/CAON2xHNM809WLR_g0ymKgU-tWxtbyH1Xvh4fqzRqy9YP2A59pg@mail.gmail.com

diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c
index 1cc4d657231afe840cb83e7520b0de4944d5f14c..6d99850105d903c80c4954df673cc45f9130bda4 100644 (file)
--- a/src/backend/statistics/attribute_stats.c
+++ b/src/backend/statistics/attribute_stats.c
@@ -488,7 +488,8 @@ attribute_statistics_update(FunctionCallInfo fcinfo)
                                                                                        atttypid, atttypmod,
                                                                                        &converted);
 
-               if (converted)
+               if (converted &&
+                       statatt_check_bounds_histogram(stavalues))
                {
                        statatt_set_slot(values, nulls, replaces,
                                                         STATISTIC_KIND_BOUNDS_HISTOGRAM,

diff --git a/src/backend/statistics/extended_stats_funcs.c b/src/backend/statistics/extended_stats_funcs.c
index a5dce8a220641f3c2151fecf6983ff64e2614511..a3e56933b916ab1ceba20e4249d4223ce3f6fb70 100644 (file)
--- a/src/backend/statistics/extended_stats_funcs.c
+++ b/src/backend/statistics/extended_stats_funcs.c
@@ -1491,7 +1491,7 @@ import_pg_statistic(Relation pgsd, JsonbContainer *cont,
                                                                  extexprargname[RANGE_BOUNDS_HISTOGRAM_ELEM],
                                                                  &val_ok);
 
-               if (val_ok)
+               if (val_ok && statatt_check_bounds_histogram(stavalues))
                        statatt_set_slot(values, nulls, replaces,
                                                         STATISTIC_KIND_BOUNDS_HISTOGRAM,
                                                         InvalidOid, InvalidOid,

diff --git a/src/backend/statistics/stat_utils.c b/src/backend/statistics/stat_utils.c
index 0b190e8823703e8116f7e54b52552c436f791ebc..5fb6a91d95b3128094b3549a4b3b8c70ca0334b6 100644 (file)
--- a/src/backend/statistics/stat_utils.c
+++ b/src/backend/statistics/stat_utils.c
@@ -33,6 +33,7 @@
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
+#include "utils/rangetypes.h"
 #include "utils/rel.h"
 #include "utils/syscache.h"
 #include "utils/typcache.h"
@@ -742,3 +743,79 @@ statatt_init_empty_tuple(Oid reloid, int16 attnum, bool inherited,
                nulls[Anum_pg_statistic_stacoll1 + slotnum - 1] = false;
        }
 }
+
+/*
+ * Check that an imported bounds histogram (STATISTIC_KIND_BOUNDS_HISTOGRAM)
+ * is shaped the same way ANALYZE builds it in compute_range_stats().
+ *
+ * For both range-typed and multirange-typed columns the histogram is an array
+ * of ranges, so we take the range type from the array's element type.
+ */
+bool
+statatt_check_bounds_histogram(Datum arrayval)
+{
+       ArrayType  *arr = DatumGetArrayTypeP(arrayval);
+       Oid                     rngtypid = ARR_ELEMTYPE(arr);
+       TypeCacheEntry *typcache;
+       int16           elmlen;
+       bool            elmbyval;
+       char            elmalign;
+       Datum      *elems;
+       bool       *nulls;
+       int                     nelems;
+       RangeBound      prev_lower = {0};
+       RangeBound      prev_upper = {0};
+
+       typcache = lookup_type_cache(rngtypid, TYPECACHE_RANGE_INFO);
+
+       /*
+        * The element type should always be a range type here.  This is
+        * defensive. If it isn't, the bounds histogram is never consulted by the
+        * range estimator, and there is nothing to verify.
+        */
+       if (typcache->rngelemtype == NULL)
+               return true;
+
+       get_typlenbyvalalign(rngtypid, &elmlen, &elmbyval, &elmalign);
+       deconstruct_array(arr, rngtypid, elmlen, elmbyval, elmalign,
+                                         &elems, &nulls, &nelems);
+
+       for (int i = 0; i < nelems; i++)
+       {
+               RangeBound      lower,
+                                       upper;
+               bool            empty;
+
+               /*
+                * NULL elements are already rejected by statatt_build_stavalues() and
+                * array_in_safe().
+                */
+               range_deserialize(typcache, DatumGetRangeTypeP(elems[i]),
+                                                 &lower, &upper, &empty);
+
+               if (empty)
+               {
+                       ereport(WARNING,
+                                       (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                        errmsg("\"%s\" must not contain empty ranges",
+                                                       "range_bounds_histogram")));
+                       return false;
+               }
+
+               if (i > 0 &&
+                       (range_cmp_bounds(typcache, &lower, &prev_lower) < 0 ||
+                        range_cmp_bounds(typcache, &upper, &prev_upper) < 0))
+               {
+                       ereport(WARNING,
+                                       (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                        errmsg("\"%s\" must have its lower and upper bounds sorted in ascending order",
+                                                       "range_bounds_histogram")));
+                       return false;
+               }
+
+               prev_lower = lower;
+               prev_upper = upper;
+       }
+
+       return true;
+}

diff --git a/src/include/statistics/stat_utils.h b/src/include/statistics/stat_utils.h
index 74da7790579692a82451a1a3c32fecf850262c00..15e962dbb7c844ee498243afcfb9864a567f4c12 100644 (file)
--- a/src/include/statistics/stat_utils.h
+++ b/src/include/statistics/stat_utils.h
@@ -58,4 +58,6 @@ extern Datum statatt_build_stavalues(const char *staname, FmgrInfo *array_in, Da
 extern bool statatt_get_elem_type(Oid atttypid, char atttyptype,
                                                                  Oid *elemtypid, Oid *elem_eq_opr);
 
+extern bool statatt_check_bounds_histogram(Datum arrayval);
+
 #endif                                                 /* STATS_UTILS_H */

diff --git a/src/test/regress/expected/stats_import.out b/src/test/regress/expected/stats_import.out
index bfdff77afa7d79aacea60eaceefc39c622e8519a..f2ccb80cf62fb92f118157935998c5b2e00ea4cd 100644 (file)
--- a/src/test/regress/expected/stats_import.out
+++ b/src/test/regress/expected/stats_import.out
@@ -1187,6 +1187,34 @@ AND attname = 'arange';
  stats_import | test      | arange  | f         |      0.29 |         0 |          0 |                  |                   |                  |             |                   |                        |                      | {399,499,Infinity}     |              0.5 | {"[-1,1)","[0,4)","[1,4)","[1,100)"}
 (1 row)
 
+-- warn: range bounds histogram with unsorted elements
+SELECT pg_catalog.pg_restore_attribute_stats(
+    'schemaname', 'stats_import',
+    'relname', 'test',
+    'attname', 'arange',
+    'inherited', false::boolean,
+    'range_bounds_histogram', '{"[50,60)","[1,2)","[90,100)","[5,6)"}'::text
+    );
+WARNING:  "range_bounds_histogram" must have its lower and upper bounds sorted in ascending order
+ pg_restore_attribute_stats 
+----------------------------
+ f
+(1 row)
+
+-- warn: range bounds histogram with empty range
+SELECT pg_catalog.pg_restore_attribute_stats(
+    'schemaname', 'stats_import',
+    'relname', 'test',
+    'attname', 'arange',
+    'inherited', false::boolean,
+    'range_bounds_histogram', '{empty,"[1,2)","[3,4)"}'::text
+    );
+WARNING:  "range_bounds_histogram" must not contain empty ranges
+ pg_restore_attribute_stats 
+----------------------------
+ f
+(1 row)
+
 -- warn: cannot set most_common_elems for range type, rest ok
 SELECT pg_catalog.pg_restore_attribute_stats(
     'schemaname', 'stats_import',
@@ -2358,6 +2386,34 @@ HINT:  "range_length_histogram", "range_empty_frac", and "range_bounds_histogram
  f
 (1 row)
 
+-- warn: range bounds histogram with unsorted elements
+SELECT pg_catalog.pg_restore_extended_stats(
+  'schemaname', 'stats_import',
+  'relname', 'test_mr',
+  'statistics_schemaname', 'stats_import',
+  'statistics_name', 'test_mr_stat',
+  'inherited', false,
+  'exprs', '[{"range_length_histogram": "{10179,10189,10199}", "range_empty_frac": "0", "range_bounds_histogram": "{\"[50,60)\",\"[1,2)\",\"[90,100)\"}"}]'::jsonb);
+WARNING:  "range_bounds_histogram" must have its lower and upper bounds sorted in ascending order
+ pg_restore_extended_stats 
+---------------------------
+ f
+(1 row)
+
+-- warn: range bounds histogram with empty range
+SELECT pg_catalog.pg_restore_extended_stats(
+  'schemaname', 'stats_import',
+  'relname', 'test_mr',
+  'statistics_schemaname', 'stats_import',
+  'statistics_name', 'test_mr_stat',
+  'inherited', false,
+  'exprs', '[{"range_length_histogram": "{10179,10189,10199}", "range_empty_frac": "0", "range_bounds_histogram": "{empty,\"[1,2)\",\"[3,4)\"}"}]'::jsonb);
+WARNING:  "range_bounds_histogram" must not contain empty ranges
+ pg_restore_extended_stats 
+---------------------------
+ f
+(1 row)
+
 -- ok: multirange stats
 SELECT pg_catalog.pg_restore_extended_stats(
   'schemaname', 'stats_import',

diff --git a/src/test/regress/sql/stats_import.sql b/src/test/regress/sql/stats_import.sql
index 58140315efbcbbbf7859e140d25e4c0c645bccd9..650ce324c7e47492992ed0be4f9f8c6dfedb3b3f 100644 (file)
--- a/src/test/regress/sql/stats_import.sql
+++ b/src/test/regress/sql/stats_import.sql
@@ -885,6 +885,24 @@ AND tablename = 'test'
 AND inherited = false
 AND attname = 'arange';
 
+-- warn: range bounds histogram with unsorted elements
+SELECT pg_catalog.pg_restore_attribute_stats(
+    'schemaname', 'stats_import',
+    'relname', 'test',
+    'attname', 'arange',
+    'inherited', false::boolean,
+    'range_bounds_histogram', '{"[50,60)","[1,2)","[90,100)","[5,6)"}'::text
+    );
+
+-- warn: range bounds histogram with empty range
+SELECT pg_catalog.pg_restore_attribute_stats(
+    'schemaname', 'stats_import',
+    'relname', 'test',
+    'attname', 'arange',
+    'inherited', false::boolean,
+    'range_bounds_histogram', '{empty,"[1,2)","[3,4)"}'::text
+    );
+
 -- warn: cannot set most_common_elems for range type, rest ok
 SELECT pg_catalog.pg_restore_attribute_stats(
     'schemaname', 'stats_import',
@@ -1696,6 +1714,22 @@ SELECT pg_catalog.pg_restore_extended_stats(
   'statistics_name', 'test_mr_stat',
   'inherited', false,
   'exprs', '[{"range_bounds_histogram": "{\"[1,10200)\"}", "range_length_histogram": "{10179}"}]'::jsonb);
+-- warn: range bounds histogram with unsorted elements
+SELECT pg_catalog.pg_restore_extended_stats(
+  'schemaname', 'stats_import',
+  'relname', 'test_mr',
+  'statistics_schemaname', 'stats_import',
+  'statistics_name', 'test_mr_stat',
+  'inherited', false,
+  'exprs', '[{"range_length_histogram": "{10179,10189,10199}", "range_empty_frac": "0", "range_bounds_histogram": "{\"[50,60)\",\"[1,2)\",\"[90,100)\"}"}]'::jsonb);
+-- warn: range bounds histogram with empty range
+SELECT pg_catalog.pg_restore_extended_stats(
+  'schemaname', 'stats_import',
+  'relname', 'test_mr',
+  'statistics_schemaname', 'stats_import',
+  'statistics_name', 'test_mr_stat',
+  'inherited', false,
+  'exprs', '[{"range_length_histogram": "{10179,10189,10199}", "range_empty_frac": "0", "range_bounds_histogram": "{empty,\"[1,2)\",\"[3,4)\"}"}]'::jsonb);
 
 -- ok: multirange stats
 SELECT pg_catalog.pg_restore_extended_stats(