rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix dependencies for extended statistics objects. 

A stats object ought to have a dependency on each individual column
it reads, not the entire table.  Doing this honestly lets us get rid
of the hard-wired logic in RemoveStatisticsExt, which seems to have
been misguidedly modeled on RemoveStatistics; and it will be far easier
to extend to multiple tables later.

Also, add overlooked dependency on owner, and make the dependency on
schema be NORMAL like every other such dependency.

There remains some unfinished work here, which is to allow statistics
objects to be extension members.  That takes more effort than just
adding the dependency call, though, so I left it out for now.

initdb forced because this changes the set of pg_depend records that
should exist for a statistics object.

Discussion: https://postgr.es/m/22676.1494557205@sss.pgh.pa.us
This commit is contained in:
Tom Lane 2017-05-12 16:26:31 -04:00
parent bc085205c8
commit 928c4de309
6 changed files with 48 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
src/backend/catalog/heap.c
View File

@ -52,7 +52,6 @@
#include "catalog/pg_opclass.h"
#include "catalog/pg_partitioned_table.h"
#include "catalog/pg_statistic.h"
#include "catalog/pg_statistic_ext.h"
#include "catalog/pg_subscription_rel.h"
#include "catalog/pg_tablespace.h"
#include "catalog/pg_type.h"
@ -1615,10 +1614,7 @@ RemoveAttributeById(Oid relid, AttrNumber attnum)
heap_close(attr_rel, RowExclusiveLock);
if (attnum > 0)
{
RemoveStatistics(relid, attnum);
RemoveStatisticsExt(relid, attnum);
}
relation_close(rel, NoLock);
}
@ -1873,7 +1869,6 @@ heap_drop_with_catalog(Oid relid)
* delete statistics
*/
RemoveStatistics(relid, 0);
RemoveStatisticsExt(relid, 0);
/*
* delete attribute tuples
@ -2785,75 +2780,6 @@ RemoveStatistics(Oid relid, AttrNumber attnum)
}
/*
* RemoveStatisticsExt --- remove entries in pg_statistic_ext for a relation
*
* If attnum is zero, remove all entries for rel; else remove only the
* one(s) involving that column.
*/
void
RemoveStatisticsExt(Oid relid, AttrNumber attnum)
{
Relation pgstatisticext;
SysScanDesc scan;
ScanKeyData key;
HeapTuple tuple;
/*
* Scan pg_statistic_ext to delete relevant tuples
*/
pgstatisticext = heap_open(StatisticExtRelationId, RowExclusiveLock);
ScanKeyInit(&key,
Anum_pg_statistic_ext_stxrelid,
BTEqualStrategyNumber, F_OIDEQ,
ObjectIdGetDatum(relid));
scan = systable_beginscan(pgstatisticext,
StatisticExtRelidIndexId,
true, NULL, 1, &key);
while (HeapTupleIsValid(tuple = systable_getnext(scan)))
{
bool delete = false;
if (attnum == 0)
delete = true;
else if (attnum != 0)
{
Form_pg_statistic_ext staForm;
int i;
/*
* Decode the stxkeys array and delete any stats that involve the
* specified column.
*/
staForm = (Form_pg_statistic_ext) GETSTRUCT(tuple);
for (i = 0; i < staForm->stxkeys.dim1; i++)
{
if (staForm->stxkeys.values[i] == attnum)
{
delete = true;
break;
}
}
}
if (delete)
{
CatalogTupleDelete(pgstatisticext, &tuple->t_self);
deleteDependencyRecordsFor(StatisticExtRelationId,
HeapTupleGetOid(tuple),
false);
}
}
systable_endscan(scan);
heap_close(pgstatisticext, RowExclusiveLock);
}
/*
* RelationTruncateIndexes - truncate all indexes associated
* with the heap relation to zero tuples.

52
src/backend/commands/statscmds.c
View File

@ -50,11 +50,11 @@ CreateStatistics(CreateStatsStmt *stmt)
{
int16 attnums[STATS_MAX_DIMENSIONS];
int numcols = 0;
ObjectAddress address = InvalidObjectAddress;
char *namestr;
NameData stxname;
Oid statoid;
Oid namespaceId;
Oid stxowner = GetUserId();
HeapTuple htup;
Datum values[Natts_pg_statistic_ext];
bool nulls[Natts_pg_statistic_ext];
@ -63,7 +63,7 @@ CreateStatistics(CreateStatsStmt *stmt)
Relation rel = NULL;
Oid relid;
ObjectAddress parentobject,
childobject;
myself;
Datum types[2]; /* one for each possible type of statistics */
int ntypes;
ArrayType *stxkind;
@ -140,7 +140,7 @@ CreateStatistics(CreateStatsStmt *stmt)
RelationGetRelationName(rel))));
/* You must own the relation to create stats on it */
if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
if (!pg_class_ownercheck(RelationGetRelid(rel), stxowner))
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
RelationGetRelationName(rel));
}
@ -185,7 +185,7 @@ CreateStatistics(CreateStatsStmt *stmt)
attForm = (Form_pg_attribute) GETSTRUCT(atttuple);
/* Disallow use of system attributes in extended stats */
if (attForm->attnum < 0)
if (attForm->attnum <= 0)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("statistics creation on system columns is not supported")));
@ -205,7 +205,7 @@ CreateStatistics(CreateStatsStmt *stmt)
errmsg("cannot have more than %d columns in statistics",
STATS_MAX_DIMENSIONS)));
attnums[numcols] = ((Form_pg_attribute) GETSTRUCT(atttuple))->attnum;
attnums[numcols] = attForm->attnum;
numcols++;
ReleaseSysCache(atttuple);
}
@ -231,10 +231,12 @@ CreateStatistics(CreateStatsStmt *stmt)
* just check consecutive elements.
*/
for (i = 1; i < numcols; i++)
{
if (attnums[i] == attnums[i - 1])
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_COLUMN),
errmsg("duplicate column name in statistics definition")));
}
/* Form an int2vector representation of the sorted column list */
stxkeys = buildint2vector(attnums, numcols);
@ -288,7 +290,7 @@ CreateStatistics(CreateStatsStmt *stmt)
values[Anum_pg_statistic_ext_stxrelid - 1] = ObjectIdGetDatum(relid);
values[Anum_pg_statistic_ext_stxname - 1] = NameGetDatum(&stxname);
values[Anum_pg_statistic_ext_stxnamespace - 1] = ObjectIdGetDatum(namespaceId);
values[Anum_pg_statistic_ext_stxowner - 1] = ObjectIdGetDatum(GetUserId());
values[Anum_pg_statistic_ext_stxowner - 1] = ObjectIdGetDatum(stxowner);
values[Anum_pg_statistic_ext_stxkeys - 1] = PointerGetDatum(stxkeys);
values[Anum_pg_statistic_ext_stxkind - 1] = PointerGetDatum(stxkind);
@ -312,29 +314,35 @@ CreateStatistics(CreateStatsStmt *stmt)
relation_close(rel, NoLock);
/*
* Add a dependency on the table, so that stats get dropped on DROP TABLE.
*
* XXX don't we need dependencies on the specific columns, instead?
* Add an AUTO dependency on each column used in the stats, so that the
* stats object goes away if any or all of them get dropped.
*/
ObjectAddressSet(parentobject, RelationRelationId, relid);
ObjectAddressSet(childobject, StatisticExtRelationId, statoid);
recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
ObjectAddressSet(myself, StatisticExtRelationId, statoid);
for (i = 0; i < numcols; i++)
{
ObjectAddressSubSet(parentobject, RelationRelationId, relid, attnums[i]);
recordDependencyOn(&myself, &parentobject, DEPENDENCY_AUTO);
}
/*
* Also add dependency on the schema. This is required to ensure that we
* drop the statistics on DROP SCHEMA. This is not handled automatically
* by DROP TABLE because the statistics might be in a different schema
* from the table itself. (This definition is a bit bizarre for the
* single-table case, but it will make more sense if/when we support
* extended stats across multiple tables.)
* Also add dependencies on namespace and owner. These are required
* because the stats object might have a different namespace and/or owner
* than the underlying table(s).
*/
ObjectAddressSet(parentobject, NamespaceRelationId, namespaceId);
recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
recordDependencyOn(&myself, &parentobject, DEPENDENCY_NORMAL);
recordDependencyOnOwner(StatisticExtRelationId, statoid, stxowner);
/*
* XXX probably there should be a recordDependencyOnCurrentExtension call
* here too, but we'd have to add support for ALTER EXTENSION ADD/DROP
* STATISTICS, which is more work than it seems worth.
*/
/* Return stats object's address */
ObjectAddressSet(address, StatisticExtRelationId, statoid);
return address;
return myself;
}
/*

2
src/include/catalog/catversion.h
View File

@ -53,6 +53,6 @@
*/
/* yyyymmddN */
#define CATALOG_VERSION_NO 201705111
#define CATALOG_VERSION_NO 201705121
#endif

1
src/include/catalog/heap.h
View File

@ -119,7 +119,6 @@ extern void RemoveAttrDefault(Oid relid, AttrNumber attnum,
DropBehavior behavior, bool complain, bool internal);
extern void RemoveAttrDefaultById(Oid attrdefId);
extern void RemoveStatistics(Oid relid, AttrNumber attnum);
extern void RemoveStatisticsExt(Oid relid, AttrNumber attnum);
extern Form_pg_attribute SystemAttributeDefinition(AttrNumber attno,
bool relhasoids);

14
src/test/regress/expected/stats_ext.out
View File

@ -47,7 +47,7 @@ DROP STATISTICS regress_schema_2.ab1_a_b_stats;
-- Ensure statistics are dropped when columns are
CREATE STATISTICS ab1_b_c_stats ON b, c FROM ab1;
CREATE STATISTICS ab1_a_b_c_stats ON a, b, c FROM ab1;
CREATE STATISTICS ab1_a_b_stats ON a, b FROM ab1;
CREATE STATISTICS ab1_b_a_stats ON b, a FROM ab1;
ALTER TABLE ab1 DROP COLUMN a;
\d ab1
Table "public.ab1"
@ -58,7 +58,19 @@ ALTER TABLE ab1 DROP COLUMN a;
Statistics:
"public"."ab1_b_c_stats" (ndistinct, dependencies) ON b, c FROM ab1
-- Ensure statistics are dropped when table is
SELECT stxname FROM pg_statistic_ext WHERE stxname LIKE 'ab1%';
stxname
---------------
ab1_b_c_stats
(1 row)
DROP TABLE ab1;
SELECT stxname FROM pg_statistic_ext WHERE stxname LIKE 'ab1%';
stxname
---------
(0 rows)
-- Ensure things work sanely with SET STATISTICS 0
CREATE TABLE ab1 (a INTEGER, b INTEGER);
ALTER TABLE ab1 ALTER a SET STATISTICS 0;

5
src/test/regress/sql/stats_ext.sql
View File

@ -34,10 +34,13 @@ DROP STATISTICS regress_schema_2.ab1_a_b_stats;
-- Ensure statistics are dropped when columns are
CREATE STATISTICS ab1_b_c_stats ON b, c FROM ab1;
CREATE STATISTICS ab1_a_b_c_stats ON a, b, c FROM ab1;
CREATE STATISTICS ab1_a_b_stats ON a, b FROM ab1;
CREATE STATISTICS ab1_b_a_stats ON b, a FROM ab1;
ALTER TABLE ab1 DROP COLUMN a;
\d ab1
-- Ensure statistics are dropped when table is
SELECT stxname FROM pg_statistic_ext WHERE stxname LIKE 'ab1%';
DROP TABLE ab1;
SELECT stxname FROM pg_statistic_ext WHERE stxname LIKE 'ab1%';
-- Ensure things work sanely with SET STATISTICS 0
CREATE TABLE ab1 (a INTEGER, b INTEGER);