Fix issues with handling of expressions in extended stats restore

This commit addresses some defects with the handling of expressions in
pg_restore_extended_stats() and pg_clear_extended_stats():
- Misleading WARNING for an incorrect number of expressions, where the
number of required expressions was reported as the number of elements
given in input rather than the actual number of expressions expected by
the extstats object definition.
- Incorrect matching of expression names, where a key name was
considered as valid as long as it matched with the prefix of a legit key
name.  For example "correlatio" given in input would match with
"correlation", and be considered valid.  The consequence of this bug was
a silent discard of the input data, where the operation would be
considered a success.  The value associated to the prefixed key was not
inserted in the catalogs, just ignored.  pg_dump would not generate such
input data patterns, but a user doing manual stats injection could.
- Missing heap_freetuple() in pg_clear_extended_stats(), for the case
where the extstats object in input does not match with its parent
relation.

Author: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/A7C11B83-7534-4A09-9071-FBD09175CFC8@gmail.com

diff --git a/src/backend/statistics/extended_stats_funcs.c b/src/backend/statistics/extended_stats_funcs.c
index 70393d3a9040fd87193e123a2412d102dbbf1075..ab748068225ae5dd630db34494f9114e9eca7e42 100644 (file)
--- a/src/backend/statistics/extended_stats_funcs.c
+++ b/src/backend/statistics/extended_stats_funcs.c
@@ -886,7 +886,8 @@ key_in_expr_argnames(JsonbValue *key)
        Assert(key->type == jbvString);
        for (int i = 0; i < NUM_ATTRIBUTE_STATS_ELEMS; i++)
        {
-               if (strncmp(extexprargname[i], key->val.string.val, key->val.string.len) == 0)
+               if (strlen(extexprargname[i]) == key->val.string.len &&
+                       strncmp(extexprargname[i], key->val.string.val, key->val.string.len) == 0)
                        return true;
        }
        return false;
@@ -1592,7 +1593,7 @@ import_expressions(Relation pgsd, int numexprs,
                ereport(WARNING,
                                errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                                errmsg("could not parse \"%s\": incorrect number of elements (%d required)",
-                                          argname, num_root_elements));
+                                          argname, numexprs));
                goto exprs_error;
        }
 
@@ -1816,6 +1817,7 @@ pg_clear_extended_stats(PG_FUNCTION_ARGS)
         */
        if (stxform->stxrelid != relid)
        {
+               heap_freetuple(tup);
                table_close(pg_stext, RowExclusiveLock);
                ereport(WARNING,
                                errcode(ERRCODE_INVALID_PARAMETER_VALUE),

diff --git a/src/test/regress/expected/stats_import.out b/src/test/regress/expected/stats_import.out
index f421e83e2327099c082bdf2630befc5b69d275d2..183e68d65c278c715fd0b358befbd540322b947e 100644 (file)
--- a/src/test/regress/expected/stats_import.out
+++ b/src/test/regress/expected/stats_import.out
@@ -2455,7 +2455,7 @@ WARNING:  could not parse "exprs": root-level array required
  f
 (1 row)
 
--- wrong number of exprs
+-- wrong number of exprs, too few
 SELECT pg_catalog.pg_restore_extended_stats(
   'schemaname', 'stats_import',
   'relname', 'test_clone',
@@ -2463,7 +2463,21 @@ SELECT pg_catalog.pg_restore_extended_stats(
   'statistics_name', 'test_stat_clone',
   'inherited', false,
   'exprs', '[ { "avg_width": "4" } ]'::jsonb);
-WARNING:  could not parse "exprs": incorrect number of elements (1 required)
+WARNING:  could not parse "exprs": incorrect number of elements (2 required)
+ pg_restore_extended_stats 
+---------------------------
+ f
+(1 row)
+
+-- wrong number of exprs, too many
+SELECT pg_catalog.pg_restore_extended_stats(
+  'schemaname', 'stats_import',
+  'relname', 'test_clone',
+  'statistics_schemaname', 'stats_import',
+  'statistics_name', 'test_stat_clone',
+  'inherited', false,
+  'exprs', '[ { "avg_width": "4" }, { "avg_width": "4" }, { "avg_width": "4" } ]'::jsonb);
+WARNING:  could not parse "exprs": incorrect number of elements (2 required)
  pg_restore_extended_stats 
 ---------------------------
  f
@@ -3256,6 +3270,20 @@ most_common_elems      | {-1,0,1,2,3}
 most_common_elem_freqs | {0.25,0.25,0.5,0.25,0.25,0.25,0.5,0.25}
 elem_count_histogram   | {1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,1.5}
 
+-- bad: exprs param which is a prefix of a valid key name
+SELECT pg_catalog.pg_restore_extended_stats(
+  'schemaname', 'stats_import',
+  'relname', 'test',
+  'statistics_schemaname', 'stats_import',
+  'statistics_name', 'test_stat_mcelem',
+  'inherited', false,
+  'exprs', '[{ "n": "-1" }]'::jsonb);
+WARNING:  could not import element in expression -1: invalid key name
+ pg_restore_extended_stats 
+---------------------------
+ f
+(1 row)
+
 -- ok: tsvector exceptions, test just the collation exceptions
 CREATE STATISTICS stats_import.test_stat_tsvec ON (length(name)), (to_tsvector(name)) FROM stats_import.test;
 SELECT pg_catalog.pg_restore_extended_stats(

diff --git a/src/test/regress/sql/stats_import.sql b/src/test/regress/sql/stats_import.sql
index c1bf55690a6bce82648e439a93dfa59361432883..6064b7722da898c9c1d65fd20865778f62e605ad 100644 (file)
--- a/src/test/regress/sql/stats_import.sql
+++ b/src/test/regress/sql/stats_import.sql
@@ -1750,7 +1750,7 @@ SELECT pg_catalog.pg_restore_extended_stats(
   'statistics_name', 'test_stat_clone',
   'inherited', false,
   'exprs', '{ "avg_width": "4", "null_frac": "0" }'::jsonb);
--- wrong number of exprs
+-- wrong number of exprs, too few
 SELECT pg_catalog.pg_restore_extended_stats(
   'schemaname', 'stats_import',
   'relname', 'test_clone',
@@ -1758,6 +1758,14 @@ SELECT pg_catalog.pg_restore_extended_stats(
   'statistics_name', 'test_stat_clone',
   'inherited', false,
   'exprs', '[ { "avg_width": "4" } ]'::jsonb);
+-- wrong number of exprs, too many
+SELECT pg_catalog.pg_restore_extended_stats(
+  'schemaname', 'stats_import',
+  'relname', 'test_clone',
+  'statistics_schemaname', 'stats_import',
+  'statistics_name', 'test_stat_clone',
+  'inherited', false,
+  'exprs', '[ { "avg_width": "4" }, { "avg_width": "4" }, { "avg_width": "4" } ]'::jsonb);
 -- incorrect type of value: should be a string or a NULL.
 SELECT pg_catalog.pg_restore_extended_stats(
   'schemaname', 'stats_import',
@@ -2244,6 +2252,15 @@ WHERE e.statistics_schemaname = 'stats_import' AND
     e.inherited = false
 \gx
 
+-- bad: exprs param which is a prefix of a valid key name
+SELECT pg_catalog.pg_restore_extended_stats(
+  'schemaname', 'stats_import',
+  'relname', 'test',
+  'statistics_schemaname', 'stats_import',
+  'statistics_name', 'test_stat_mcelem',
+  'inherited', false,
+  'exprs', '[{ "n": "-1" }]'::jsonb);
+
 -- ok: tsvector exceptions, test just the collation exceptions
 CREATE STATISTICS stats_import.test_stat_tsvec ON (length(name)), (to_tsvector(name)) FROM stats_import.test;
 SELECT pg_catalog.pg_restore_extended_stats(