mirror of
https://github.com/postgres/postgres.git
synced 2026-02-03 20:40:14 -05:00
Fix pg_restore_extended_stats() with expressions
This commit fixes an issue with the restore of ndistinct and
dependencies statistics, causing the operation to fail when any of these
kinds included expressions.
In extended statistics, expressions use strictly negative attribute
numbers, decremented from -1. For example, let's imagine an object
defined as follows:
CREATE STATISTICS stats_obj (dependencies) ON lower(name), upper(name)
FROM tab_obj;
This object would generate dependencies stats using -1 and -2 as
attribute numbers, like that:
[{"attributes": [-1], "dependency": -2, "degree": 1.000000},
{"attributes": [-2], "dependency": -1, "degree": 1.000000}]
However, pg_restore_extended_stats() forgot to account for the number of
expressions defined in an extended statistics object. This would cause
the validation step of ndistinct and dependencies data to fail,
preventing a restore of their stats even if the input is valid.
This issue has come up due to an incorrect split of the patch set. Some
tests are included to cover this behavior.
Author: Corey Huinker <corey.huinker@gmail.com>
Co-authored-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/aXl4bMfSTQUxM_yy@paquier.xyz
This commit is contained in:
Michael Paquier
parent
f9562b95c6
commit
e3094679b9
3 changed files with 179 additions and 1 deletions
|
|
@ -24,6 +24,8 @@
|
|||
#include "catalog/pg_statistic_ext_data.h"
|
||||
#include "miscadmin.h"
|
||||
#include "nodes/makefuncs.h"
|
||||
#include "nodes/nodeFuncs.h"
|
||||
#include "optimizer/optimizer.h"
|
||||
#include "statistics/extended_stats_internal.h"
|
||||
#include "statistics/stat_utils.h"
|
||||
#include "utils/acl.h"
|
||||
|
|
@ -248,6 +250,8 @@ extended_statistics_update(FunctionCallInfo fcinfo)
|
|||
bool nulls[Natts_pg_statistic_ext_data] = {0};
|
||||
bool replaces[Natts_pg_statistic_ext_data] = {0};
|
||||
bool success = true;
|
||||
Datum exprdatum;
|
||||
bool isnull;
|
||||
int numexprs = 0;
|
||||
|
||||
/* arrays of type info, if we need them */
|
||||
|
|
@ -341,6 +345,38 @@ extended_statistics_update(FunctionCallInfo fcinfo)
|
|||
/* Find out what extended statistics kinds we should expect. */
|
||||
expand_stxkind(tup, &enabled);
|
||||
|
||||
/* decode expression (if any) */
|
||||
exprdatum = SysCacheGetAttr(STATEXTOID,
|
||||
tup,
|
||||
Anum_pg_statistic_ext_stxexprs,
|
||||
&isnull);
|
||||
if (!isnull)
|
||||
{
|
||||
char *s;
|
||||
List *exprs;
|
||||
|
||||
s = TextDatumGetCString(exprdatum);
|
||||
exprs = (List *) stringToNode(s);
|
||||
pfree(s);
|
||||
|
||||
/*
|
||||
* Run the expressions through eval_const_expressions(). This is not
|
||||
* just an optimization, but is necessary, because the planner will be
|
||||
* comparing them to similarly-processed qual clauses, and may fail to
|
||||
* detect valid matches without this.
|
||||
*
|
||||
* We must not use canonicalize_qual(), however, since these are not
|
||||
* qual expressions.
|
||||
*/
|
||||
exprs = (List *) eval_const_expressions(NULL, (Node *) exprs);
|
||||
|
||||
/* May as well fix opfuncids too */
|
||||
fix_opfuncids((Node *) exprs);
|
||||
|
||||
/* Compute the number of expression, for input validation. */
|
||||
numexprs = list_length(exprs);
|
||||
}
|
||||
|
||||
/*
|
||||
* If the object cannot support ndistinct, we should not have data for it.
|
||||
*/
|
||||
|
|
@ -422,7 +458,8 @@ extended_statistics_update(FunctionCallInfo fcinfo)
|
|||
bytea *data = DatumGetByteaPP(dependencies_datum);
|
||||
MVDependencies *dependencies = statext_dependencies_deserialize(data);
|
||||
|
||||
if (statext_dependencies_validate(dependencies, &stxform->stxkeys, numexprs, WARNING))
|
||||
if (statext_dependencies_validate(dependencies, &stxform->stxkeys,
|
||||
numexprs, WARNING))
|
||||
{
|
||||
values[Anum_pg_statistic_ext_data_stxddependencies - 1] = dependencies_datum;
|
||||
nulls[Anum_pg_statistic_ext_data_stxddependencies - 1] = false;
|
||||
|
|
|
|||
|
|
@ -1147,6 +1147,12 @@ CREATE STATISTICS stats_import.test_stat_ndistinct (ndistinct)
|
|||
CREATE STATISTICS stats_import.test_stat_dependencies (dependencies)
|
||||
ON name, comp
|
||||
FROM stats_import.test;
|
||||
CREATE STATISTICS stats_import.test_stat_ndistinct_exprs (ndistinct)
|
||||
ON lower(name), upper(name)
|
||||
FROM stats_import.test;
|
||||
CREATE STATISTICS stats_import.test_stat_dependencies_exprs (dependencies)
|
||||
ON lower(name), upper(name)
|
||||
FROM stats_import.test;
|
||||
-- Generate statistics on table with data
|
||||
ANALYZE stats_import.test;
|
||||
CREATE TABLE stats_import.test_clone ( LIKE stats_import.test )
|
||||
|
|
@ -1814,6 +1820,62 @@ SELECT pg_catalog.pg_restore_extended_stats(
|
|||
t
|
||||
(1 row)
|
||||
|
||||
-- ndistinct with expressions, invalid attributes.
|
||||
SELECT pg_catalog.pg_restore_extended_stats(
|
||||
'schemaname', 'stats_import',
|
||||
'relname', 'test',
|
||||
'statistics_schemaname', 'stats_import',
|
||||
'statistics_name', 'test_stat_ndistinct_exprs',
|
||||
'inherited', false,
|
||||
'n_distinct', '[{"attributes" : [1,-1], "ndistinct" : 4}]'::pg_ndistinct);
|
||||
WARNING: could not validate "pg_ndistinct" object: invalid attribute number 1 found
|
||||
pg_restore_extended_stats
|
||||
---------------------------
|
||||
f
|
||||
(1 row)
|
||||
|
||||
-- ok: ndistinct with expressions.
|
||||
SELECT pg_catalog.pg_restore_extended_stats(
|
||||
'schemaname', 'stats_import',
|
||||
'relname', 'test',
|
||||
'statistics_schemaname', 'stats_import',
|
||||
'statistics_name', 'test_stat_ndistinct_exprs',
|
||||
'inherited', false,
|
||||
'n_distinct', '[{"attributes" : [-1,-2], "ndistinct" : 4}]'::pg_ndistinct);
|
||||
pg_restore_extended_stats
|
||||
---------------------------
|
||||
t
|
||||
(1 row)
|
||||
|
||||
-- dependencies with expressions, invalid attributes.
|
||||
SELECT pg_catalog.pg_restore_extended_stats(
|
||||
'schemaname', 'stats_import',
|
||||
'relname', 'test',
|
||||
'statistics_schemaname', 'stats_import',
|
||||
'statistics_name', 'test_stat_dependencies_exprs',
|
||||
'inherited', false,
|
||||
'dependencies', '[{"attributes": [-1], "dependency": 1, "degree": 1.000000},
|
||||
{"attributes": [1], "dependency": -1, "degree": 1.000000}]'::pg_dependencies);
|
||||
WARNING: could not validate "pg_dependencies" object: invalid attribute number 1 found
|
||||
pg_restore_extended_stats
|
||||
---------------------------
|
||||
f
|
||||
(1 row)
|
||||
|
||||
-- ok: dependencies with expressions
|
||||
SELECT pg_catalog.pg_restore_extended_stats(
|
||||
'schemaname', 'stats_import',
|
||||
'relname', 'test',
|
||||
'statistics_schemaname', 'stats_import',
|
||||
'statistics_name', 'test_stat_dependencies_exprs',
|
||||
'inherited', false,
|
||||
'dependencies', '[{"attributes": [-1], "dependency": -2, "degree": 1.000000},
|
||||
{"attributes": [-2], "dependency": -1, "degree": 1.000000}]'::pg_dependencies);
|
||||
pg_restore_extended_stats
|
||||
---------------------------
|
||||
t
|
||||
(1 row)
|
||||
|
||||
-- Check the presence of the restored stats, for each object.
|
||||
SELECT replace(e.n_distinct, '}, ', E'},\n') AS n_distinct
|
||||
FROM pg_stats_ext AS e
|
||||
|
|
@ -1836,6 +1898,27 @@ WHERE e.statistics_schemaname = 'stats_import' AND
|
|||
{"attributes": [3], "dependency": 2, "degree": 1.000000}]
|
||||
(1 row)
|
||||
|
||||
SELECT replace(e.n_distinct, '}, ', E'},\n') AS n_distinct
|
||||
FROM pg_stats_ext AS e
|
||||
WHERE e.statistics_schemaname = 'stats_import' AND
|
||||
e.statistics_name = 'test_stat_ndistinct_exprs' AND
|
||||
e.inherited = false;
|
||||
n_distinct
|
||||
--------------------------------------------
|
||||
[{"attributes": [-1, -2], "ndistinct": 4}]
|
||||
(1 row)
|
||||
|
||||
SELECT replace(e.dependencies, '}, ', E'},\n') AS dependencies
|
||||
FROM pg_stats_ext AS e
|
||||
WHERE e.statistics_schemaname = 'stats_import' AND
|
||||
e.statistics_name = 'test_stat_dependencies_exprs' AND
|
||||
e.inherited = false;
|
||||
dependencies
|
||||
--------------------------------------------------------------
|
||||
[{"attributes": [-1], "dependency": -2, "degree": 1.000000},+
|
||||
{"attributes": [-2], "dependency": -1, "degree": 1.000000}]
|
||||
(1 row)
|
||||
|
||||
DROP SCHEMA stats_import CASCADE;
|
||||
NOTICE: drop cascades to 7 other objects
|
||||
DETAIL: drop cascades to type stats_import.complex_type
|
||||
|
|
|
|||
|
|
@ -819,6 +819,14 @@ CREATE STATISTICS stats_import.test_stat_dependencies (dependencies)
|
|||
ON name, comp
|
||||
FROM stats_import.test;
|
||||
|
||||
CREATE STATISTICS stats_import.test_stat_ndistinct_exprs (ndistinct)
|
||||
ON lower(name), upper(name)
|
||||
FROM stats_import.test;
|
||||
|
||||
CREATE STATISTICS stats_import.test_stat_dependencies_exprs (dependencies)
|
||||
ON lower(name), upper(name)
|
||||
FROM stats_import.test;
|
||||
|
||||
-- Generate statistics on table with data
|
||||
ANALYZE stats_import.test;
|
||||
|
||||
|
|
@ -1297,6 +1305,44 @@ SELECT pg_catalog.pg_restore_extended_stats(
|
|||
'dependencies', '[{"attributes": [2], "dependency": 3, "degree": 1.000000},
|
||||
{"attributes": [3], "dependency": 2, "degree": 1.000000}]'::pg_dependencies);
|
||||
|
||||
-- ndistinct with expressions, invalid attributes.
|
||||
SELECT pg_catalog.pg_restore_extended_stats(
|
||||
'schemaname', 'stats_import',
|
||||
'relname', 'test',
|
||||
'statistics_schemaname', 'stats_import',
|
||||
'statistics_name', 'test_stat_ndistinct_exprs',
|
||||
'inherited', false,
|
||||
'n_distinct', '[{"attributes" : [1,-1], "ndistinct" : 4}]'::pg_ndistinct);
|
||||
|
||||
-- ok: ndistinct with expressions.
|
||||
SELECT pg_catalog.pg_restore_extended_stats(
|
||||
'schemaname', 'stats_import',
|
||||
'relname', 'test',
|
||||
'statistics_schemaname', 'stats_import',
|
||||
'statistics_name', 'test_stat_ndistinct_exprs',
|
||||
'inherited', false,
|
||||
'n_distinct', '[{"attributes" : [-1,-2], "ndistinct" : 4}]'::pg_ndistinct);
|
||||
|
||||
-- dependencies with expressions, invalid attributes.
|
||||
SELECT pg_catalog.pg_restore_extended_stats(
|
||||
'schemaname', 'stats_import',
|
||||
'relname', 'test',
|
||||
'statistics_schemaname', 'stats_import',
|
||||
'statistics_name', 'test_stat_dependencies_exprs',
|
||||
'inherited', false,
|
||||
'dependencies', '[{"attributes": [-1], "dependency": 1, "degree": 1.000000},
|
||||
{"attributes": [1], "dependency": -1, "degree": 1.000000}]'::pg_dependencies);
|
||||
|
||||
-- ok: dependencies with expressions
|
||||
SELECT pg_catalog.pg_restore_extended_stats(
|
||||
'schemaname', 'stats_import',
|
||||
'relname', 'test',
|
||||
'statistics_schemaname', 'stats_import',
|
||||
'statistics_name', 'test_stat_dependencies_exprs',
|
||||
'inherited', false,
|
||||
'dependencies', '[{"attributes": [-1], "dependency": -2, "degree": 1.000000},
|
||||
{"attributes": [-2], "dependency": -1, "degree": 1.000000}]'::pg_dependencies);
|
||||
|
||||
-- Check the presence of the restored stats, for each object.
|
||||
SELECT replace(e.n_distinct, '}, ', E'},\n') AS n_distinct
|
||||
FROM pg_stats_ext AS e
|
||||
|
|
@ -1310,4 +1356,16 @@ WHERE e.statistics_schemaname = 'stats_import' AND
|
|||
e.statistics_name = 'test_stat_dependencies' AND
|
||||
e.inherited = false;
|
||||
|
||||
SELECT replace(e.n_distinct, '}, ', E'},\n') AS n_distinct
|
||||
FROM pg_stats_ext AS e
|
||||
WHERE e.statistics_schemaname = 'stats_import' AND
|
||||
e.statistics_name = 'test_stat_ndistinct_exprs' AND
|
||||
e.inherited = false;
|
||||
|
||||
SELECT replace(e.dependencies, '}, ', E'},\n') AS dependencies
|
||||
FROM pg_stats_ext AS e
|
||||
WHERE e.statistics_schemaname = 'stats_import' AND
|
||||
e.statistics_name = 'test_stat_dependencies_exprs' AND
|
||||
e.inherited = false;
|
||||
|
||||
DROP SCHEMA stats_import CASCADE;
|
||||
|
|
|
|||
Loading…
Reference in a new issue