Multivariate MCV list vs. statistics target
Hi,
One slightly inconvenient thing I realized while playing with the
address data set is that it's somewhat difficult to set the desired size
of the multi-column MCV list.
At the moment, we simply use the maximum statistic target for attributes
the MCV list is built on. But that does not allow keeping default size
for per-column stats, and only increase size of multi-column MCV lists.
So I'm thinking we should allow tweaking the statistics for extended
stats, and serialize it in the pg_statistic_ext catalog. Any opinions
why that would be a bad idea?
I suppose it should be part of the CREATE STATISTICS command, but I'm
not sure what'd be the best syntax. We might also have something more
similar to ALTER COLUMNT, but perhaps
ALTER STATISTICS s SET STATISTICS 1000;
looks a bit too weird.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, 18 Jun 2019 at 22:34, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
One slightly inconvenient thing I realized while playing with the
address data set is that it's somewhat difficult to set the desired size
of the multi-column MCV list.At the moment, we simply use the maximum statistic target for attributes
the MCV list is built on. But that does not allow keeping default size
for per-column stats, and only increase size of multi-column MCV lists.So I'm thinking we should allow tweaking the statistics for extended
stats, and serialize it in the pg_statistic_ext catalog. Any opinions
why that would be a bad idea?
Seems reasonable to me. This might not be the only option we'll ever
want to add though, so perhaps a "stxoptions text[]" column along the
lines of a relation's reloptions would be the way to go.
I suppose it should be part of the CREATE STATISTICS command, but I'm
not sure what'd be the best syntax. We might also have something more
similar to ALTER COLUMNT, but perhapsALTER STATISTICS s SET STATISTICS 1000;
looks a bit too weird.
Yes it does look a bit weird, but that's the natural generalisation of
what we have for per-column statistics, so it's probably preferable to
do that rather than invent some other syntax that wouldn't be so
consistent.
Regards,
Dean
On Thu, Jun 20, 2019 at 08:08:44AM +0100, Dean Rasheed wrote:
On Tue, 18 Jun 2019 at 22:34, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
One slightly inconvenient thing I realized while playing with the
address data set is that it's somewhat difficult to set the desired size
of the multi-column MCV list.At the moment, we simply use the maximum statistic target for attributes
the MCV list is built on. But that does not allow keeping default size
for per-column stats, and only increase size of multi-column MCV lists.So I'm thinking we should allow tweaking the statistics for extended
stats, and serialize it in the pg_statistic_ext catalog. Any opinions
why that would be a bad idea?Seems reasonable to me. This might not be the only option we'll ever
want to add though, so perhaps a "stxoptions text[]" column along the
lines of a relation's reloptions would be the way to go.
I don't know - I kinda dislike the idea of stashing stuff like this into
text[] arrays unless there's a clear need for such flexibility (i.e.
vision to have more such options). Which I'm not sure is the case here.
And we kinda have a precedent in pg_attribute.attstattarget, so I'd use
the same approach here.
I suppose it should be part of the CREATE STATISTICS command, but I'm
not sure what'd be the best syntax. We might also have something more
similar to ALTER COLUMNT, but perhapsALTER STATISTICS s SET STATISTICS 1000;
looks a bit too weird.
Yes it does look a bit weird, but that's the natural generalisation of
what we have for per-column statistics, so it's probably preferable to
do that rather than invent some other syntax that wouldn't be so
consistent.
Yeah, I agree.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, 20 Jun 2019 at 23:12, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
On Thu, Jun 20, 2019 at 08:08:44AM +0100, Dean Rasheed wrote:
On Tue, 18 Jun 2019 at 22:34, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
So I'm thinking we should allow tweaking the statistics for extended
stats, and serialize it in the pg_statistic_ext catalog. Any opinions
why that would be a bad idea?Seems reasonable to me. This might not be the only option we'll ever
want to add though, so perhaps a "stxoptions text[]" column along the
lines of a relation's reloptions would be the way to go.I don't know - I kinda dislike the idea of stashing stuff like this into
text[] arrays unless there's a clear need for such flexibility (i.e.
vision to have more such options). Which I'm not sure is the case here.
And we kinda have a precedent in pg_attribute.attstattarget, so I'd use
the same approach here.
Hmm, maybe. I can certainly understand your dislike of using text[].
I'm not sure that we can confidently say that multivariate statistics
won't ever need additional tuning knobs, but I have no idea at the
moment what they might be, and nothing else has come up so far in all
the time spent considering MCV lists and histograms, so maybe this is
OK.
Regards,
Dean
On Fri, Jun 21, 2019 at 08:09:18AM +0100, Dean Rasheed wrote:
On Thu, 20 Jun 2019 at 23:12, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
On Thu, Jun 20, 2019 at 08:08:44AM +0100, Dean Rasheed wrote:
On Tue, 18 Jun 2019 at 22:34, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
So I'm thinking we should allow tweaking the statistics for extended
stats, and serialize it in the pg_statistic_ext catalog. Any opinions
why that would be a bad idea?Seems reasonable to me. This might not be the only option we'll ever
want to add though, so perhaps a "stxoptions text[]" column along the
lines of a relation's reloptions would be the way to go.I don't know - I kinda dislike the idea of stashing stuff like this into
text[] arrays unless there's a clear need for such flexibility (i.e.
vision to have more such options). Which I'm not sure is the case here.
And we kinda have a precedent in pg_attribute.attstattarget, so I'd use
the same approach here.Hmm, maybe. I can certainly understand your dislike of using text[].
I'm not sure that we can confidently say that multivariate statistics
won't ever need additional tuning knobs, but I have no idea at the
moment what they might be, and nothing else has come up so far in all
the time spent considering MCV lists and histograms, so maybe this is
OK.
OK, attached is a patch implementing this - it adds
ALTER STATISTICS ... SET STATISTICS ...
modifying a new stxstattarget column in pg_statistic_ext catalog,
following the same logic as pg_attribute.attstattarget.
During analyze, the per-ext-statistic value is determined like this:
1) When pg_statistic_ext.stxstattarget != (-1), we just use this value
and we're done.
2) Otherwise we inspect per-column attstattarget values, and use the
largest value. This is what we do now, so it's backwards-compatible
behavior.
3) If the value is still (-1), we use default_statistics_target.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
alter-statistics-v1.patchtext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/alter_statistics.sgml b/doc/src/sgml/ref/alter_statistics.sgml
index 58c7ed020d..987f9dbc6f 100644
--- a/doc/src/sgml/ref/alter_statistics.sgml
+++ b/doc/src/sgml/ref/alter_statistics.sgml
@@ -26,6 +26,7 @@ PostgreSQL documentation
ALTER STATISTICS <replaceable class="parameter">name</replaceable> OWNER TO { <replaceable class="parameter">new_owner</replaceable> | CURRENT_USER | SESSION_USER }
ALTER STATISTICS <replaceable class="parameter">name</replaceable> RENAME TO <replaceable class="parameter">new_name</replaceable>
ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET SCHEMA <replaceable class="parameter">new_schema</replaceable>
+ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET STATISTICS <replaceable class="parameter">new_target</replaceable>
</synopsis>
</refsynopsisdiv>
@@ -93,6 +94,22 @@ ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET SCHEMA <r
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">new_target</replaceable></term>
+ <listitem>
+ <para>
+ The statistic-gathering target for this statistic object for subsequent
+ <xref linkend="sql-analyze"/> operations.
+ The target can be set in the range 0 to 10000; alternatively, set it
+ to -1 to revert to using the system default statistics
+ target (<xref linkend="guc-default-statistics-target"/>).
+ For more information on the use of statistics by the
+ <productname>PostgreSQL</productname> query planner, refer to
+ <xref linkend="planner-stats"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
</refsect1>
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index d7004e5313..caa4fca99d 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -490,6 +490,19 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
}
}
+ /*
+ * Look at extended statistic objects too, because those may define their
+ * own statistic target. So we need to sample enough rows and then build
+ * the statistics with enough detail.
+ */
+ {
+ int nrows = ComputeExtStatisticsTarget(onerel,
+ attr_cnt, vacattrstats);
+
+ if (targrows < nrows)
+ targrows = nrows;
+ }
+
/*
* Acquire the sample rows
*/
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index cf406f6f96..356b6096ac 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -14,6 +14,7 @@
*/
#include "postgres.h"
+#include "access/heapam.h"
#include "access/relation.h"
#include "access/relscan.h"
#include "access/table.h"
@@ -21,6 +22,7 @@
#include "catalog/dependency.h"
#include "catalog/indexing.h"
#include "catalog/namespace.h"
+#include "catalog/objectaccess.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_statistic_ext.h"
#include "catalog/pg_statistic_ext_data.h"
@@ -29,6 +31,7 @@
#include "miscadmin.h"
#include "statistics/statistics.h"
#include "utils/builtins.h"
+#include "utils/fmgroids.h"
#include "utils/inval.h"
#include "utils/memutils.h"
#include "utils/rel.h"
@@ -336,6 +339,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_stxstattarget - 1] = Int32GetDatum(-1);
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);
@@ -414,6 +418,85 @@ CreateStatistics(CreateStatsStmt *stmt)
return myself;
}
+
+/*
+ * ALTER STATISTICS
+ */
+ObjectAddress
+AlterStatistics(AlterStatsStmt *stmt)
+{
+ Relation rel;
+ Oid stxoid;
+ HeapTuple oldtup;
+ HeapTuple newtup;
+ Datum repl_val[Natts_pg_statistic_ext];
+ bool repl_null[Natts_pg_statistic_ext];
+ bool repl_repl[Natts_pg_statistic_ext];
+ ObjectAddress address;
+ int newtarget = stmt->stxstattarget;
+
+ /* Limit target to a sane range */
+ if (newtarget < -1)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("statistics target %d is too low",
+ newtarget)));
+ }
+ else if (newtarget > 10000)
+ {
+ newtarget = 10000;
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("lowering statistics target to %d",
+ newtarget)));
+ }
+
+ /* lookup OID of the statistic object */
+ stxoid = get_statistics_object_oid(stmt->defnames, false);
+
+ /* Search pg_statistic_ext */
+ rel = table_open(StatisticExtRelationId, RowExclusiveLock);
+
+ oldtup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(stxoid));
+
+ /* Must be owner of the existing statistic object */
+ if (!pg_statistics_object_ownercheck(stxoid, GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_STATISTIC_EXT,
+ NameListToString(stmt->defnames));
+
+ /* Build new tuple. */
+ memset(repl_val, 0, sizeof(repl_val));
+ memset(repl_null, false, sizeof(repl_null));
+ memset(repl_repl, false, sizeof(repl_repl));
+
+ /* replace the stxstattarget column */
+ repl_repl[Anum_pg_statistic_ext_stxstattarget - 1] = true;
+ repl_val[Anum_pg_statistic_ext_stxstattarget - 1] = Int32GetDatum(newtarget);
+
+ newtup = heap_modify_tuple(oldtup, RelationGetDescr(rel),
+ repl_val, repl_null, repl_repl);
+
+ /* Update system catalog. */
+ CatalogTupleUpdate(rel, &newtup->t_self, newtup);
+
+ InvokeObjectPostAlterHook(StatisticExtRelationId, stxoid, 0);
+
+ ObjectAddressSet(address, StatisticExtRelationId, stxoid);
+
+ /*
+ * NOTE: because we only support altering the statistic target, not the
+ * other fields, there is no need to update dependencies.
+ */
+
+ heap_freetuple(newtup);
+ ReleaseSysCache(oldtup);
+
+ table_close(rel, RowExclusiveLock);
+
+ return address;
+}
+
/*
* Guts of statistics object deletion.
*/
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 78deade89b..048239fe34 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3493,6 +3493,18 @@ _copyCreateStatsStmt(const CreateStatsStmt *from)
return newnode;
}
+static AlterStatsStmt *
+_copyAlterStatsStmt(const AlterStatsStmt *from)
+{
+ AlterStatsStmt *newnode = makeNode(AlterStatsStmt);
+
+ COPY_NODE_FIELD(defnames);
+ COPY_SCALAR_FIELD(stxstattarget);
+ COPY_SCALAR_FIELD(if_not_exists);
+
+ return newnode;
+}
+
static CreateFunctionStmt *
_copyCreateFunctionStmt(const CreateFunctionStmt *from)
{
@@ -5249,6 +5261,9 @@ copyObjectImpl(const void *from)
case T_CreateStatsStmt:
retval = _copyCreateStatsStmt(from);
break;
+ case T_AlterStatsStmt:
+ retval = _copyAlterStatsStmt(from);
+ break;
case T_CreateFunctionStmt:
retval = _copyCreateFunctionStmt(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 4f2ebe5118..4c35cdf41e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1365,6 +1365,16 @@ _equalCreateStatsStmt(const CreateStatsStmt *a, const CreateStatsStmt *b)
return true;
}
+static bool
+_equalAlterStatsStmt(const AlterStatsStmt *a, const AlterStatsStmt *b)
+{
+ COMPARE_NODE_FIELD(defnames);
+ COMPARE_SCALAR_FIELD(stxstattarget);
+ COMPARE_SCALAR_FIELD(if_not_exists);
+
+ return true;
+}
+
static bool
_equalCreateFunctionStmt(const CreateFunctionStmt *a, const CreateFunctionStmt *b)
{
@@ -3309,6 +3319,9 @@ equal(const void *a, const void *b)
case T_CreateStatsStmt:
retval = _equalCreateStatsStmt(a, b);
break;
+ case T_AlterStatsStmt:
+ retval = _equalAlterStatsStmt(a, b);
+ break;
case T_CreateFunctionStmt:
retval = _equalCreateFunctionStmt(a, b);
break;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 8400dd319e..4551d7a023 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2662,6 +2662,16 @@ _outCreateStatsStmt(StringInfo str, const CreateStatsStmt *node)
WRITE_BOOL_FIELD(if_not_exists);
}
+static void
+_outAlterStatsStmt(StringInfo str, const AlterStatsStmt *node)
+{
+ WRITE_NODE_TYPE("ALTERSTATSSTMT");
+
+ WRITE_NODE_FIELD(defnames);
+ WRITE_INT_FIELD(stxstattarget);
+ WRITE_BOOL_FIELD(if_not_exists);
+}
+
static void
_outNotifyStmt(StringInfo str, const NotifyStmt *node)
{
@@ -4124,6 +4134,9 @@ outNode(StringInfo str, const void *obj)
case T_CreateStatsStmt:
_outCreateStatsStmt(str, obj);
break;
+ case T_AlterStatsStmt:
+ _outAlterStatsStmt(str, obj);
+ break;
case T_NotifyStmt:
_outNotifyStmt(str, obj);
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 8311b1dd46..d9fe653511 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -252,7 +252,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
AlterOperatorStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
AlterCompositeTypeStmt AlterUserMappingStmt
- AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt
+ AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt AlterStatsStmt
AlterDefaultPrivilegesStmt DefACLAction
AnalyzeStmt CallStmt ClosePortalStmt ClusterStmt CommentStmt
ConstraintsSetStmt CopyStmt CreateAsStmt CreateCastStmt
@@ -852,6 +852,7 @@ stmt :
| AlterRoleSetStmt
| AlterRoleStmt
| AlterSubscriptionStmt
+ | AlterStatsStmt
| AlterTSConfigurationStmt
| AlterTSDictionaryStmt
| AlterUserMappingStmt
@@ -3984,6 +3985,34 @@ CreateStatsStmt:
}
;
+
+/*****************************************************************************
+ *
+ * QUERY :
+ * ALTER STATISTICS [IF EXISTS] stats_name
+ * SET STATISTICS <SignedIconst>
+ *
+ *****************************************************************************/
+
+AlterStatsStmt:
+ ALTER STATISTICS any_name SET STATISTICS SignedIconst
+ {
+ AlterStatsStmt *n = makeNode(AlterStatsStmt);
+ n->defnames = $3;
+ n->if_not_exists = false;
+ n->stxstattarget = $6;
+ $$ = (Node *)n;
+ }
+ | ALTER STATISTICS IF_P EXISTS any_name SET STATISTICS SignedIconst
+ {
+ AlterStatsStmt *n = makeNode(AlterStatsStmt);
+ n->defnames = $5;
+ n->if_not_exists = true;
+ n->stxstattarget = $8;
+ $$ = (Node *)n;
+ }
+ ;
+
/*****************************************************************************
*
* QUERY :
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index c56ed48270..2ab0690c30 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -61,6 +61,7 @@ typedef struct StatExtEntry
char *name; /* statistics object's name */
Bitmapset *columns; /* attribute numbers covered by the object */
List *types; /* 'char' list of enabled statistic kinds */
+ int stattarget; /* statistic target (-1 for default) */
} StatExtEntry;
@@ -70,7 +71,8 @@ static VacAttrStats **lookup_var_attr_stats(Relation rel, Bitmapset *attrs,
static void statext_store(Oid relid,
MVNDistinct *ndistinct, MVDependencies *dependencies,
MCVList *mcv, VacAttrStats **stats);
-
+static int statext_compute_stattarget(int stattarget,
+ int natts, VacAttrStats **stats);
/*
* Compute requested extended stats, using the rows sampled for the plain
@@ -106,6 +108,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
MCVList *mcv = NULL;
VacAttrStats **stats;
ListCell *lc2;
+ int stattarget;
/*
* Check if we can build these stats based on the column analyzed. If
@@ -130,6 +133,12 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
Assert(bms_num_members(stat->columns) >= 2 &&
bms_num_members(stat->columns) <= STATS_MAX_DIMENSIONS);
+ /* compute current statistic target */
+ stattarget = statext_compute_stattarget(stat->stattarget,
+ natts, stats);
+
+ /* XXX What if the target is set to 0? Reset the statistic? */
+
/* compute statistic of each requested type */
foreach(lc2, stat->types)
{
@@ -143,7 +152,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
stat->columns, stats);
else if (t == STATS_EXT_MCV)
mcv = statext_mcv_build(numrows, rows, stat->columns, stats,
- totalrows);
+ totalrows, stattarget);
}
/* store the statistics in the catalog */
@@ -156,6 +165,116 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
MemoryContextDelete(cxt);
}
+/*
+ * ComputeExtStatisticsTarget
+ * Compute the largest statistic target for all extended statistics.
+ */
+int
+ComputeExtStatisticsTarget(Relation onerel,
+ int natts, VacAttrStats **vacattrstats)
+{
+ Relation pg_stext;
+ ListCell *lc;
+ List *lstats;
+ MemoryContext cxt;
+ MemoryContext oldcxt;
+ int result = 0;
+
+ cxt = AllocSetContextCreate(CurrentMemoryContext,
+ "ComputeExtStatisticsTarget",
+ ALLOCSET_DEFAULT_SIZES);
+ oldcxt = MemoryContextSwitchTo(cxt);
+
+ pg_stext = table_open(StatisticExtRelationId, RowExclusiveLock);
+ lstats = fetch_statentries_for_relation(pg_stext, RelationGetRelid(onerel));
+
+ foreach(lc, lstats)
+ {
+ StatExtEntry *stat = (StatExtEntry *) lfirst(lc);
+ int stattarget = stat->stattarget;
+ VacAttrStats **stats;
+ int nattrs = bms_num_members(stat->columns);
+
+ /*
+ * Check if we can build these stats based on the column analyzed. If
+ * not, report this fact (except in autovacuum) and move on.
+ */
+ stats = lookup_var_attr_stats(onerel, stat->columns,
+ natts, vacattrstats);
+ if (!stats)
+ {
+ if (!IsAutoVacuumWorkerProcess())
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("statistics object \"%s.%s\" could not be computed for relation \"%s.%s\"",
+ stat->schema, stat->name,
+ get_namespace_name(onerel->rd_rel->relnamespace),
+ RelationGetRelationName(onerel)),
+ errtable(onerel)));
+ continue;
+ }
+
+ /* compute statistic target, based on the statistic object and attributes */
+ stattarget = statext_compute_stattarget(stat->stattarget, nattrs, stats);
+
+ /* Use the largest value for all statistic objects. */
+ if (stattarget > result)
+ result = stattarget;
+ }
+
+ table_close(pg_stext, RowExclusiveLock);
+
+ MemoryContextSwitchTo(oldcxt);
+ MemoryContextDelete(cxt);
+
+ return (300 * result);
+}
+
+/*
+ * statext_compute_stattarget
+ * compute statistic target for an extended statistic
+ *
+ * If the statistic object has custom value (set using ALTER STATISTICS ...
+ * SET STATISTICS), then we simply use that. Otherwise we look at targets
+ * for columns the object is defined on and use the largest value. Finally,
+ * if the target is still (-1), we use the default_statistics_target.
+ */
+static int
+statext_compute_stattarget(int stattarget, int nattrs, VacAttrStats **stats)
+{
+ int i;
+
+ /* if there's statistic target set for the statistic object, use it */
+ if (stattarget >= 0)
+ return stattarget;
+
+ /*
+ * The stattarget column is negative, we look at targets for each
+ * column the statistics is based on, and use use the largest value
+ * for any of the columns.
+ */
+ {
+ for (i = 0; i < nattrs; i++)
+ {
+ if (stats[i]->attr->attstattarget > stattarget)
+ stattarget = stats[i]->attr->attstattarget;
+ }
+ }
+
+ /*
+ * If the value is still negative (so neither the extended statistic
+ * object nor any of the columns) have custom statistic target set.
+ * In that case use the default target.
+ */
+ if (stattarget < 0)
+ stattarget = default_statistics_target;
+
+ /* As this point we should have a valid statistic target. */
+ Assert((stattarget >= 0) && (stattarget <= 10000));
+
+ return stattarget;
+}
+
/*
* statext_is_kind_built
* Is this stat kind built in the given pg_statistic_ext_data tuple?
@@ -224,6 +343,7 @@ fetch_statentries_for_relation(Relation pg_statext, Oid relid)
entry->statOid = staForm->oid;
entry->schema = get_namespace_name(staForm->stxnamespace);
entry->name = pstrdup(NameStr(staForm->stxname));
+ entry->stattarget = staForm->stxstattarget;
for (i = 0; i < staForm->stxkeys.dim1; i++)
{
entry->columns = bms_add_member(entry->columns,
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index 5fe61ea0a4..54a8476891 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -160,7 +160,8 @@ get_mincount_for_mcv_list(int samplerows, double totalrows)
*/
MCVList *
statext_mcv_build(int numrows, HeapTuple *rows, Bitmapset *attrs,
- VacAttrStats **stats, double totalrows)
+ VacAttrStats **stats, double totalrows,
+ int stattarget)
{
int i,
numattrs,
@@ -192,12 +193,7 @@ statext_mcv_build(int numrows, HeapTuple *rows, Bitmapset *attrs,
* Maximum number of MCV items to store, based on the attribute with the
* largest stats target (and the number of groups we have available).
*/
- nitems = stats[0]->attr->attstattarget;
- for (i = 1; i < numattrs; i++)
- {
- if (stats[i]->attr->attstattarget > nitems)
- nitems = stats[i]->attr->attstattarget;
- }
+ nitems = stattarget;
if (nitems > ngroups)
nitems = ngroups;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 05ec7f3ac6..1299066c63 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1688,6 +1688,10 @@ ProcessUtilitySlow(ParseState *pstate,
address = CreateStatistics((CreateStatsStmt *) parsetree);
break;
+ case T_AlterStatsStmt:
+ address = AlterStatistics((AlterStatsStmt *) parsetree);
+ break;
+
case T_AlterCollationStmt:
address = AlterCollation((AlterCollationStmt *) parsetree);
break;
@@ -2805,6 +2809,10 @@ CreateCommandTag(Node *parsetree)
tag = "CREATE STATISTICS";
break;
+ case T_AlterStatsStmt:
+ tag = "ALTER STATISTICS";
+ break;
+
case T_DeallocateStmt:
{
DeallocateStmt *stmt = (DeallocateStmt *) parsetree;
@@ -3393,6 +3401,10 @@ GetCommandLogLevel(Node *parsetree)
lev = LOGSTMT_DDL;
break;
+ case T_AlterStatsStmt:
+ lev = LOGSTMT_DDL;
+ break;
+
case T_AlterCollationStmt:
lev = LOGSTMT_DDL;
break;
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 7dcf342413..e0a4e56b30 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1837,7 +1837,7 @@ psql_completion(const char *text, int start, int end)
/* ALTER STATISTICS <name> */
else if (Matches("ALTER", "STATISTICS", MatchAny))
- COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA");
+ COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA", "SET STATISTICS");
/* ALTER TRIGGER <name>, add ON */
else if (Matches("ALTER", "TRIGGER", MatchAny))
diff --git a/src/include/catalog/pg_statistic_ext.h b/src/include/catalog/pg_statistic_ext.h
index d8c5e0651e..5f6bfdd56c 100644
--- a/src/include/catalog/pg_statistic_ext.h
+++ b/src/include/catalog/pg_statistic_ext.h
@@ -41,6 +41,7 @@ CATALOG(pg_statistic_ext,3381,StatisticExtRelationId)
Oid stxnamespace; /* OID of statistics object's namespace */
Oid stxowner; /* statistics object's owner */
+ int32 stxstattarget BKI_DEFAULT(-1); /* statistic target */
/*
* variable-length fields start here, but we allow direct access to
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index b4e7db67c3..1dc6dc2ca0 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -87,6 +87,7 @@ extern ObjectAddress AlterOperator(AlterOperatorStmt *stmt);
/* commands/statscmds.c */
extern ObjectAddress CreateStatistics(CreateStatsStmt *stmt);
+extern ObjectAddress AlterStatistics(AlterStatsStmt *stmt);
extern void RemoveStatisticsById(Oid statsOid);
extern void UpdateStatisticsForTypeChange(Oid statsOid,
Oid relationOid, int attnum,
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 4e2fb39105..57589d4fac 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -420,6 +420,7 @@ typedef enum NodeTag
T_CreateStatsStmt,
T_AlterCollationStmt,
T_CallStmt,
+ T_AlterStatsStmt,
/*
* TAGS FOR PARSE TREE NODES (parsenodes.h)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 94ded3c135..a6a3382a46 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2789,6 +2789,18 @@ typedef struct CreateStatsStmt
bool if_not_exists; /* do nothing if stats name already exists */
} CreateStatsStmt;
+/* ----------------------
+ * Alter Statistics Statement
+ * ----------------------
+ */
+typedef struct AlterStatsStmt
+{
+ NodeTag type;
+ List *defnames; /* qualified name (list of Value strings) */
+ int stxstattarget; /* statistics target, or NULL */
+ bool if_not_exists; /* do nothing if stats name already exists */
+} AlterStatsStmt;
+
/* ----------------------
* Create Function Statement
* ----------------------
diff --git a/src/include/statistics/extended_stats_internal.h b/src/include/statistics/extended_stats_internal.h
index fb03c52f50..c8b7deffbf 100644
--- a/src/include/statistics/extended_stats_internal.h
+++ b/src/include/statistics/extended_stats_internal.h
@@ -70,7 +70,7 @@ extern MVDependencies *statext_dependencies_deserialize(bytea *data);
extern MCVList *statext_mcv_build(int numrows, HeapTuple *rows,
Bitmapset *attrs, VacAttrStats **stats,
- double totalrows);
+ double totalrows, int stattarget);
extern bytea *statext_mcv_serialize(MCVList *mcv, VacAttrStats **stats);
extern MCVList *statext_mcv_deserialize(bytea *data);
diff --git a/src/include/statistics/statistics.h b/src/include/statistics/statistics.h
index cb7bc630e9..53ec906936 100644
--- a/src/include/statistics/statistics.h
+++ b/src/include/statistics/statistics.h
@@ -100,6 +100,8 @@ extern MCVList *statext_mcv_load(Oid mvoid);
extern void BuildRelationExtStatistics(Relation onerel, double totalrows,
int numrows, HeapTuple *rows,
int natts, VacAttrStats **vacattrstats);
+extern int ComputeExtStatisticsTarget(Relation onerel,
+ int natts, VacAttrStats **stats);
extern bool statext_is_kind_built(HeapTuple htup, char kind);
extern Selectivity dependencies_clauselist_selectivity(PlannerInfo *root,
List *clauses,
On Fri, 21 Jun 2019 at 19:09, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
Hmm, maybe. I can certainly understand your dislike of using text[].
I'm not sure that we can confidently say that multivariate statistics
won't ever need additional tuning knobs, but I have no idea at the
moment what they might be, and nothing else has come up so far in all
the time spent considering MCV lists and histograms, so maybe this is
OK.
I agree with having the stxstattarget column. Even if something did
come up in the future, then we could consider merging the
stxstattarget column with a new text[] column at that time.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Hi,
apparently v1 of the ALTER STATISTICS patch was a bit confused about
length of the VacAttrStats array passed to statext_compute_stattarget,
causing segfaults. Attached v2 patch fixes that, and it also makes sure
we print warnings about ignored statistics only once.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
alter-statistics-v2.patchtext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/alter_statistics.sgml b/doc/src/sgml/ref/alter_statistics.sgml
index 58c7ed020d..987f9dbc6f 100644
--- a/doc/src/sgml/ref/alter_statistics.sgml
+++ b/doc/src/sgml/ref/alter_statistics.sgml
@@ -26,6 +26,7 @@ PostgreSQL documentation
ALTER STATISTICS <replaceable class="parameter">name</replaceable> OWNER TO { <replaceable class="parameter">new_owner</replaceable> | CURRENT_USER | SESSION_USER }
ALTER STATISTICS <replaceable class="parameter">name</replaceable> RENAME TO <replaceable class="parameter">new_name</replaceable>
ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET SCHEMA <replaceable class="parameter">new_schema</replaceable>
+ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET STATISTICS <replaceable class="parameter">new_target</replaceable>
</synopsis>
</refsynopsisdiv>
@@ -93,6 +94,22 @@ ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET SCHEMA <r
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">new_target</replaceable></term>
+ <listitem>
+ <para>
+ The statistic-gathering target for this statistic object for subsequent
+ <xref linkend="sql-analyze"/> operations.
+ The target can be set in the range 0 to 10000; alternatively, set it
+ to -1 to revert to using the system default statistics
+ target (<xref linkend="guc-default-statistics-target"/>).
+ For more information on the use of statistics by the
+ <productname>PostgreSQL</productname> query planner, refer to
+ <xref linkend="planner-stats"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
</refsect1>
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index d7004e5313..caa4fca99d 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -490,6 +490,19 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
}
}
+ /*
+ * Look at extended statistic objects too, because those may define their
+ * own statistic target. So we need to sample enough rows and then build
+ * the statistics with enough detail.
+ */
+ {
+ int nrows = ComputeExtStatisticsTarget(onerel,
+ attr_cnt, vacattrstats);
+
+ if (targrows < nrows)
+ targrows = nrows;
+ }
+
/*
* Acquire the sample rows
*/
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index cf406f6f96..356b6096ac 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -14,6 +14,7 @@
*/
#include "postgres.h"
+#include "access/heapam.h"
#include "access/relation.h"
#include "access/relscan.h"
#include "access/table.h"
@@ -21,6 +22,7 @@
#include "catalog/dependency.h"
#include "catalog/indexing.h"
#include "catalog/namespace.h"
+#include "catalog/objectaccess.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_statistic_ext.h"
#include "catalog/pg_statistic_ext_data.h"
@@ -29,6 +31,7 @@
#include "miscadmin.h"
#include "statistics/statistics.h"
#include "utils/builtins.h"
+#include "utils/fmgroids.h"
#include "utils/inval.h"
#include "utils/memutils.h"
#include "utils/rel.h"
@@ -336,6 +339,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_stxstattarget - 1] = Int32GetDatum(-1);
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);
@@ -414,6 +418,85 @@ CreateStatistics(CreateStatsStmt *stmt)
return myself;
}
+
+/*
+ * ALTER STATISTICS
+ */
+ObjectAddress
+AlterStatistics(AlterStatsStmt *stmt)
+{
+ Relation rel;
+ Oid stxoid;
+ HeapTuple oldtup;
+ HeapTuple newtup;
+ Datum repl_val[Natts_pg_statistic_ext];
+ bool repl_null[Natts_pg_statistic_ext];
+ bool repl_repl[Natts_pg_statistic_ext];
+ ObjectAddress address;
+ int newtarget = stmt->stxstattarget;
+
+ /* Limit target to a sane range */
+ if (newtarget < -1)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("statistics target %d is too low",
+ newtarget)));
+ }
+ else if (newtarget > 10000)
+ {
+ newtarget = 10000;
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("lowering statistics target to %d",
+ newtarget)));
+ }
+
+ /* lookup OID of the statistic object */
+ stxoid = get_statistics_object_oid(stmt->defnames, false);
+
+ /* Search pg_statistic_ext */
+ rel = table_open(StatisticExtRelationId, RowExclusiveLock);
+
+ oldtup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(stxoid));
+
+ /* Must be owner of the existing statistic object */
+ if (!pg_statistics_object_ownercheck(stxoid, GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_STATISTIC_EXT,
+ NameListToString(stmt->defnames));
+
+ /* Build new tuple. */
+ memset(repl_val, 0, sizeof(repl_val));
+ memset(repl_null, false, sizeof(repl_null));
+ memset(repl_repl, false, sizeof(repl_repl));
+
+ /* replace the stxstattarget column */
+ repl_repl[Anum_pg_statistic_ext_stxstattarget - 1] = true;
+ repl_val[Anum_pg_statistic_ext_stxstattarget - 1] = Int32GetDatum(newtarget);
+
+ newtup = heap_modify_tuple(oldtup, RelationGetDescr(rel),
+ repl_val, repl_null, repl_repl);
+
+ /* Update system catalog. */
+ CatalogTupleUpdate(rel, &newtup->t_self, newtup);
+
+ InvokeObjectPostAlterHook(StatisticExtRelationId, stxoid, 0);
+
+ ObjectAddressSet(address, StatisticExtRelationId, stxoid);
+
+ /*
+ * NOTE: because we only support altering the statistic target, not the
+ * other fields, there is no need to update dependencies.
+ */
+
+ heap_freetuple(newtup);
+ ReleaseSysCache(oldtup);
+
+ table_close(rel, RowExclusiveLock);
+
+ return address;
+}
+
/*
* Guts of statistics object deletion.
*/
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 78deade89b..048239fe34 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3493,6 +3493,18 @@ _copyCreateStatsStmt(const CreateStatsStmt *from)
return newnode;
}
+static AlterStatsStmt *
+_copyAlterStatsStmt(const AlterStatsStmt *from)
+{
+ AlterStatsStmt *newnode = makeNode(AlterStatsStmt);
+
+ COPY_NODE_FIELD(defnames);
+ COPY_SCALAR_FIELD(stxstattarget);
+ COPY_SCALAR_FIELD(if_not_exists);
+
+ return newnode;
+}
+
static CreateFunctionStmt *
_copyCreateFunctionStmt(const CreateFunctionStmt *from)
{
@@ -5249,6 +5261,9 @@ copyObjectImpl(const void *from)
case T_CreateStatsStmt:
retval = _copyCreateStatsStmt(from);
break;
+ case T_AlterStatsStmt:
+ retval = _copyAlterStatsStmt(from);
+ break;
case T_CreateFunctionStmt:
retval = _copyCreateFunctionStmt(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 4f2ebe5118..4c35cdf41e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1365,6 +1365,16 @@ _equalCreateStatsStmt(const CreateStatsStmt *a, const CreateStatsStmt *b)
return true;
}
+static bool
+_equalAlterStatsStmt(const AlterStatsStmt *a, const AlterStatsStmt *b)
+{
+ COMPARE_NODE_FIELD(defnames);
+ COMPARE_SCALAR_FIELD(stxstattarget);
+ COMPARE_SCALAR_FIELD(if_not_exists);
+
+ return true;
+}
+
static bool
_equalCreateFunctionStmt(const CreateFunctionStmt *a, const CreateFunctionStmt *b)
{
@@ -3309,6 +3319,9 @@ equal(const void *a, const void *b)
case T_CreateStatsStmt:
retval = _equalCreateStatsStmt(a, b);
break;
+ case T_AlterStatsStmt:
+ retval = _equalAlterStatsStmt(a, b);
+ break;
case T_CreateFunctionStmt:
retval = _equalCreateFunctionStmt(a, b);
break;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 8400dd319e..4551d7a023 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2662,6 +2662,16 @@ _outCreateStatsStmt(StringInfo str, const CreateStatsStmt *node)
WRITE_BOOL_FIELD(if_not_exists);
}
+static void
+_outAlterStatsStmt(StringInfo str, const AlterStatsStmt *node)
+{
+ WRITE_NODE_TYPE("ALTERSTATSSTMT");
+
+ WRITE_NODE_FIELD(defnames);
+ WRITE_INT_FIELD(stxstattarget);
+ WRITE_BOOL_FIELD(if_not_exists);
+}
+
static void
_outNotifyStmt(StringInfo str, const NotifyStmt *node)
{
@@ -4124,6 +4134,9 @@ outNode(StringInfo str, const void *obj)
case T_CreateStatsStmt:
_outCreateStatsStmt(str, obj);
break;
+ case T_AlterStatsStmt:
+ _outAlterStatsStmt(str, obj);
+ break;
case T_NotifyStmt:
_outNotifyStmt(str, obj);
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 208b4a1f28..29e24fdfea 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -252,7 +252,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
AlterOperatorStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
AlterCompositeTypeStmt AlterUserMappingStmt
- AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt
+ AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt AlterStatsStmt
AlterDefaultPrivilegesStmt DefACLAction
AnalyzeStmt CallStmt ClosePortalStmt ClusterStmt CommentStmt
ConstraintsSetStmt CopyStmt CreateAsStmt CreateCastStmt
@@ -852,6 +852,7 @@ stmt :
| AlterRoleSetStmt
| AlterRoleStmt
| AlterSubscriptionStmt
+ | AlterStatsStmt
| AlterTSConfigurationStmt
| AlterTSDictionaryStmt
| AlterUserMappingStmt
@@ -3984,6 +3985,34 @@ CreateStatsStmt:
}
;
+
+/*****************************************************************************
+ *
+ * QUERY :
+ * ALTER STATISTICS [IF EXISTS] stats_name
+ * SET STATISTICS <SignedIconst>
+ *
+ *****************************************************************************/
+
+AlterStatsStmt:
+ ALTER STATISTICS any_name SET STATISTICS SignedIconst
+ {
+ AlterStatsStmt *n = makeNode(AlterStatsStmt);
+ n->defnames = $3;
+ n->if_not_exists = false;
+ n->stxstattarget = $6;
+ $$ = (Node *)n;
+ }
+ | ALTER STATISTICS IF_P EXISTS any_name SET STATISTICS SignedIconst
+ {
+ AlterStatsStmt *n = makeNode(AlterStatsStmt);
+ n->defnames = $5;
+ n->if_not_exists = true;
+ n->stxstattarget = $8;
+ $$ = (Node *)n;
+ }
+ ;
+
/*****************************************************************************
*
* QUERY :
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index c56ed48270..f0d0deabaf 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -61,6 +61,7 @@ typedef struct StatExtEntry
char *name; /* statistics object's name */
Bitmapset *columns; /* attribute numbers covered by the object */
List *types; /* 'char' list of enabled statistic kinds */
+ int stattarget; /* statistic target (-1 for default) */
} StatExtEntry;
@@ -70,7 +71,8 @@ static VacAttrStats **lookup_var_attr_stats(Relation rel, Bitmapset *attrs,
static void statext_store(Oid relid,
MVNDistinct *ndistinct, MVDependencies *dependencies,
MCVList *mcv, VacAttrStats **stats);
-
+static int statext_compute_stattarget(int stattarget,
+ int natts, VacAttrStats **stats);
/*
* Compute requested extended stats, using the rows sampled for the plain
@@ -106,6 +108,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
MCVList *mcv = NULL;
VacAttrStats **stats;
ListCell *lc2;
+ int stattarget;
/*
* Check if we can build these stats based on the column analyzed. If
@@ -130,6 +133,13 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
Assert(bms_num_members(stat->columns) >= 2 &&
bms_num_members(stat->columns) <= STATS_MAX_DIMENSIONS);
+ /* compute current statistic target */
+ stattarget = statext_compute_stattarget(stat->stattarget,
+ bms_num_members(stat->columns),
+ stats);
+
+ /* XXX What if the target is set to 0? Reset the statistic? */
+
/* compute statistic of each requested type */
foreach(lc2, stat->types)
{
@@ -143,7 +153,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
stat->columns, stats);
else if (t == STATS_EXT_MCV)
mcv = statext_mcv_build(numrows, rows, stat->columns, stats,
- totalrows);
+ totalrows, stattarget);
}
/* store the statistics in the catalog */
@@ -156,6 +166,108 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
MemoryContextDelete(cxt);
}
+/*
+ * ComputeExtStatisticsTarget
+ * Compute the largest statistic target for all extended statistics.
+ */
+int
+ComputeExtStatisticsTarget(Relation onerel,
+ int natts, VacAttrStats **vacattrstats)
+{
+ Relation pg_stext;
+ ListCell *lc;
+ List *lstats;
+ MemoryContext cxt;
+ MemoryContext oldcxt;
+ int result = 0;
+
+ cxt = AllocSetContextCreate(CurrentMemoryContext,
+ "ComputeExtStatisticsTarget",
+ ALLOCSET_DEFAULT_SIZES);
+ oldcxt = MemoryContextSwitchTo(cxt);
+
+ pg_stext = table_open(StatisticExtRelationId, RowExclusiveLock);
+ lstats = fetch_statentries_for_relation(pg_stext, RelationGetRelid(onerel));
+
+ foreach(lc, lstats)
+ {
+ StatExtEntry *stat = (StatExtEntry *) lfirst(lc);
+ int stattarget = stat->stattarget;
+ VacAttrStats **stats;
+ int nattrs = bms_num_members(stat->columns);
+
+ /*
+ * Check if we can build these stats based on the column analyzed. If
+ * not, report this fact (except in autovacuum) and move on.
+ */
+ stats = lookup_var_attr_stats(onerel, stat->columns,
+ natts, vacattrstats);
+
+ /* ignore statistics we'll skip later */
+ if (!stats)
+ continue;
+
+ /* compute statistic target, based on the statistic object and attributes */
+ stattarget = statext_compute_stattarget(stat->stattarget, nattrs, stats);
+
+ /* Use the largest value for all statistic objects. */
+ if (stattarget > result)
+ result = stattarget;
+ }
+
+ table_close(pg_stext, RowExclusiveLock);
+
+ MemoryContextSwitchTo(oldcxt);
+ MemoryContextDelete(cxt);
+
+ return (300 * result);
+}
+
+/*
+ * statext_compute_stattarget
+ * compute statistic target for an extended statistic
+ *
+ * If the statistic object has custom value (set using ALTER STATISTICS ...
+ * SET STATISTICS), then we simply use that. Otherwise we look at targets
+ * for columns the object is defined on and use the largest value. Finally,
+ * if the target is still (-1), we use the default_statistics_target.
+ */
+static int
+statext_compute_stattarget(int stattarget, int nattrs, VacAttrStats **stats)
+{
+ int i;
+
+ /* if there's statistic target set for the statistic object, use it */
+ if (stattarget >= 0)
+ return stattarget;
+
+ /*
+ * The stattarget column is negative, we look at targets for each
+ * column the statistics is based on, and use use the largest value
+ * for any of the columns.
+ */
+ {
+ for (i = 0; i < nattrs; i++)
+ {
+ if (stats[i]->attr->attstattarget > stattarget)
+ stattarget = stats[i]->attr->attstattarget;
+ }
+ }
+
+ /*
+ * If the value is still negative (so neither the extended statistic
+ * object nor any of the columns) have custom statistic target set.
+ * In that case use the default target.
+ */
+ if (stattarget < 0)
+ stattarget = default_statistics_target;
+
+ /* As this point we should have a valid statistic target. */
+ Assert((stattarget >= 0) && (stattarget <= 10000));
+
+ return stattarget;
+}
+
/*
* statext_is_kind_built
* Is this stat kind built in the given pg_statistic_ext_data tuple?
@@ -224,6 +336,7 @@ fetch_statentries_for_relation(Relation pg_statext, Oid relid)
entry->statOid = staForm->oid;
entry->schema = get_namespace_name(staForm->stxnamespace);
entry->name = pstrdup(NameStr(staForm->stxname));
+ entry->stattarget = staForm->stxstattarget;
for (i = 0; i < staForm->stxkeys.dim1; i++)
{
entry->columns = bms_add_member(entry->columns,
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index 913a72ff67..8491b20af3 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -162,7 +162,8 @@ get_mincount_for_mcv_list(int samplerows, double totalrows)
*/
MCVList *
statext_mcv_build(int numrows, HeapTuple *rows, Bitmapset *attrs,
- VacAttrStats **stats, double totalrows)
+ VacAttrStats **stats, double totalrows,
+ int stattarget)
{
int i,
numattrs,
@@ -194,12 +195,7 @@ statext_mcv_build(int numrows, HeapTuple *rows, Bitmapset *attrs,
* Maximum number of MCV items to store, based on the attribute with the
* largest stats target (and the number of groups we have available).
*/
- nitems = stats[0]->attr->attstattarget;
- for (i = 1; i < numattrs; i++)
- {
- if (stats[i]->attr->attstattarget > nitems)
- nitems = stats[i]->attr->attstattarget;
- }
+ nitems = stattarget;
if (nitems > ngroups)
nitems = ngroups;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 05ec7f3ac6..1299066c63 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1688,6 +1688,10 @@ ProcessUtilitySlow(ParseState *pstate,
address = CreateStatistics((CreateStatsStmt *) parsetree);
break;
+ case T_AlterStatsStmt:
+ address = AlterStatistics((AlterStatsStmt *) parsetree);
+ break;
+
case T_AlterCollationStmt:
address = AlterCollation((AlterCollationStmt *) parsetree);
break;
@@ -2805,6 +2809,10 @@ CreateCommandTag(Node *parsetree)
tag = "CREATE STATISTICS";
break;
+ case T_AlterStatsStmt:
+ tag = "ALTER STATISTICS";
+ break;
+
case T_DeallocateStmt:
{
DeallocateStmt *stmt = (DeallocateStmt *) parsetree;
@@ -3393,6 +3401,10 @@ GetCommandLogLevel(Node *parsetree)
lev = LOGSTMT_DDL;
break;
+ case T_AlterStatsStmt:
+ lev = LOGSTMT_DDL;
+ break;
+
case T_AlterCollationStmt:
lev = LOGSTMT_DDL;
break;
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 9009b05e12..35a264c29b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1840,7 +1840,7 @@ psql_completion(const char *text, int start, int end)
/* ALTER STATISTICS <name> */
else if (Matches("ALTER", "STATISTICS", MatchAny))
- COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA");
+ COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA", "SET STATISTICS");
/* ALTER TRIGGER <name>, add ON */
else if (Matches("ALTER", "TRIGGER", MatchAny))
diff --git a/src/include/catalog/pg_statistic_ext.h b/src/include/catalog/pg_statistic_ext.h
index d8c5e0651e..5f6bfdd56c 100644
--- a/src/include/catalog/pg_statistic_ext.h
+++ b/src/include/catalog/pg_statistic_ext.h
@@ -41,6 +41,7 @@ CATALOG(pg_statistic_ext,3381,StatisticExtRelationId)
Oid stxnamespace; /* OID of statistics object's namespace */
Oid stxowner; /* statistics object's owner */
+ int32 stxstattarget BKI_DEFAULT(-1); /* statistic target */
/*
* variable-length fields start here, but we allow direct access to
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index b4e7db67c3..1dc6dc2ca0 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -87,6 +87,7 @@ extern ObjectAddress AlterOperator(AlterOperatorStmt *stmt);
/* commands/statscmds.c */
extern ObjectAddress CreateStatistics(CreateStatsStmt *stmt);
+extern ObjectAddress AlterStatistics(AlterStatsStmt *stmt);
extern void RemoveStatisticsById(Oid statsOid);
extern void UpdateStatisticsForTypeChange(Oid statsOid,
Oid relationOid, int attnum,
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 4e2fb39105..57589d4fac 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -420,6 +420,7 @@ typedef enum NodeTag
T_CreateStatsStmt,
T_AlterCollationStmt,
T_CallStmt,
+ T_AlterStatsStmt,
/*
* TAGS FOR PARSE TREE NODES (parsenodes.h)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 94ded3c135..a6a3382a46 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2789,6 +2789,18 @@ typedef struct CreateStatsStmt
bool if_not_exists; /* do nothing if stats name already exists */
} CreateStatsStmt;
+/* ----------------------
+ * Alter Statistics Statement
+ * ----------------------
+ */
+typedef struct AlterStatsStmt
+{
+ NodeTag type;
+ List *defnames; /* qualified name (list of Value strings) */
+ int stxstattarget; /* statistics target, or NULL */
+ bool if_not_exists; /* do nothing if stats name already exists */
+} AlterStatsStmt;
+
/* ----------------------
* Create Function Statement
* ----------------------
diff --git a/src/include/statistics/extended_stats_internal.h b/src/include/statistics/extended_stats_internal.h
index 8fc541993f..71fd509f16 100644
--- a/src/include/statistics/extended_stats_internal.h
+++ b/src/include/statistics/extended_stats_internal.h
@@ -71,7 +71,7 @@ extern MVDependencies *statext_dependencies_deserialize(bytea *data);
extern MCVList *statext_mcv_build(int numrows, HeapTuple *rows,
Bitmapset *attrs, VacAttrStats **stats,
- double totalrows);
+ double totalrows, int stattarget);
extern bytea *statext_mcv_serialize(MCVList *mcv, VacAttrStats **stats);
extern MCVList *statext_mcv_deserialize(bytea *data);
diff --git a/src/include/statistics/statistics.h b/src/include/statistics/statistics.h
index cb7bc630e9..53ec906936 100644
--- a/src/include/statistics/statistics.h
+++ b/src/include/statistics/statistics.h
@@ -100,6 +100,8 @@ extern MCVList *statext_mcv_load(Oid mvoid);
extern void BuildRelationExtStatistics(Relation onerel, double totalrows,
int numrows, HeapTuple *rows,
int natts, VacAttrStats **vacattrstats);
+extern int ComputeExtStatisticsTarget(Relation onerel,
+ int natts, VacAttrStats **stats);
extern bool statext_is_kind_built(HeapTuple htup, char kind);
extern Selectivity dependencies_clauselist_selectivity(PlannerInfo *root,
List *clauses,
On Sun, Jul 07, 2019 at 12:02:38AM +0200, Tomas Vondra wrote:
Hi,
apparently v1 of the ALTER STATISTICS patch was a bit confused about
length of the VacAttrStats array passed to statext_compute_stattarget,
causing segfaults. Attached v2 patch fixes that, and it also makes sure
we print warnings about ignored statistics only once.
v3 of the patch, adding pg_dump support - it works just like when you
tweak statistics target for a column, for example. When the value stored
in the catalog is not -1, pg_dump emits a separate ALTER STATISTICS
command setting it (for the already created statistics object).
I've considered making it part of CREATE STATISTICS itself, but it seems
a bit cumbersome and we don't do it for columns either. I'm not against
adding it in the future, but at this point I don't see a need.
At this point I'm not aware of any missing or broken pieces, so I'd
welcome feedback.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
alter-statistics-v3.patchtext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/alter_statistics.sgml b/doc/src/sgml/ref/alter_statistics.sgml
index 58c7ed020d..987f9dbc6f 100644
--- a/doc/src/sgml/ref/alter_statistics.sgml
+++ b/doc/src/sgml/ref/alter_statistics.sgml
@@ -26,6 +26,7 @@ PostgreSQL documentation
ALTER STATISTICS <replaceable class="parameter">name</replaceable> OWNER TO { <replaceable class="parameter">new_owner</replaceable> | CURRENT_USER | SESSION_USER }
ALTER STATISTICS <replaceable class="parameter">name</replaceable> RENAME TO <replaceable class="parameter">new_name</replaceable>
ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET SCHEMA <replaceable class="parameter">new_schema</replaceable>
+ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET STATISTICS <replaceable class="parameter">new_target</replaceable>
</synopsis>
</refsynopsisdiv>
@@ -93,6 +94,22 @@ ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET SCHEMA <r
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">new_target</replaceable></term>
+ <listitem>
+ <para>
+ The statistic-gathering target for this statistic object for subsequent
+ <xref linkend="sql-analyze"/> operations.
+ The target can be set in the range 0 to 10000; alternatively, set it
+ to -1 to revert to using the system default statistics
+ target (<xref linkend="guc-default-statistics-target"/>).
+ For more information on the use of statistics by the
+ <productname>PostgreSQL</productname> query planner, refer to
+ <xref linkend="planner-stats"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
</refsect1>
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index d7004e5313..caa4fca99d 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -490,6 +490,19 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
}
}
+ /*
+ * Look at extended statistic objects too, because those may define their
+ * own statistic target. So we need to sample enough rows and then build
+ * the statistics with enough detail.
+ */
+ {
+ int nrows = ComputeExtStatisticsTarget(onerel,
+ attr_cnt, vacattrstats);
+
+ if (targrows < nrows)
+ targrows = nrows;
+ }
+
/*
* Acquire the sample rows
*/
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index cf406f6f96..356b6096ac 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -14,6 +14,7 @@
*/
#include "postgres.h"
+#include "access/heapam.h"
#include "access/relation.h"
#include "access/relscan.h"
#include "access/table.h"
@@ -21,6 +22,7 @@
#include "catalog/dependency.h"
#include "catalog/indexing.h"
#include "catalog/namespace.h"
+#include "catalog/objectaccess.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_statistic_ext.h"
#include "catalog/pg_statistic_ext_data.h"
@@ -29,6 +31,7 @@
#include "miscadmin.h"
#include "statistics/statistics.h"
#include "utils/builtins.h"
+#include "utils/fmgroids.h"
#include "utils/inval.h"
#include "utils/memutils.h"
#include "utils/rel.h"
@@ -336,6 +339,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_stxstattarget - 1] = Int32GetDatum(-1);
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);
@@ -414,6 +418,85 @@ CreateStatistics(CreateStatsStmt *stmt)
return myself;
}
+
+/*
+ * ALTER STATISTICS
+ */
+ObjectAddress
+AlterStatistics(AlterStatsStmt *stmt)
+{
+ Relation rel;
+ Oid stxoid;
+ HeapTuple oldtup;
+ HeapTuple newtup;
+ Datum repl_val[Natts_pg_statistic_ext];
+ bool repl_null[Natts_pg_statistic_ext];
+ bool repl_repl[Natts_pg_statistic_ext];
+ ObjectAddress address;
+ int newtarget = stmt->stxstattarget;
+
+ /* Limit target to a sane range */
+ if (newtarget < -1)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("statistics target %d is too low",
+ newtarget)));
+ }
+ else if (newtarget > 10000)
+ {
+ newtarget = 10000;
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("lowering statistics target to %d",
+ newtarget)));
+ }
+
+ /* lookup OID of the statistic object */
+ stxoid = get_statistics_object_oid(stmt->defnames, false);
+
+ /* Search pg_statistic_ext */
+ rel = table_open(StatisticExtRelationId, RowExclusiveLock);
+
+ oldtup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(stxoid));
+
+ /* Must be owner of the existing statistic object */
+ if (!pg_statistics_object_ownercheck(stxoid, GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_STATISTIC_EXT,
+ NameListToString(stmt->defnames));
+
+ /* Build new tuple. */
+ memset(repl_val, 0, sizeof(repl_val));
+ memset(repl_null, false, sizeof(repl_null));
+ memset(repl_repl, false, sizeof(repl_repl));
+
+ /* replace the stxstattarget column */
+ repl_repl[Anum_pg_statistic_ext_stxstattarget - 1] = true;
+ repl_val[Anum_pg_statistic_ext_stxstattarget - 1] = Int32GetDatum(newtarget);
+
+ newtup = heap_modify_tuple(oldtup, RelationGetDescr(rel),
+ repl_val, repl_null, repl_repl);
+
+ /* Update system catalog. */
+ CatalogTupleUpdate(rel, &newtup->t_self, newtup);
+
+ InvokeObjectPostAlterHook(StatisticExtRelationId, stxoid, 0);
+
+ ObjectAddressSet(address, StatisticExtRelationId, stxoid);
+
+ /*
+ * NOTE: because we only support altering the statistic target, not the
+ * other fields, there is no need to update dependencies.
+ */
+
+ heap_freetuple(newtup);
+ ReleaseSysCache(oldtup);
+
+ table_close(rel, RowExclusiveLock);
+
+ return address;
+}
+
/*
* Guts of statistics object deletion.
*/
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 78deade89b..048239fe34 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3493,6 +3493,18 @@ _copyCreateStatsStmt(const CreateStatsStmt *from)
return newnode;
}
+static AlterStatsStmt *
+_copyAlterStatsStmt(const AlterStatsStmt *from)
+{
+ AlterStatsStmt *newnode = makeNode(AlterStatsStmt);
+
+ COPY_NODE_FIELD(defnames);
+ COPY_SCALAR_FIELD(stxstattarget);
+ COPY_SCALAR_FIELD(if_not_exists);
+
+ return newnode;
+}
+
static CreateFunctionStmt *
_copyCreateFunctionStmt(const CreateFunctionStmt *from)
{
@@ -5249,6 +5261,9 @@ copyObjectImpl(const void *from)
case T_CreateStatsStmt:
retval = _copyCreateStatsStmt(from);
break;
+ case T_AlterStatsStmt:
+ retval = _copyAlterStatsStmt(from);
+ break;
case T_CreateFunctionStmt:
retval = _copyCreateFunctionStmt(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 4f2ebe5118..4c35cdf41e 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1365,6 +1365,16 @@ _equalCreateStatsStmt(const CreateStatsStmt *a, const CreateStatsStmt *b)
return true;
}
+static bool
+_equalAlterStatsStmt(const AlterStatsStmt *a, const AlterStatsStmt *b)
+{
+ COMPARE_NODE_FIELD(defnames);
+ COMPARE_SCALAR_FIELD(stxstattarget);
+ COMPARE_SCALAR_FIELD(if_not_exists);
+
+ return true;
+}
+
static bool
_equalCreateFunctionStmt(const CreateFunctionStmt *a, const CreateFunctionStmt *b)
{
@@ -3309,6 +3319,9 @@ equal(const void *a, const void *b)
case T_CreateStatsStmt:
retval = _equalCreateStatsStmt(a, b);
break;
+ case T_AlterStatsStmt:
+ retval = _equalAlterStatsStmt(a, b);
+ break;
case T_CreateFunctionStmt:
retval = _equalCreateFunctionStmt(a, b);
break;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 8400dd319e..4551d7a023 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2662,6 +2662,16 @@ _outCreateStatsStmt(StringInfo str, const CreateStatsStmt *node)
WRITE_BOOL_FIELD(if_not_exists);
}
+static void
+_outAlterStatsStmt(StringInfo str, const AlterStatsStmt *node)
+{
+ WRITE_NODE_TYPE("ALTERSTATSSTMT");
+
+ WRITE_NODE_FIELD(defnames);
+ WRITE_INT_FIELD(stxstattarget);
+ WRITE_BOOL_FIELD(if_not_exists);
+}
+
static void
_outNotifyStmt(StringInfo str, const NotifyStmt *node)
{
@@ -4124,6 +4134,9 @@ outNode(StringInfo str, const void *obj)
case T_CreateStatsStmt:
_outCreateStatsStmt(str, obj);
break;
+ case T_AlterStatsStmt:
+ _outAlterStatsStmt(str, obj);
+ break;
case T_NotifyStmt:
_outNotifyStmt(str, obj);
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 208b4a1f28..29e24fdfea 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -252,7 +252,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
AlterOperatorStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
AlterCompositeTypeStmt AlterUserMappingStmt
- AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt
+ AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt AlterStatsStmt
AlterDefaultPrivilegesStmt DefACLAction
AnalyzeStmt CallStmt ClosePortalStmt ClusterStmt CommentStmt
ConstraintsSetStmt CopyStmt CreateAsStmt CreateCastStmt
@@ -852,6 +852,7 @@ stmt :
| AlterRoleSetStmt
| AlterRoleStmt
| AlterSubscriptionStmt
+ | AlterStatsStmt
| AlterTSConfigurationStmt
| AlterTSDictionaryStmt
| AlterUserMappingStmt
@@ -3984,6 +3985,34 @@ CreateStatsStmt:
}
;
+
+/*****************************************************************************
+ *
+ * QUERY :
+ * ALTER STATISTICS [IF EXISTS] stats_name
+ * SET STATISTICS <SignedIconst>
+ *
+ *****************************************************************************/
+
+AlterStatsStmt:
+ ALTER STATISTICS any_name SET STATISTICS SignedIconst
+ {
+ AlterStatsStmt *n = makeNode(AlterStatsStmt);
+ n->defnames = $3;
+ n->if_not_exists = false;
+ n->stxstattarget = $6;
+ $$ = (Node *)n;
+ }
+ | ALTER STATISTICS IF_P EXISTS any_name SET STATISTICS SignedIconst
+ {
+ AlterStatsStmt *n = makeNode(AlterStatsStmt);
+ n->defnames = $5;
+ n->if_not_exists = true;
+ n->stxstattarget = $8;
+ $$ = (Node *)n;
+ }
+ ;
+
/*****************************************************************************
*
* QUERY :
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index c56ed48270..f0d0deabaf 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -61,6 +61,7 @@ typedef struct StatExtEntry
char *name; /* statistics object's name */
Bitmapset *columns; /* attribute numbers covered by the object */
List *types; /* 'char' list of enabled statistic kinds */
+ int stattarget; /* statistic target (-1 for default) */
} StatExtEntry;
@@ -70,7 +71,8 @@ static VacAttrStats **lookup_var_attr_stats(Relation rel, Bitmapset *attrs,
static void statext_store(Oid relid,
MVNDistinct *ndistinct, MVDependencies *dependencies,
MCVList *mcv, VacAttrStats **stats);
-
+static int statext_compute_stattarget(int stattarget,
+ int natts, VacAttrStats **stats);
/*
* Compute requested extended stats, using the rows sampled for the plain
@@ -106,6 +108,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
MCVList *mcv = NULL;
VacAttrStats **stats;
ListCell *lc2;
+ int stattarget;
/*
* Check if we can build these stats based on the column analyzed. If
@@ -130,6 +133,13 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
Assert(bms_num_members(stat->columns) >= 2 &&
bms_num_members(stat->columns) <= STATS_MAX_DIMENSIONS);
+ /* compute current statistic target */
+ stattarget = statext_compute_stattarget(stat->stattarget,
+ bms_num_members(stat->columns),
+ stats);
+
+ /* XXX What if the target is set to 0? Reset the statistic? */
+
/* compute statistic of each requested type */
foreach(lc2, stat->types)
{
@@ -143,7 +153,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
stat->columns, stats);
else if (t == STATS_EXT_MCV)
mcv = statext_mcv_build(numrows, rows, stat->columns, stats,
- totalrows);
+ totalrows, stattarget);
}
/* store the statistics in the catalog */
@@ -156,6 +166,108 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
MemoryContextDelete(cxt);
}
+/*
+ * ComputeExtStatisticsTarget
+ * Compute the largest statistic target for all extended statistics.
+ */
+int
+ComputeExtStatisticsTarget(Relation onerel,
+ int natts, VacAttrStats **vacattrstats)
+{
+ Relation pg_stext;
+ ListCell *lc;
+ List *lstats;
+ MemoryContext cxt;
+ MemoryContext oldcxt;
+ int result = 0;
+
+ cxt = AllocSetContextCreate(CurrentMemoryContext,
+ "ComputeExtStatisticsTarget",
+ ALLOCSET_DEFAULT_SIZES);
+ oldcxt = MemoryContextSwitchTo(cxt);
+
+ pg_stext = table_open(StatisticExtRelationId, RowExclusiveLock);
+ lstats = fetch_statentries_for_relation(pg_stext, RelationGetRelid(onerel));
+
+ foreach(lc, lstats)
+ {
+ StatExtEntry *stat = (StatExtEntry *) lfirst(lc);
+ int stattarget = stat->stattarget;
+ VacAttrStats **stats;
+ int nattrs = bms_num_members(stat->columns);
+
+ /*
+ * Check if we can build these stats based on the column analyzed. If
+ * not, report this fact (except in autovacuum) and move on.
+ */
+ stats = lookup_var_attr_stats(onerel, stat->columns,
+ natts, vacattrstats);
+
+ /* ignore statistics we'll skip later */
+ if (!stats)
+ continue;
+
+ /* compute statistic target, based on the statistic object and attributes */
+ stattarget = statext_compute_stattarget(stat->stattarget, nattrs, stats);
+
+ /* Use the largest value for all statistic objects. */
+ if (stattarget > result)
+ result = stattarget;
+ }
+
+ table_close(pg_stext, RowExclusiveLock);
+
+ MemoryContextSwitchTo(oldcxt);
+ MemoryContextDelete(cxt);
+
+ return (300 * result);
+}
+
+/*
+ * statext_compute_stattarget
+ * compute statistic target for an extended statistic
+ *
+ * If the statistic object has custom value (set using ALTER STATISTICS ...
+ * SET STATISTICS), then we simply use that. Otherwise we look at targets
+ * for columns the object is defined on and use the largest value. Finally,
+ * if the target is still (-1), we use the default_statistics_target.
+ */
+static int
+statext_compute_stattarget(int stattarget, int nattrs, VacAttrStats **stats)
+{
+ int i;
+
+ /* if there's statistic target set for the statistic object, use it */
+ if (stattarget >= 0)
+ return stattarget;
+
+ /*
+ * The stattarget column is negative, we look at targets for each
+ * column the statistics is based on, and use use the largest value
+ * for any of the columns.
+ */
+ {
+ for (i = 0; i < nattrs; i++)
+ {
+ if (stats[i]->attr->attstattarget > stattarget)
+ stattarget = stats[i]->attr->attstattarget;
+ }
+ }
+
+ /*
+ * If the value is still negative (so neither the extended statistic
+ * object nor any of the columns) have custom statistic target set.
+ * In that case use the default target.
+ */
+ if (stattarget < 0)
+ stattarget = default_statistics_target;
+
+ /* As this point we should have a valid statistic target. */
+ Assert((stattarget >= 0) && (stattarget <= 10000));
+
+ return stattarget;
+}
+
/*
* statext_is_kind_built
* Is this stat kind built in the given pg_statistic_ext_data tuple?
@@ -224,6 +336,7 @@ fetch_statentries_for_relation(Relation pg_statext, Oid relid)
entry->statOid = staForm->oid;
entry->schema = get_namespace_name(staForm->stxnamespace);
entry->name = pstrdup(NameStr(staForm->stxname));
+ entry->stattarget = staForm->stxstattarget;
for (i = 0; i < staForm->stxkeys.dim1; i++)
{
entry->columns = bms_add_member(entry->columns,
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index 913a72ff67..8491b20af3 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -162,7 +162,8 @@ get_mincount_for_mcv_list(int samplerows, double totalrows)
*/
MCVList *
statext_mcv_build(int numrows, HeapTuple *rows, Bitmapset *attrs,
- VacAttrStats **stats, double totalrows)
+ VacAttrStats **stats, double totalrows,
+ int stattarget)
{
int i,
numattrs,
@@ -194,12 +195,7 @@ statext_mcv_build(int numrows, HeapTuple *rows, Bitmapset *attrs,
* Maximum number of MCV items to store, based on the attribute with the
* largest stats target (and the number of groups we have available).
*/
- nitems = stats[0]->attr->attstattarget;
- for (i = 1; i < numattrs; i++)
- {
- if (stats[i]->attr->attstattarget > nitems)
- nitems = stats[i]->attr->attstattarget;
- }
+ nitems = stattarget;
if (nitems > ngroups)
nitems = ngroups;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 05ec7f3ac6..1299066c63 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1688,6 +1688,10 @@ ProcessUtilitySlow(ParseState *pstate,
address = CreateStatistics((CreateStatsStmt *) parsetree);
break;
+ case T_AlterStatsStmt:
+ address = AlterStatistics((AlterStatsStmt *) parsetree);
+ break;
+
case T_AlterCollationStmt:
address = AlterCollation((AlterCollationStmt *) parsetree);
break;
@@ -2805,6 +2809,10 @@ CreateCommandTag(Node *parsetree)
tag = "CREATE STATISTICS";
break;
+ case T_AlterStatsStmt:
+ tag = "ALTER STATISTICS";
+ break;
+
case T_DeallocateStmt:
{
DeallocateStmt *stmt = (DeallocateStmt *) parsetree;
@@ -3393,6 +3401,10 @@ GetCommandLogLevel(Node *parsetree)
lev = LOGSTMT_DDL;
break;
+ case T_AlterStatsStmt:
+ lev = LOGSTMT_DDL;
+ break;
+
case T_AlterCollationStmt:
lev = LOGSTMT_DDL;
break;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 806fc78f04..f40cbaae34 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7172,6 +7172,7 @@ getExtendedStatistics(Archive *fout)
int i_stxname;
int i_stxnamespace;
int i_rolname;
+ int i_stattarget;
int i;
/* Extended statistics were new in v10 */
@@ -7180,10 +7181,16 @@ getExtendedStatistics(Archive *fout)
query = createPQExpBuffer();
- appendPQExpBuffer(query, "SELECT tableoid, oid, stxname, "
- "stxnamespace, (%s stxowner) AS rolname "
- "FROM pg_catalog.pg_statistic_ext",
- username_subquery);
+ if (fout->remoteVersion < 130000)
+ appendPQExpBuffer(query, "SELECT tableoid, oid, stxname, "
+ "stxnamespace, (%s stxowner) AS rolname, (-1) AS stxstattarget "
+ "FROM pg_catalog.pg_statistic_ext",
+ username_subquery);
+ else
+ appendPQExpBuffer(query, "SELECT tableoid, oid, stxname, "
+ "stxnamespace, (%s stxowner) AS rolname, stxstattarget "
+ "FROM pg_catalog.pg_statistic_ext",
+ username_subquery);
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -7194,6 +7201,7 @@ getExtendedStatistics(Archive *fout)
i_stxname = PQfnumber(res, "stxname");
i_stxnamespace = PQfnumber(res, "stxnamespace");
i_rolname = PQfnumber(res, "rolname");
+ i_stattarget = PQfnumber(res, "stxstattarget");
statsextinfo = (StatsExtInfo *) pg_malloc(ntups * sizeof(StatsExtInfo));
@@ -7208,7 +7216,8 @@ getExtendedStatistics(Archive *fout)
findNamespace(fout,
atooid(PQgetvalue(res, i, i_stxnamespace)));
statsextinfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
-
+ statsextinfo[i].stattarget = atoi(PQgetvalue(res, i, i_stattarget));
+
/* Decide whether we want to dump it */
selectDumpableObject(&(statsextinfo[i].dobj), fout);
@@ -16495,6 +16504,19 @@ dumpStatisticsExt(Archive *fout, StatsExtInfo *statsextinfo)
/* Result of pg_get_statisticsobjdef is complete except for semicolon */
appendPQExpBuffer(q, "%s;\n", stxdef);
+ /*
+ * We only issue an ALTER STATISTICS statement if the stxstattarget entry
+ * for this statictics object is non-negative (i.e. it's not the default
+ * value).
+ */
+ if (statsextinfo->stattarget >= 0)
+ {
+ appendPQExpBuffer(q, "ALTER STATISTICS %s ",
+ fmtQualifiedDumpable(statsextinfo));
+ appendPQExpBuffer(q, "SET STATISTICS %d;\n",
+ statsextinfo->stattarget);
+ }
+
appendPQExpBuffer(delq, "DROP STATISTICS %s;\n",
fmtQualifiedDumpable(statsextinfo));
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index c3c2ea1473..baaa2a24a0 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -386,6 +386,7 @@ typedef struct _statsExtInfo
{
DumpableObject dobj;
char *rolname; /* name of owner, or empty string */
+ int stattarget; /* statistic target */
} StatsExtInfo;
typedef struct _ruleInfo
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index c56bf00e4b..7617f35a4f 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2504,6 +2504,17 @@ my %tests = (
unlike => { exclude_dump_test_schema => 1, },
},
+ 'ALTER STATISTICS extended_stats_options' => {
+ create_order => 98,
+ create_sql => 'ALTER STATISTICS dump_test.test_ext_stats_opts SET STATISTICS 1000',
+ regexp => qr/^
+ \QALTER STATISTICS dump_test.test_ext_stats_opts SET STATISTICS 1000;\E
+ /xms,
+ like =>
+ { %full_runs, %dump_test_schema_runs, section_post_data => 1, },
+ unlike => { exclude_dump_test_schema => 1, },
+ },
+
'CREATE SEQUENCE test_table_col1_seq' => {
regexp => qr/^
\QCREATE SEQUENCE dump_test.test_table_col1_seq\E
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 9009b05e12..35a264c29b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1840,7 +1840,7 @@ psql_completion(const char *text, int start, int end)
/* ALTER STATISTICS <name> */
else if (Matches("ALTER", "STATISTICS", MatchAny))
- COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA");
+ COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA", "SET STATISTICS");
/* ALTER TRIGGER <name>, add ON */
else if (Matches("ALTER", "TRIGGER", MatchAny))
diff --git a/src/include/catalog/pg_statistic_ext.h b/src/include/catalog/pg_statistic_ext.h
index d8c5e0651e..5f6bfdd56c 100644
--- a/src/include/catalog/pg_statistic_ext.h
+++ b/src/include/catalog/pg_statistic_ext.h
@@ -41,6 +41,7 @@ CATALOG(pg_statistic_ext,3381,StatisticExtRelationId)
Oid stxnamespace; /* OID of statistics object's namespace */
Oid stxowner; /* statistics object's owner */
+ int32 stxstattarget BKI_DEFAULT(-1); /* statistic target */
/*
* variable-length fields start here, but we allow direct access to
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index b4e7db67c3..1dc6dc2ca0 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -87,6 +87,7 @@ extern ObjectAddress AlterOperator(AlterOperatorStmt *stmt);
/* commands/statscmds.c */
extern ObjectAddress CreateStatistics(CreateStatsStmt *stmt);
+extern ObjectAddress AlterStatistics(AlterStatsStmt *stmt);
extern void RemoveStatisticsById(Oid statsOid);
extern void UpdateStatisticsForTypeChange(Oid statsOid,
Oid relationOid, int attnum,
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 4e2fb39105..57589d4fac 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -420,6 +420,7 @@ typedef enum NodeTag
T_CreateStatsStmt,
T_AlterCollationStmt,
T_CallStmt,
+ T_AlterStatsStmt,
/*
* TAGS FOR PARSE TREE NODES (parsenodes.h)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 94ded3c135..a6a3382a46 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2789,6 +2789,18 @@ typedef struct CreateStatsStmt
bool if_not_exists; /* do nothing if stats name already exists */
} CreateStatsStmt;
+/* ----------------------
+ * Alter Statistics Statement
+ * ----------------------
+ */
+typedef struct AlterStatsStmt
+{
+ NodeTag type;
+ List *defnames; /* qualified name (list of Value strings) */
+ int stxstattarget; /* statistics target, or NULL */
+ bool if_not_exists; /* do nothing if stats name already exists */
+} AlterStatsStmt;
+
/* ----------------------
* Create Function Statement
* ----------------------
diff --git a/src/include/statistics/extended_stats_internal.h b/src/include/statistics/extended_stats_internal.h
index 8fc541993f..71fd509f16 100644
--- a/src/include/statistics/extended_stats_internal.h
+++ b/src/include/statistics/extended_stats_internal.h
@@ -71,7 +71,7 @@ extern MVDependencies *statext_dependencies_deserialize(bytea *data);
extern MCVList *statext_mcv_build(int numrows, HeapTuple *rows,
Bitmapset *attrs, VacAttrStats **stats,
- double totalrows);
+ double totalrows, int stattarget);
extern bytea *statext_mcv_serialize(MCVList *mcv, VacAttrStats **stats);
extern MCVList *statext_mcv_deserialize(bytea *data);
diff --git a/src/include/statistics/statistics.h b/src/include/statistics/statistics.h
index cb7bc630e9..53ec906936 100644
--- a/src/include/statistics/statistics.h
+++ b/src/include/statistics/statistics.h
@@ -100,6 +100,8 @@ extern MCVList *statext_mcv_load(Oid mvoid);
extern void BuildRelationExtStatistics(Relation onerel, double totalrows,
int numrows, HeapTuple *rows,
int natts, VacAttrStats **vacattrstats);
+extern int ComputeExtStatisticsTarget(Relation onerel,
+ int natts, VacAttrStats **stats);
extern bool statext_is_kind_built(HeapTuple htup, char kind);
extern Selectivity dependencies_clauselist_selectivity(PlannerInfo *root,
List *clauses,
On Tuesday, July 9, 2019, Tomas Vondra wrote:
apparently v1 of the ALTER STATISTICS patch was a bit confused about
length of the VacAttrStats array passed to statext_compute_stattarget,
causing segfaults. Attached v2 patch fixes that, and it also makes sure
we print warnings about ignored statistics only once.v3 of the patch, adding pg_dump support - it works just like when you tweak
statistics target for a column, for example. When the value stored in the
catalog is not -1, pg_dump emits a separate ALTER STATISTICS command setting
it (for the already created statistics object).
Hi Tomas, I stumbled upon your patch.
According to the CF bot, your patch applies cleanly, builds successfully, and
passes make world. Meaning, the pg_dump tap test passed, but there was no
test for the new SET STATISTICS yet. So you might want to add a regression
test for that and integrate it in the existing alter_generic file.
Upon quick read-through, the syntax and docs are correct because it's similar
to the format of ALTER TABLE/INDEX... SET STATISTICS... :
ALTER [ COLUMN ] column_name SET STATISTICS integer
+ /* XXX What if the target is set to 0? Reset the statistic? */
This also makes me wonder. I haven't looked deeply into the code, but since 0 is
a valid value, I believe it should reset the stats.
After lookup though, this is how it's tested in ALTER TABLE:
/test/regress/sql/stats_ext.sql:-- Ensure things work sanely with SET STATISTICS 0
I've considered making it part of CREATE STATISTICS itself, but it seems a
bit cumbersome and we don't do it for columns either. I'm not against adding
it in the future, but at this point I don't see a need.
I agree. Perhaps that's for another patch should you decide to add it in the future.
Regards,
Kirk Jamison
On Fri, Jul 19, 2019 at 06:12:20AM +0000, Jamison, Kirk wrote:
On Tuesday, July 9, 2019, Tomas Vondra wrote:
apparently v1 of the ALTER STATISTICS patch was a bit confused about
length of the VacAttrStats array passed to statext_compute_stattarget,
causing segfaults. Attached v2 patch fixes that, and it also makes sure
we print warnings about ignored statistics only once.v3 of the patch, adding pg_dump support - it works just like when you tweak
statistics target for a column, for example. When the value stored in the
catalog is not -1, pg_dump emits a separate ALTER STATISTICS command setting
it (for the already created statistics object).Hi Tomas, I stumbled upon your patch.
According to the CF bot, your patch applies cleanly, builds successfully, and
passes make world. Meaning, the pg_dump tap test passed, but there was no
test for the new SET STATISTICS yet. So you might want to add a regression
test for that and integrate it in the existing alter_generic file.Upon quick read-through, the syntax and docs are correct because it's similar
to the format of ALTER TABLE/INDEX... SET STATISTICS... :
ALTER [ COLUMN ] column_name SET STATISTICS integer+ /* XXX What if the target is set to 0? Reset the statistic? */
This also makes me wonder. I haven't looked deeply into the code, but since 0 is
a valid value, I believe it should reset the stats.
I agree resetting the stats after setting the target to 0 seems quite
reasonable. But that's not what we do for attribute stats, because in
that case we simply skip the attribute during the future ANALYZE runs -
we don't reset the stats, we keep the existing stats. So I've done the
same thing here, and I've removed the XXX comment.
If we want to change that, I'd do it in a separate patch for both the
regular and extended stats.
After lookup though, this is how it's tested in ALTER TABLE:
/test/regress/sql/stats_ext.sql:-- Ensure things work sanely with SET STATISTICS 0
Well, yeah. But that tests we skip building the extended statistic
(because we excluded the column from the ANALYZE run).
I've considered making it part of CREATE STATISTICS itself, but it seems a
bit cumbersome and we don't do it for columns either. I'm not against adding
it in the future, but at this point I don't see a need.I agree. Perhaps that's for another patch should you decide to add it in the future.
Right.
Attached is v4 of the patch, with a couple more improvements:
1) I've renamed the if_not_exists flag to missing_ok, because that's
more consistent with the "IF EXISTS" clause in the grammar (the old flag
was kinda the exact opposite), and I've added a NOTICE about the skip.
2) I've renamed ComputeExtStatsTarget to ComputeExtStatsRows, because
that's what the function was doing anyway (computing sample size).
3) I've added a couple of regression tests to stats_ext.sql
Aside from that, I've cleaned up a couple of places and improved a bunch
of comments. Nothing huge.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
alter-statistics-v4.patchtext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/alter_statistics.sgml b/doc/src/sgml/ref/alter_statistics.sgml
index 58c7ed020d..987f9dbc6f 100644
--- a/doc/src/sgml/ref/alter_statistics.sgml
+++ b/doc/src/sgml/ref/alter_statistics.sgml
@@ -26,6 +26,7 @@ PostgreSQL documentation
ALTER STATISTICS <replaceable class="parameter">name</replaceable> OWNER TO { <replaceable class="parameter">new_owner</replaceable> | CURRENT_USER | SESSION_USER }
ALTER STATISTICS <replaceable class="parameter">name</replaceable> RENAME TO <replaceable class="parameter">new_name</replaceable>
ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET SCHEMA <replaceable class="parameter">new_schema</replaceable>
+ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET STATISTICS <replaceable class="parameter">new_target</replaceable>
</synopsis>
</refsynopsisdiv>
@@ -93,6 +94,22 @@ ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET SCHEMA <r
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">new_target</replaceable></term>
+ <listitem>
+ <para>
+ The statistic-gathering target for this statistic object for subsequent
+ <xref linkend="sql-analyze"/> operations.
+ The target can be set in the range 0 to 10000; alternatively, set it
+ to -1 to revert to using the system default statistics
+ target (<xref linkend="guc-default-statistics-target"/>).
+ For more information on the use of statistics by the
+ <productname>PostgreSQL</productname> query planner, refer to
+ <xref linkend="planner-stats"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
</refsect1>
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 8d633f2585..edbd24f5e9 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -307,7 +307,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
VacAttrStats **vacattrstats;
AnlIndexData *indexdata;
int targrows,
- numrows;
+ numrows,
+ minrows;
double totalrows,
totaldeadrows;
HeapTuple *rows;
@@ -491,6 +492,16 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
}
}
+ /*
+ * Look at extended statistic objects too, as those may define custom
+ * statistic target. So we may need to sample more rows and then build
+ * the statistics with enough detail.
+ */
+ minrows = ComputeExtStatisticsRows(onerel, attr_cnt, vacattrstats);
+
+ if (targrows < minrows)
+ targrows = minrows;
+
/*
* Acquire the sample rows
*/
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index cf406f6f96..35bb08949b 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -14,6 +14,7 @@
*/
#include "postgres.h"
+#include "access/heapam.h"
#include "access/relation.h"
#include "access/relscan.h"
#include "access/table.h"
@@ -21,6 +22,7 @@
#include "catalog/dependency.h"
#include "catalog/indexing.h"
#include "catalog/namespace.h"
+#include "catalog/objectaccess.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_statistic_ext.h"
#include "catalog/pg_statistic_ext_data.h"
@@ -29,6 +31,7 @@
#include "miscadmin.h"
#include "statistics/statistics.h"
#include "utils/builtins.h"
+#include "utils/fmgroids.h"
#include "utils/inval.h"
#include "utils/memutils.h"
#include "utils/rel.h"
@@ -336,6 +339,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_stxstattarget - 1] = Int32GetDatum(-1);
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);
@@ -414,6 +418,111 @@ CreateStatistics(CreateStatsStmt *stmt)
return myself;
}
+
+/*
+ * ALTER STATISTICS
+ */
+ObjectAddress
+AlterStatistics(AlterStatsStmt *stmt)
+{
+ Relation rel;
+ Oid stxoid;
+ HeapTuple oldtup;
+ HeapTuple newtup;
+ Datum repl_val[Natts_pg_statistic_ext];
+ bool repl_null[Natts_pg_statistic_ext];
+ bool repl_repl[Natts_pg_statistic_ext];
+ ObjectAddress address;
+ int newtarget = stmt->stxstattarget;
+
+ /* Limit target to a sane range */
+ if (newtarget < -1)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("statistics target %d is too low",
+ newtarget)));
+ }
+ else if (newtarget > 10000)
+ {
+ newtarget = 10000;
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("lowering statistics target to %d",
+ newtarget)));
+ }
+
+ /* lookup OID of the statistic object */
+ stxoid = get_statistics_object_oid(stmt->defnames, stmt->missing_ok);
+
+ /*
+ * If we got here and the OID is not valid, it means the statistics
+ * does not exist, but the command specified IF EXISTS. So report
+ * this as NOTICE and we're done.
+ */
+ if (!OidIsValid(stxoid))
+ {
+ char *schemaname;
+ char *statname;
+
+ Assert(stmt->missing_ok);
+
+ DeconstructQualifiedName(stmt->defnames, &schemaname, &statname);
+
+ if (schemaname)
+ ereport(NOTICE,
+ (errmsg("statistics object \"%s.%s\" does not exist, skipping",
+ schemaname, statname)));
+ else
+ ereport(NOTICE,
+ (errmsg("statistics object \"%s\" does not exist, skipping",
+ statname)));
+
+ return InvalidObjectAddress;
+ }
+
+ /* Search pg_statistic_ext */
+ rel = table_open(StatisticExtRelationId, RowExclusiveLock);
+
+ oldtup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(stxoid));
+
+ /* Must be owner of the existing statistic object */
+ if (!pg_statistics_object_ownercheck(stxoid, GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_STATISTIC_EXT,
+ NameListToString(stmt->defnames));
+
+ /* Build new tuple. */
+ memset(repl_val, 0, sizeof(repl_val));
+ memset(repl_null, false, sizeof(repl_null));
+ memset(repl_repl, false, sizeof(repl_repl));
+
+ /* replace the stxstattarget column */
+ repl_repl[Anum_pg_statistic_ext_stxstattarget - 1] = true;
+ repl_val[Anum_pg_statistic_ext_stxstattarget - 1] = Int32GetDatum(newtarget);
+
+ newtup = heap_modify_tuple(oldtup, RelationGetDescr(rel),
+ repl_val, repl_null, repl_repl);
+
+ /* Update system catalog. */
+ CatalogTupleUpdate(rel, &newtup->t_self, newtup);
+
+ InvokeObjectPostAlterHook(StatisticExtRelationId, stxoid, 0);
+
+ ObjectAddressSet(address, StatisticExtRelationId, stxoid);
+
+ /*
+ * NOTE: because we only support altering the statistic target, not the
+ * other fields, there is no need to update dependencies.
+ */
+
+ heap_freetuple(newtup);
+ ReleaseSysCache(oldtup);
+
+ table_close(rel, RowExclusiveLock);
+
+ return address;
+}
+
/*
* Guts of statistics object deletion.
*/
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 6414aded0e..44267856a9 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3493,6 +3493,18 @@ _copyCreateStatsStmt(const CreateStatsStmt *from)
return newnode;
}
+static AlterStatsStmt *
+_copyAlterStatsStmt(const AlterStatsStmt *from)
+{
+ AlterStatsStmt *newnode = makeNode(AlterStatsStmt);
+
+ COPY_NODE_FIELD(defnames);
+ COPY_SCALAR_FIELD(stxstattarget);
+ COPY_SCALAR_FIELD(missing_ok);
+
+ return newnode;
+}
+
static CreateFunctionStmt *
_copyCreateFunctionStmt(const CreateFunctionStmt *from)
{
@@ -5207,6 +5219,9 @@ copyObjectImpl(const void *from)
case T_CreateStatsStmt:
retval = _copyCreateStatsStmt(from);
break;
+ case T_AlterStatsStmt:
+ retval = _copyAlterStatsStmt(from);
+ break;
case T_CreateFunctionStmt:
retval = _copyCreateFunctionStmt(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 4f2ebe5118..18cb014373 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1365,6 +1365,16 @@ _equalCreateStatsStmt(const CreateStatsStmt *a, const CreateStatsStmt *b)
return true;
}
+static bool
+_equalAlterStatsStmt(const AlterStatsStmt *a, const AlterStatsStmt *b)
+{
+ COMPARE_NODE_FIELD(defnames);
+ COMPARE_SCALAR_FIELD(stxstattarget);
+ COMPARE_SCALAR_FIELD(missing_ok);
+
+ return true;
+}
+
static bool
_equalCreateFunctionStmt(const CreateFunctionStmt *a, const CreateFunctionStmt *b)
{
@@ -3309,6 +3319,9 @@ equal(const void *a, const void *b)
case T_CreateStatsStmt:
retval = _equalCreateStatsStmt(a, b);
break;
+ case T_AlterStatsStmt:
+ retval = _equalAlterStatsStmt(a, b);
+ break;
case T_CreateFunctionStmt:
retval = _equalCreateFunctionStmt(a, b);
break;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 8e31fae47f..1394bdaaa2 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2662,6 +2662,16 @@ _outCreateStatsStmt(StringInfo str, const CreateStatsStmt *node)
WRITE_BOOL_FIELD(if_not_exists);
}
+static void
+_outAlterStatsStmt(StringInfo str, const AlterStatsStmt *node)
+{
+ WRITE_NODE_TYPE("ALTERSTATSSTMT");
+
+ WRITE_NODE_FIELD(defnames);
+ WRITE_INT_FIELD(stxstattarget);
+ WRITE_BOOL_FIELD(missing_ok);
+}
+
static void
_outNotifyStmt(StringInfo str, const NotifyStmt *node)
{
@@ -4124,6 +4134,9 @@ outNode(StringInfo str, const void *obj)
case T_CreateStatsStmt:
_outCreateStatsStmt(str, obj);
break;
+ case T_AlterStatsStmt:
+ _outAlterStatsStmt(str, obj);
+ break;
case T_NotifyStmt:
_outNotifyStmt(str, obj);
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c97bb367f8..e90c5d1be3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -252,7 +252,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
AlterOperatorStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
AlterCompositeTypeStmt AlterUserMappingStmt
- AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt
+ AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt AlterStatsStmt
AlterDefaultPrivilegesStmt DefACLAction
AnalyzeStmt CallStmt ClosePortalStmt ClusterStmt CommentStmt
ConstraintsSetStmt CopyStmt CreateAsStmt CreateCastStmt
@@ -852,6 +852,7 @@ stmt :
| AlterRoleSetStmt
| AlterRoleStmt
| AlterSubscriptionStmt
+ | AlterStatsStmt
| AlterTSConfigurationStmt
| AlterTSDictionaryStmt
| AlterUserMappingStmt
@@ -3984,6 +3985,34 @@ CreateStatsStmt:
}
;
+
+/*****************************************************************************
+ *
+ * QUERY :
+ * ALTER STATISTICS [IF EXISTS] stats_name
+ * SET STATISTICS <SignedIconst>
+ *
+ *****************************************************************************/
+
+AlterStatsStmt:
+ ALTER STATISTICS any_name SET STATISTICS SignedIconst
+ {
+ AlterStatsStmt *n = makeNode(AlterStatsStmt);
+ n->defnames = $3;
+ n->missing_ok = false;
+ n->stxstattarget = $6;
+ $$ = (Node *)n;
+ }
+ | ALTER STATISTICS IF_P EXISTS any_name SET STATISTICS SignedIconst
+ {
+ AlterStatsStmt *n = makeNode(AlterStatsStmt);
+ n->defnames = $5;
+ n->missing_ok = true;
+ n->stxstattarget = $8;
+ $$ = (Node *)n;
+ }
+ ;
+
/*****************************************************************************
*
* QUERY :
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index d2346cbe02..4288d6c6a5 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -61,6 +61,7 @@ typedef struct StatExtEntry
char *name; /* statistics object's name */
Bitmapset *columns; /* attribute numbers covered by the object */
List *types; /* 'char' list of enabled statistic kinds */
+ int stattarget; /* statistic target (-1 for default) */
} StatExtEntry;
@@ -70,7 +71,8 @@ static VacAttrStats **lookup_var_attr_stats(Relation rel, Bitmapset *attrs,
static void statext_store(Oid relid,
MVNDistinct *ndistinct, MVDependencies *dependencies,
MCVList *mcv, VacAttrStats **stats);
-
+static int statext_compute_stattarget(int stattarget,
+ int natts, VacAttrStats **stats);
/*
* Compute requested extended stats, using the rows sampled for the plain
@@ -106,6 +108,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
MCVList *mcv = NULL;
VacAttrStats **stats;
ListCell *lc2;
+ int stattarget;
/*
* Check if we can build these stats based on the column analyzed. If
@@ -130,6 +133,19 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
Assert(bms_num_members(stat->columns) >= 2 &&
bms_num_members(stat->columns) <= STATS_MAX_DIMENSIONS);
+ /* compute current statistic target */
+ stattarget = statext_compute_stattarget(stat->stattarget,
+ bms_num_members(stat->columns),
+ stats);
+
+ /*
+ * Don't rebuild statistic objects with statistic target set to 0 (we
+ * just leave the existing values around, just like we do for regular
+ * per-column statistics).
+ */
+ if (stattarget == 0)
+ continue;
+
/* compute statistic of each requested type */
foreach(lc2, stat->types)
{
@@ -143,7 +159,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
stat->columns, stats);
else if (t == STATS_EXT_MCV)
mcv = statext_mcv_build(numrows, rows, stat->columns, stats,
- totalrows);
+ totalrows, stattarget);
}
/* store the statistics in the catalog */
@@ -156,6 +172,134 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
MemoryContextDelete(cxt);
}
+/*
+ * ComputeExtStatisticsRows
+ * Compute number of rows required by extended statistics on a table.
+ *
+ * Computes number of rows we need to sample to build extended statistics on a
+ * table. This only looks at statistics we can actually build - for example
+ * when analyzing only some of the columns, this will skip statistics objects
+ * that would require additional columns.
+ *
+ * See statext_compute_stattarget for details about how we compute statistics
+ * target for a statistics objects (from the object target, attribute targets
+ * and default statistics target).
+ */
+int
+ComputeExtStatisticsRows(Relation onerel,
+ int natts, VacAttrStats **vacattrstats)
+{
+ Relation pg_stext;
+ ListCell *lc;
+ List *lstats;
+ MemoryContext cxt;
+ MemoryContext oldcxt;
+ int result = 0;
+
+ cxt = AllocSetContextCreate(CurrentMemoryContext,
+ "ComputeExtStatisticsRows",
+ ALLOCSET_DEFAULT_SIZES);
+ oldcxt = MemoryContextSwitchTo(cxt);
+
+ pg_stext = table_open(StatisticExtRelationId, RowExclusiveLock);
+ lstats = fetch_statentries_for_relation(pg_stext, RelationGetRelid(onerel));
+
+ foreach(lc, lstats)
+ {
+ StatExtEntry *stat = (StatExtEntry *) lfirst(lc);
+ int stattarget = stat->stattarget;
+ VacAttrStats **stats;
+ int nattrs = bms_num_members(stat->columns);
+
+ /*
+ * Check if we can build this statistics object based on the columns
+ * analyzed. If not, ignore it (don't report anything, we'll do that
+ * during the actual build BuildRelationExtStatistics).
+ */
+ stats = lookup_var_attr_stats(onerel, stat->columns,
+ natts, vacattrstats);
+
+ if (!stats)
+ continue;
+
+ /*
+ * Compute statistic target, based on what's set for the statistic
+ * object itself, and for it's attributes.
+ */
+ stattarget = statext_compute_stattarget(stat->stattarget,
+ nattrs, stats);
+
+ /* Use the largest value for all statistic objects. */
+ if (stattarget > result)
+ result = stattarget;
+ }
+
+ table_close(pg_stext, RowExclusiveLock);
+
+ MemoryContextSwitchTo(oldcxt);
+ MemoryContextDelete(cxt);
+
+ /* compute sample size based on the statistic target */
+ return (300 * result);
+}
+
+/*
+ * statext_compute_stattarget
+ * compute statistic target for an extended statistic
+ *
+ * When computing target for extended statistic objects, we consider three
+ * places where the target may be set - the statistic object itself, for
+ * attributes it's defined on, and finallly the default statistic target.
+ *
+ * First we look at what's set for the statistic object itself, using the
+ * ALTER STATISTICS ... SET STATISTICS command. If we find a valid target
+ * value there (i.e. not -1) we're done. Otherwise we look at targets set
+ * for any of the attributes the statistic is defined on, and if there are
+ * columns with defined target, we use the maximum value. We do this mostly
+ * for backwards compatibility, because this is what we did before having
+ * statistics target for extended statistics.
+ *
+ * And finally, if we still don't have statistics target, we use the value
+ * set in default_statistics_target.
+ */
+static int
+statext_compute_stattarget(int stattarget, int nattrs, VacAttrStats **stats)
+{
+ int i;
+
+ /*
+ * If there's statistic target set for the statistic object, use it.
+ * It may be set to 0 which disables building of that statistic.
+ */
+ if (stattarget >= 0)
+ return stattarget;
+
+ /*
+ * The target for the statistic object is set to -1, in which case we
+ * look at the maximum target set for any of the attributes the object
+ * is defined on.
+ */
+ for (i = 0; i < nattrs; i++)
+ {
+ /* keep the maximmum statistic target */
+ if (stats[i]->attr->attstattarget > stattarget)
+ stattarget = stats[i]->attr->attstattarget;
+ }
+
+ /*
+ * If the value is still negative (so neither the statistic object nor
+ * any of the columns have custom statistic target set), use the global
+ * default target.
+ */
+ if (stattarget < 0)
+ stattarget = default_statistics_target;
+
+ /* As this point we should have a valid statistic target. */
+ Assert((stattarget >= 0) && (stattarget <= 10000));
+
+ return stattarget;
+}
+
/*
* statext_is_kind_built
* Is this stat kind built in the given pg_statistic_ext_data tuple?
@@ -224,6 +368,7 @@ fetch_statentries_for_relation(Relation pg_statext, Oid relid)
entry->statOid = staForm->oid;
entry->schema = get_namespace_name(staForm->stxnamespace);
entry->name = pstrdup(NameStr(staForm->stxname));
+ entry->stattarget = staForm->stxstattarget;
for (i = 0; i < staForm->stxkeys.dim1; i++)
{
entry->columns = bms_add_member(entry->columns,
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index e5a4e86c5d..ff68b9f6c9 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -180,7 +180,8 @@ get_mincount_for_mcv_list(int samplerows, double totalrows)
*/
MCVList *
statext_mcv_build(int numrows, HeapTuple *rows, Bitmapset *attrs,
- VacAttrStats **stats, double totalrows)
+ VacAttrStats **stats, double totalrows,
+ int stattarget)
{
int i,
numattrs,
@@ -212,12 +213,7 @@ statext_mcv_build(int numrows, HeapTuple *rows, Bitmapset *attrs,
* Maximum number of MCV items to store, based on the attribute with the
* largest stats target (and the number of groups we have available).
*/
- nitems = stats[0]->attr->attstattarget;
- for (i = 1; i < numattrs; i++)
- {
- if (stats[i]->attr->attstattarget > nitems)
- nitems = stats[i]->attr->attstattarget;
- }
+ nitems = stattarget;
if (nitems > ngroups)
nitems = ngroups;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 7f6f0b6498..c6faa6619d 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1688,6 +1688,10 @@ ProcessUtilitySlow(ParseState *pstate,
address = CreateStatistics((CreateStatsStmt *) parsetree);
break;
+ case T_AlterStatsStmt:
+ address = AlterStatistics((AlterStatsStmt *) parsetree);
+ break;
+
case T_AlterCollationStmt:
address = AlterCollation((AlterCollationStmt *) parsetree);
break;
@@ -2805,6 +2809,10 @@ CreateCommandTag(Node *parsetree)
tag = "CREATE STATISTICS";
break;
+ case T_AlterStatsStmt:
+ tag = "ALTER STATISTICS";
+ break;
+
case T_DeallocateStmt:
{
DeallocateStmt *stmt = (DeallocateStmt *) parsetree;
@@ -3393,6 +3401,10 @@ GetCommandLogLevel(Node *parsetree)
lev = LOGSTMT_DDL;
break;
+ case T_AlterStatsStmt:
+ lev = LOGSTMT_DDL;
+ break;
+
case T_AlterCollationStmt:
lev = LOGSTMT_DDL;
break;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index bbf44a1820..93798713b4 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7170,6 +7170,7 @@ getExtendedStatistics(Archive *fout)
int i_stxname;
int i_stxnamespace;
int i_rolname;
+ int i_stattarget;
int i;
/* Extended statistics were new in v10 */
@@ -7178,10 +7179,16 @@ getExtendedStatistics(Archive *fout)
query = createPQExpBuffer();
- appendPQExpBuffer(query, "SELECT tableoid, oid, stxname, "
- "stxnamespace, (%s stxowner) AS rolname "
- "FROM pg_catalog.pg_statistic_ext",
- username_subquery);
+ if (fout->remoteVersion < 130000)
+ appendPQExpBuffer(query, "SELECT tableoid, oid, stxname, "
+ "stxnamespace, (%s stxowner) AS rolname, (-1) AS stxstattarget "
+ "FROM pg_catalog.pg_statistic_ext",
+ username_subquery);
+ else
+ appendPQExpBuffer(query, "SELECT tableoid, oid, stxname, "
+ "stxnamespace, (%s stxowner) AS rolname, stxstattarget "
+ "FROM pg_catalog.pg_statistic_ext",
+ username_subquery);
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -7192,6 +7199,7 @@ getExtendedStatistics(Archive *fout)
i_stxname = PQfnumber(res, "stxname");
i_stxnamespace = PQfnumber(res, "stxnamespace");
i_rolname = PQfnumber(res, "rolname");
+ i_stattarget = PQfnumber(res, "stxstattarget");
statsextinfo = (StatsExtInfo *) pg_malloc(ntups * sizeof(StatsExtInfo));
@@ -7206,7 +7214,8 @@ getExtendedStatistics(Archive *fout)
findNamespace(fout,
atooid(PQgetvalue(res, i, i_stxnamespace)));
statsextinfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
-
+ statsextinfo[i].stattarget = atoi(PQgetvalue(res, i, i_stattarget));
+
/* Decide whether we want to dump it */
selectDumpableObject(&(statsextinfo[i].dobj), fout);
@@ -16493,6 +16502,19 @@ dumpStatisticsExt(Archive *fout, StatsExtInfo *statsextinfo)
/* Result of pg_get_statisticsobjdef is complete except for semicolon */
appendPQExpBuffer(q, "%s;\n", stxdef);
+ /*
+ * We only issue an ALTER STATISTICS statement if the stxstattarget entry
+ * for this statictics object is non-negative (i.e. it's not the default
+ * value).
+ */
+ if (statsextinfo->stattarget >= 0)
+ {
+ appendPQExpBuffer(q, "ALTER STATISTICS %s ",
+ fmtQualifiedDumpable(statsextinfo));
+ appendPQExpBuffer(q, "SET STATISTICS %d;\n",
+ statsextinfo->stattarget);
+ }
+
appendPQExpBuffer(delq, "DROP STATISTICS %s;\n",
fmtQualifiedDumpable(statsextinfo));
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index ccf2153fac..997d6ecb3a 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -386,6 +386,7 @@ typedef struct _statsExtInfo
{
DumpableObject dobj;
char *rolname; /* name of owner, or empty string */
+ int stattarget; /* statistic target */
} StatsExtInfo;
typedef struct _ruleInfo
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index c56bf00e4b..7617f35a4f 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2504,6 +2504,17 @@ my %tests = (
unlike => { exclude_dump_test_schema => 1, },
},
+ 'ALTER STATISTICS extended_stats_options' => {
+ create_order => 98,
+ create_sql => 'ALTER STATISTICS dump_test.test_ext_stats_opts SET STATISTICS 1000',
+ regexp => qr/^
+ \QALTER STATISTICS dump_test.test_ext_stats_opts SET STATISTICS 1000;\E
+ /xms,
+ like =>
+ { %full_runs, %dump_test_schema_runs, section_post_data => 1, },
+ unlike => { exclude_dump_test_schema => 1, },
+ },
+
'CREATE SEQUENCE test_table_col1_seq' => {
regexp => qr/^
\QCREATE SEQUENCE dump_test.test_table_col1_seq\E
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 3f7001fb69..cebdd7b2a0 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1840,7 +1840,7 @@ psql_completion(const char *text, int start, int end)
/* ALTER STATISTICS <name> */
else if (Matches("ALTER", "STATISTICS", MatchAny))
- COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA");
+ COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA", "SET STATISTICS");
/* ALTER TRIGGER <name>, add ON */
else if (Matches("ALTER", "TRIGGER", MatchAny))
diff --git a/src/include/catalog/pg_statistic_ext.h b/src/include/catalog/pg_statistic_ext.h
index d8c5e0651e..5f6bfdd56c 100644
--- a/src/include/catalog/pg_statistic_ext.h
+++ b/src/include/catalog/pg_statistic_ext.h
@@ -41,6 +41,7 @@ CATALOG(pg_statistic_ext,3381,StatisticExtRelationId)
Oid stxnamespace; /* OID of statistics object's namespace */
Oid stxowner; /* statistics object's owner */
+ int32 stxstattarget BKI_DEFAULT(-1); /* statistic target */
/*
* variable-length fields start here, but we allow direct access to
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index b4e7db67c3..1dc6dc2ca0 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -87,6 +87,7 @@ extern ObjectAddress AlterOperator(AlterOperatorStmt *stmt);
/* commands/statscmds.c */
extern ObjectAddress CreateStatistics(CreateStatsStmt *stmt);
+extern ObjectAddress AlterStatistics(AlterStatsStmt *stmt);
extern void RemoveStatisticsById(Oid statsOid);
extern void UpdateStatisticsForTypeChange(Oid statsOid,
Oid relationOid, int attnum,
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 4e2fb39105..57589d4fac 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -420,6 +420,7 @@ typedef enum NodeTag
T_CreateStatsStmt,
T_AlterCollationStmt,
T_CallStmt,
+ T_AlterStatsStmt,
/*
* TAGS FOR PARSE TREE NODES (parsenodes.h)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 94ded3c135..fdacc54cb8 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2789,6 +2789,18 @@ typedef struct CreateStatsStmt
bool if_not_exists; /* do nothing if stats name already exists */
} CreateStatsStmt;
+/* ----------------------
+ * Alter Statistics Statement
+ * ----------------------
+ */
+typedef struct AlterStatsStmt
+{
+ NodeTag type;
+ List *defnames; /* qualified name (list of Value strings) */
+ int stxstattarget; /* statistics target, or NULL */
+ bool missing_ok; /* do nothing if statistics does not exist */
+} AlterStatsStmt;
+
/* ----------------------
* Create Function Statement
* ----------------------
diff --git a/src/include/statistics/extended_stats_internal.h b/src/include/statistics/extended_stats_internal.h
index c7f01d4edc..56c5599312 100644
--- a/src/include/statistics/extended_stats_internal.h
+++ b/src/include/statistics/extended_stats_internal.h
@@ -71,7 +71,7 @@ extern MVDependencies *statext_dependencies_deserialize(bytea *data);
extern MCVList *statext_mcv_build(int numrows, HeapTuple *rows,
Bitmapset *attrs, VacAttrStats **stats,
- double totalrows);
+ double totalrows, int stattarget);
extern bytea *statext_mcv_serialize(MCVList *mcv, VacAttrStats **stats);
extern MCVList *statext_mcv_deserialize(bytea *data);
diff --git a/src/include/statistics/statistics.h b/src/include/statistics/statistics.h
index cb7bc630e9..33abb22e44 100644
--- a/src/include/statistics/statistics.h
+++ b/src/include/statistics/statistics.h
@@ -100,6 +100,8 @@ extern MCVList *statext_mcv_load(Oid mvoid);
extern void BuildRelationExtStatistics(Relation onerel, double totalrows,
int numrows, HeapTuple *rows,
int natts, VacAttrStats **vacattrstats);
+extern int ComputeExtStatisticsRows(Relation onerel,
+ int natts, VacAttrStats **stats);
extern bool statext_is_kind_built(HeapTuple htup, char kind);
extern Selectivity dependencies_clauselist_selectivity(PlannerInfo *root,
List *clauses,
diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index 94b8a8f8b8..f4534e3f7c 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -98,11 +98,28 @@ CREATE STATISTICS ab1_a_b_stats ON a, b FROM ab1;
ANALYZE ab1;
WARNING: statistics object "public.ab1_a_b_stats" could not be computed for relation "public.ab1"
ALTER TABLE ab1 ALTER a SET STATISTICS -1;
+-- setting statistics target 0 skips the statistics, without printing any message, so check catalog
+ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0;
+ANALYZE ab1;
+SELECT stxname, stxdndistinct, stxddependencies, stxdmcv
+ FROM pg_statistic_ext s, pg_statistic_ext_data d
+ WHERE s.stxname = 'ab1_a_b_stats'
+ AND d.stxoid = s.oid;
+ stxname | stxdndistinct | stxddependencies | stxdmcv
+---------------+---------------+------------------+---------
+ ab1_a_b_stats | | |
+(1 row)
+
+ALTER STATISTICS ab1_a_b_stats SET STATISTICS -1;
-- partial analyze doesn't build stats either
ANALYZE ab1 (a);
WARNING: statistics object "public.ab1_a_b_stats" could not be computed for relation "public.ab1"
ANALYZE ab1;
DROP TABLE ab1;
+ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0;
+ERROR: statistics object "ab1_a_b_stats" does not exist
+ALTER STATISTICS IF EXISTS ab1_a_b_stats SET STATISTICS 0;
+NOTICE: statistics object "ab1_a_b_stats" does not exist, skipping
-- Verify supported object types for extended statistics
CREATE schema tststats;
CREATE TABLE tststats.t (a int, b int, c text);
diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index 4bc1536727..231d7cac83 100644
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -68,10 +68,20 @@ INSERT INTO ab1 SELECT a, a%23 FROM generate_series(1, 1000) a;
CREATE STATISTICS ab1_a_b_stats ON a, b FROM ab1;
ANALYZE ab1;
ALTER TABLE ab1 ALTER a SET STATISTICS -1;
+-- setting statistics target 0 skips the statistics, without printing any message, so check catalog
+ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0;
+ANALYZE ab1;
+SELECT stxname, stxdndistinct, stxddependencies, stxdmcv
+ FROM pg_statistic_ext s, pg_statistic_ext_data d
+ WHERE s.stxname = 'ab1_a_b_stats'
+ AND d.stxoid = s.oid;
+ALTER STATISTICS ab1_a_b_stats SET STATISTICS -1;
-- partial analyze doesn't build stats either
ANALYZE ab1 (a);
ANALYZE ab1;
DROP TABLE ab1;
+ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0;
+ALTER STATISTICS IF EXISTS ab1_a_b_stats SET STATISTICS 0;
-- Verify supported object types for extended statistics
CREATE schema tststats;
On Sat, July 20, 2019 8:12 AM (GMT+9), Tomas Vondra wrote:
+ /* XXX What if the target is set to 0? Reset the statistic?
*/
This also makes me wonder. I haven't looked deeply into the code, but
since 0 is a valid value, I believe it should reset the stats.I agree resetting the stats after setting the target to 0 seems quite
reasonable. But that's not what we do for attribute stats, because in that
case we simply skip the attribute during the future ANALYZE runs - we don't
reset the stats, we keep the existing stats. So I've done the same thing here,
and I've removed the XXX comment.If we want to change that, I'd do it in a separate patch for both the regular
and extended stats.
Hi, Tomas
Sorry for my late reply.
You're right. I have no strong opinion whether we'd want to change that behavior.
I've also confirmed the change in the patch where setting statistics target 0
skips the statistics.
Maybe only some minor nitpicks in the source code comments below:
1. "it's" should be "its":
+ * Compute statistic target, based on what's set for the statistic + * object itself, and for it's attributes.
2. Consistency whether you'd use either "statistic " or "statisticS ".
Ex. statistic target vs statisticS target, statistics object vs statistic object, etc.
Attached is v4 of the patch, with a couple more improvements:
1) I've renamed the if_not_exists flag to missing_ok, because that's more
consistent with the "IF EXISTS" clause in the grammar (the old flag was kinda
the exact opposite), and I've added a NOTICE about the skip.
+ bool missing_ok; /* do nothing if statistics does not exist */
Confirmed. So we ignore if statistic does not exist, and skip the error.
Maybe to make it consistent with other data structures in parsernodes.h,
you can change the comment of missing_ok to:
/* skip error if statistics object does not exist */
2) I've renamed ComputeExtStatsTarget to ComputeExtStatsRows, because that's
what the function was doing anyway (computing sample size).3) I've added a couple of regression tests to stats_ext.sql
Aside from that, I've cleaned up a couple of places and improved a bunch of
comments. Nothing huge.
I have a question though regarding ComputeExtStatisticsRows.
I'm just curious with the value 300 when computing sample size.
Where did this value come from?
+ /* compute sample size based on the statistic target */
+ return (300 * result);Overall, the patch is almost already in good shape for commit.
I'll wait for the next update.
Regards,
Kirk Jamison
On Fri, Jul 26, 2019 at 07:03:41AM +0000, Jamison, Kirk wrote:
On Sat, July 20, 2019 8:12 AM (GMT+9), Tomas Vondra wrote:
+ /* XXX What if the target is set to 0? Reset the statistic?
*/
This also makes me wonder. I haven't looked deeply into the code, but
since 0 is a valid value, I believe it should reset the stats.I agree resetting the stats after setting the target to 0 seems quite
reasonable. But that's not what we do for attribute stats, because in that
case we simply skip the attribute during the future ANALYZE runs - we don't
reset the stats, we keep the existing stats. So I've done the same thing here,
and I've removed the XXX comment.If we want to change that, I'd do it in a separate patch for both the regular
and extended stats.Hi, Tomas
Sorry for my late reply.
You're right. I have no strong opinion whether we'd want to change that behavior.
I've also confirmed the change in the patch where setting statistics target 0
skips the statistics.
OK, thanks.
Maybe only some minor nitpicks in the source code comments below:
1. "it's" should be "its":+ * Compute statistic target, based on what's set for the statistic + * object itself, and for it's attributes.2. Consistency whether you'd use either "statistic " or "statisticS ".
Ex. statistic target vs statisticS target, statistics object vs statistic object, etc.Attached is v4 of the patch, with a couple more improvements:
1) I've renamed the if_not_exists flag to missing_ok, because that's more
consistent with the "IF EXISTS" clause in the grammar (the old flag was kinda
the exact opposite), and I've added a NOTICE about the skip.+ bool missing_ok; /* do nothing if statistics does not exist */
Confirmed. So we ignore if statistic does not exist, and skip the error.
Maybe to make it consistent with other data structures in parsernodes.h,
you can change the comment of missing_ok to:
/* skip error if statistics object does not exist */
Thanks, I've fixed all those places in the attached v5.
2) I've renamed ComputeExtStatsTarget to ComputeExtStatsRows, because that's
what the function was doing anyway (computing sample size).3) I've added a couple of regression tests to stats_ext.sql
Aside from that, I've cleaned up a couple of places and improved a bunch of
comments. Nothing huge.I have a question though regarding ComputeExtStatisticsRows.
I'm just curious with the value 300 when computing sample size.
Where did this value come from?+ /* compute sample size based on the statistic target */ + return (300 * result);Overall, the patch is almost already in good shape for commit.
I'll wait for the next update.
That's how we compute number of rows to sample, based on the statistics
target. See std_typanalyze() in analyze.c, which also cites the paper
where this comes from.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
alter-statistics-v5.patchtext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/ref/alter_statistics.sgml b/doc/src/sgml/ref/alter_statistics.sgml
index 58c7ed020d..be4c3f1f05 100644
--- a/doc/src/sgml/ref/alter_statistics.sgml
+++ b/doc/src/sgml/ref/alter_statistics.sgml
@@ -26,6 +26,7 @@ PostgreSQL documentation
ALTER STATISTICS <replaceable class="parameter">name</replaceable> OWNER TO { <replaceable class="parameter">new_owner</replaceable> | CURRENT_USER | SESSION_USER }
ALTER STATISTICS <replaceable class="parameter">name</replaceable> RENAME TO <replaceable class="parameter">new_name</replaceable>
ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET SCHEMA <replaceable class="parameter">new_schema</replaceable>
+ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET STATISTICS <replaceable class="parameter">new_target</replaceable>
</synopsis>
</refsynopsisdiv>
@@ -93,6 +94,22 @@ ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET SCHEMA <r
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">new_target</replaceable></term>
+ <listitem>
+ <para>
+ The statistic-gathering target for this statistics object for subsequent
+ <xref linkend="sql-analyze"/> operations.
+ The target can be set in the range 0 to 10000; alternatively, set it
+ to -1 to revert to using the system default statistics
+ target (<xref linkend="guc-default-statistics-target"/>).
+ For more information on the use of statistics by the
+ <productname>PostgreSQL</productname> query planner, refer to
+ <xref linkend="planner-stats"/>.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
</refsect1>
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 8d633f2585..c27df32d92 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -307,7 +307,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
VacAttrStats **vacattrstats;
AnlIndexData *indexdata;
int targrows,
- numrows;
+ numrows,
+ minrows;
double totalrows,
totaldeadrows;
HeapTuple *rows;
@@ -491,6 +492,16 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
}
}
+ /*
+ * Look at extended statistics objects too, as those may define custom
+ * statistics target. So we may need to sample more rows and then build
+ * the statistics with enough detail.
+ */
+ minrows = ComputeExtStatisticsRows(onerel, attr_cnt, vacattrstats);
+
+ if (targrows < minrows)
+ targrows = minrows;
+
/*
* Acquire the sample rows
*/
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 34d11c2a98..ba71029ef0 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -14,6 +14,7 @@
*/
#include "postgres.h"
+#include "access/heapam.h"
#include "access/relation.h"
#include "access/relscan.h"
#include "access/table.h"
@@ -21,6 +22,7 @@
#include "catalog/dependency.h"
#include "catalog/indexing.h"
#include "catalog/namespace.h"
+#include "catalog/objectaccess.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_statistic_ext.h"
#include "catalog/pg_statistic_ext_data.h"
@@ -29,6 +31,7 @@
#include "miscadmin.h"
#include "statistics/statistics.h"
#include "utils/builtins.h"
+#include "utils/fmgroids.h"
#include "utils/inval.h"
#include "utils/memutils.h"
#include "utils/rel.h"
@@ -336,6 +339,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_stxstattarget - 1] = Int32GetDatum(-1);
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);
@@ -414,6 +418,111 @@ CreateStatistics(CreateStatsStmt *stmt)
return myself;
}
+
+/*
+ * ALTER STATISTICS
+ */
+ObjectAddress
+AlterStatistics(AlterStatsStmt *stmt)
+{
+ Relation rel;
+ Oid stxoid;
+ HeapTuple oldtup;
+ HeapTuple newtup;
+ Datum repl_val[Natts_pg_statistic_ext];
+ bool repl_null[Natts_pg_statistic_ext];
+ bool repl_repl[Natts_pg_statistic_ext];
+ ObjectAddress address;
+ int newtarget = stmt->stxstattarget;
+
+ /* Limit target to a sane range */
+ if (newtarget < -1)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("statistics target %d is too low",
+ newtarget)));
+ }
+ else if (newtarget > 10000)
+ {
+ newtarget = 10000;
+ ereport(WARNING,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("lowering statistics target to %d",
+ newtarget)));
+ }
+
+ /* lookup OID of the statistics object */
+ stxoid = get_statistics_object_oid(stmt->defnames, stmt->missing_ok);
+
+ /*
+ * If we got here and the OID is not valid, it means the statistics
+ * does not exist, but the command specified IF EXISTS. So report
+ * this as NOTICE and we're done.
+ */
+ if (!OidIsValid(stxoid))
+ {
+ char *schemaname;
+ char *statname;
+
+ Assert(stmt->missing_ok);
+
+ DeconstructQualifiedName(stmt->defnames, &schemaname, &statname);
+
+ if (schemaname)
+ ereport(NOTICE,
+ (errmsg("statistics object \"%s.%s\" does not exist, skipping",
+ schemaname, statname)));
+ else
+ ereport(NOTICE,
+ (errmsg("statistics object \"%s\" does not exist, skipping",
+ statname)));
+
+ return InvalidObjectAddress;
+ }
+
+ /* Search pg_statistic_ext */
+ rel = table_open(StatisticExtRelationId, RowExclusiveLock);
+
+ oldtup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(stxoid));
+
+ /* Must be owner of the existing statistics object */
+ if (!pg_statistics_object_ownercheck(stxoid, GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_STATISTIC_EXT,
+ NameListToString(stmt->defnames));
+
+ /* Build new tuple. */
+ memset(repl_val, 0, sizeof(repl_val));
+ memset(repl_null, false, sizeof(repl_null));
+ memset(repl_repl, false, sizeof(repl_repl));
+
+ /* replace the stxstattarget column */
+ repl_repl[Anum_pg_statistic_ext_stxstattarget - 1] = true;
+ repl_val[Anum_pg_statistic_ext_stxstattarget - 1] = Int32GetDatum(newtarget);
+
+ newtup = heap_modify_tuple(oldtup, RelationGetDescr(rel),
+ repl_val, repl_null, repl_repl);
+
+ /* Update system catalog. */
+ CatalogTupleUpdate(rel, &newtup->t_self, newtup);
+
+ InvokeObjectPostAlterHook(StatisticExtRelationId, stxoid, 0);
+
+ ObjectAddressSet(address, StatisticExtRelationId, stxoid);
+
+ /*
+ * NOTE: because we only support altering the statistics target, not the
+ * other fields, there is no need to update dependencies.
+ */
+
+ heap_freetuple(newtup);
+ ReleaseSysCache(oldtup);
+
+ table_close(rel, RowExclusiveLock);
+
+ return address;
+}
+
/*
* Guts of statistics object deletion.
*/
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 6414aded0e..44267856a9 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3493,6 +3493,18 @@ _copyCreateStatsStmt(const CreateStatsStmt *from)
return newnode;
}
+static AlterStatsStmt *
+_copyAlterStatsStmt(const AlterStatsStmt *from)
+{
+ AlterStatsStmt *newnode = makeNode(AlterStatsStmt);
+
+ COPY_NODE_FIELD(defnames);
+ COPY_SCALAR_FIELD(stxstattarget);
+ COPY_SCALAR_FIELD(missing_ok);
+
+ return newnode;
+}
+
static CreateFunctionStmt *
_copyCreateFunctionStmt(const CreateFunctionStmt *from)
{
@@ -5207,6 +5219,9 @@ copyObjectImpl(const void *from)
case T_CreateStatsStmt:
retval = _copyCreateStatsStmt(from);
break;
+ case T_AlterStatsStmt:
+ retval = _copyAlterStatsStmt(from);
+ break;
case T_CreateFunctionStmt:
retval = _copyCreateFunctionStmt(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 4f2ebe5118..18cb014373 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1365,6 +1365,16 @@ _equalCreateStatsStmt(const CreateStatsStmt *a, const CreateStatsStmt *b)
return true;
}
+static bool
+_equalAlterStatsStmt(const AlterStatsStmt *a, const AlterStatsStmt *b)
+{
+ COMPARE_NODE_FIELD(defnames);
+ COMPARE_SCALAR_FIELD(stxstattarget);
+ COMPARE_SCALAR_FIELD(missing_ok);
+
+ return true;
+}
+
static bool
_equalCreateFunctionStmt(const CreateFunctionStmt *a, const CreateFunctionStmt *b)
{
@@ -3309,6 +3319,9 @@ equal(const void *a, const void *b)
case T_CreateStatsStmt:
retval = _equalCreateStatsStmt(a, b);
break;
+ case T_AlterStatsStmt:
+ retval = _equalAlterStatsStmt(a, b);
+ break;
case T_CreateFunctionStmt:
retval = _equalCreateFunctionStmt(a, b);
break;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 86c31a48c9..87d51b5c13 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2664,6 +2664,16 @@ _outCreateStatsStmt(StringInfo str, const CreateStatsStmt *node)
WRITE_BOOL_FIELD(if_not_exists);
}
+static void
+_outAlterStatsStmt(StringInfo str, const AlterStatsStmt *node)
+{
+ WRITE_NODE_TYPE("ALTERSTATSSTMT");
+
+ WRITE_NODE_FIELD(defnames);
+ WRITE_INT_FIELD(stxstattarget);
+ WRITE_BOOL_FIELD(missing_ok);
+}
+
static void
_outNotifyStmt(StringInfo str, const NotifyStmt *node)
{
@@ -4126,6 +4136,9 @@ outNode(StringInfo str, const void *obj)
case T_CreateStatsStmt:
_outCreateStatsStmt(str, obj);
break;
+ case T_AlterStatsStmt:
+ _outAlterStatsStmt(str, obj);
+ break;
case T_NotifyStmt:
_outNotifyStmt(str, obj);
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c97bb367f8..e90c5d1be3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -252,7 +252,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
AlterOperatorStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
AlterCompositeTypeStmt AlterUserMappingStmt
- AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt
+ AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt AlterStatsStmt
AlterDefaultPrivilegesStmt DefACLAction
AnalyzeStmt CallStmt ClosePortalStmt ClusterStmt CommentStmt
ConstraintsSetStmt CopyStmt CreateAsStmt CreateCastStmt
@@ -852,6 +852,7 @@ stmt :
| AlterRoleSetStmt
| AlterRoleStmt
| AlterSubscriptionStmt
+ | AlterStatsStmt
| AlterTSConfigurationStmt
| AlterTSDictionaryStmt
| AlterUserMappingStmt
@@ -3984,6 +3985,34 @@ CreateStatsStmt:
}
;
+
+/*****************************************************************************
+ *
+ * QUERY :
+ * ALTER STATISTICS [IF EXISTS] stats_name
+ * SET STATISTICS <SignedIconst>
+ *
+ *****************************************************************************/
+
+AlterStatsStmt:
+ ALTER STATISTICS any_name SET STATISTICS SignedIconst
+ {
+ AlterStatsStmt *n = makeNode(AlterStatsStmt);
+ n->defnames = $3;
+ n->missing_ok = false;
+ n->stxstattarget = $6;
+ $$ = (Node *)n;
+ }
+ | ALTER STATISTICS IF_P EXISTS any_name SET STATISTICS SignedIconst
+ {
+ AlterStatsStmt *n = makeNode(AlterStatsStmt);
+ n->defnames = $5;
+ n->missing_ok = true;
+ n->stxstattarget = $8;
+ $$ = (Node *)n;
+ }
+ ;
+
/*****************************************************************************
*
* QUERY :
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index 23c74f7d90..dac8b68eb7 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -61,6 +61,7 @@ typedef struct StatExtEntry
char *name; /* statistics object's name */
Bitmapset *columns; /* attribute numbers covered by the object */
List *types; /* 'char' list of enabled statistic kinds */
+ int stattarget; /* statistics target (-1 for default) */
} StatExtEntry;
@@ -70,7 +71,8 @@ static VacAttrStats **lookup_var_attr_stats(Relation rel, Bitmapset *attrs,
static void statext_store(Oid relid,
MVNDistinct *ndistinct, MVDependencies *dependencies,
MCVList *mcv, VacAttrStats **stats);
-
+static int statext_compute_stattarget(int stattarget,
+ int natts, VacAttrStats **stats);
/*
* Compute requested extended stats, using the rows sampled for the plain
@@ -106,6 +108,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
MCVList *mcv = NULL;
VacAttrStats **stats;
ListCell *lc2;
+ int stattarget;
/*
* Check if we can build these stats based on the column analyzed. If
@@ -130,6 +133,19 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
Assert(bms_num_members(stat->columns) >= 2 &&
bms_num_members(stat->columns) <= STATS_MAX_DIMENSIONS);
+ /* compute current statistics target */
+ stattarget = statext_compute_stattarget(stat->stattarget,
+ bms_num_members(stat->columns),
+ stats);
+
+ /*
+ * Don't rebuild statistics objects with statistics target set to 0 (we
+ * just leave the existing values around, just like we do for regular
+ * per-column statistics).
+ */
+ if (stattarget == 0)
+ continue;
+
/* compute statistic of each requested type */
foreach(lc2, stat->types)
{
@@ -143,7 +159,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
stat->columns, stats);
else if (t == STATS_EXT_MCV)
mcv = statext_mcv_build(numrows, rows, stat->columns, stats,
- totalrows);
+ totalrows, stattarget);
}
/* store the statistics in the catalog */
@@ -156,6 +172,134 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
MemoryContextDelete(cxt);
}
+/*
+ * ComputeExtStatisticsRows
+ * Compute number of rows required by extended statistics on a table.
+ *
+ * Computes number of rows we need to sample to build extended statistics on a
+ * table. This only looks at statistics we can actually build - for example
+ * when analyzing only some of the columns, this will skip statistics objects
+ * that would require additional columns.
+ *
+ * See statext_compute_stattarget for details about how we compute statistics
+ * target for a statistics objects (from the object target, attribute targets
+ * and default statistics target).
+ */
+int
+ComputeExtStatisticsRows(Relation onerel,
+ int natts, VacAttrStats **vacattrstats)
+{
+ Relation pg_stext;
+ ListCell *lc;
+ List *lstats;
+ MemoryContext cxt;
+ MemoryContext oldcxt;
+ int result = 0;
+
+ cxt = AllocSetContextCreate(CurrentMemoryContext,
+ "ComputeExtStatisticsRows",
+ ALLOCSET_DEFAULT_SIZES);
+ oldcxt = MemoryContextSwitchTo(cxt);
+
+ pg_stext = table_open(StatisticExtRelationId, RowExclusiveLock);
+ lstats = fetch_statentries_for_relation(pg_stext, RelationGetRelid(onerel));
+
+ foreach(lc, lstats)
+ {
+ StatExtEntry *stat = (StatExtEntry *) lfirst(lc);
+ int stattarget = stat->stattarget;
+ VacAttrStats **stats;
+ int nattrs = bms_num_members(stat->columns);
+
+ /*
+ * Check if we can build this statistics object based on the columns
+ * analyzed. If not, ignore it (don't report anything, we'll do that
+ * during the actual build BuildRelationExtStatistics).
+ */
+ stats = lookup_var_attr_stats(onerel, stat->columns,
+ natts, vacattrstats);
+
+ if (!stats)
+ continue;
+
+ /*
+ * Compute statistics target, based on what's set for the statistic
+ * object itself, and for its attributes.
+ */
+ stattarget = statext_compute_stattarget(stat->stattarget,
+ nattrs, stats);
+
+ /* Use the largest value for all statistics objects. */
+ if (stattarget > result)
+ result = stattarget;
+ }
+
+ table_close(pg_stext, RowExclusiveLock);
+
+ MemoryContextSwitchTo(oldcxt);
+ MemoryContextDelete(cxt);
+
+ /* compute sample size based on the statistics target */
+ return (300 * result);
+}
+
+/*
+ * statext_compute_stattarget
+ * compute statistics target for an extended statistic
+ *
+ * When computing target for extended statistics objects, we consider three
+ * places where the target may be set - the statistics object itself, for
+ * attributes it's defined on, and finallly the default statistics target.
+ *
+ * First we look at what's set for the statistics object itself, using the
+ * ALTER STATISTICS ... SET STATISTICS command. If we find a valid target
+ * value there (i.e. not -1) we're done. Otherwise we look at targets set
+ * for any of the attributes the statistic is defined on, and if there are
+ * columns with defined target, we use the maximum value. We do this mostly
+ * for backwards compatibility, because this is what we did before having
+ * statistics target for extended statistics.
+ *
+ * And finally, if we still don't have statistics target, we use the value
+ * set in default_statistics_target.
+ */
+static int
+statext_compute_stattarget(int stattarget, int nattrs, VacAttrStats **stats)
+{
+ int i;
+
+ /*
+ * If there's statistics target set for the statistics object, use it.
+ * It may be set to 0 which disables building of that statistic.
+ */
+ if (stattarget >= 0)
+ return stattarget;
+
+ /*
+ * The target for the statistics object is set to -1, in which case we
+ * look at the maximum target set for any of the attributes the object
+ * is defined on.
+ */
+ for (i = 0; i < nattrs; i++)
+ {
+ /* keep the maximmum statistics target */
+ if (stats[i]->attr->attstattarget > stattarget)
+ stattarget = stats[i]->attr->attstattarget;
+ }
+
+ /*
+ * If the value is still negative (so neither the statistics object nor
+ * any of the columns have custom statistics target set), use the global
+ * default target.
+ */
+ if (stattarget < 0)
+ stattarget = default_statistics_target;
+
+ /* As this point we should have a valid statistics target. */
+ Assert((stattarget >= 0) && (stattarget <= 10000));
+
+ return stattarget;
+}
+
/*
* statext_is_kind_built
* Is this stat kind built in the given pg_statistic_ext_data tuple?
@@ -224,6 +368,7 @@ fetch_statentries_for_relation(Relation pg_statext, Oid relid)
entry->statOid = staForm->oid;
entry->schema = get_namespace_name(staForm->stxnamespace);
entry->name = pstrdup(NameStr(staForm->stxname));
+ entry->stattarget = staForm->stxstattarget;
for (i = 0; i < staForm->stxkeys.dim1; i++)
{
entry->columns = bms_add_member(entry->columns,
diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c
index cec06f8c44..0c659f45fc 100644
--- a/src/backend/statistics/mcv.c
+++ b/src/backend/statistics/mcv.c
@@ -180,7 +180,8 @@ get_mincount_for_mcv_list(int samplerows, double totalrows)
*/
MCVList *
statext_mcv_build(int numrows, HeapTuple *rows, Bitmapset *attrs,
- VacAttrStats **stats, double totalrows)
+ VacAttrStats **stats, double totalrows,
+ int stattarget)
{
int i,
numattrs,
@@ -212,12 +213,7 @@ statext_mcv_build(int numrows, HeapTuple *rows, Bitmapset *attrs,
* Maximum number of MCV items to store, based on the attribute with the
* largest stats target (and the number of groups we have available).
*/
- nitems = stats[0]->attr->attstattarget;
- for (i = 1; i < numattrs; i++)
- {
- if (stats[i]->attr->attstattarget > nitems)
- nitems = stats[i]->attr->attstattarget;
- }
+ nitems = stattarget;
if (nitems > ngroups)
nitems = ngroups;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 7f6f0b6498..c6faa6619d 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1688,6 +1688,10 @@ ProcessUtilitySlow(ParseState *pstate,
address = CreateStatistics((CreateStatsStmt *) parsetree);
break;
+ case T_AlterStatsStmt:
+ address = AlterStatistics((AlterStatsStmt *) parsetree);
+ break;
+
case T_AlterCollationStmt:
address = AlterCollation((AlterCollationStmt *) parsetree);
break;
@@ -2805,6 +2809,10 @@ CreateCommandTag(Node *parsetree)
tag = "CREATE STATISTICS";
break;
+ case T_AlterStatsStmt:
+ tag = "ALTER STATISTICS";
+ break;
+
case T_DeallocateStmt:
{
DeallocateStmt *stmt = (DeallocateStmt *) parsetree;
@@ -3393,6 +3401,10 @@ GetCommandLogLevel(Node *parsetree)
lev = LOGSTMT_DDL;
break;
+ case T_AlterStatsStmt:
+ lev = LOGSTMT_DDL;
+ break;
+
case T_AlterCollationStmt:
lev = LOGSTMT_DDL;
break;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 8a31672247..8a01ac00ee 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7178,6 +7178,7 @@ getExtendedStatistics(Archive *fout)
int i_stxname;
int i_stxnamespace;
int i_rolname;
+ int i_stattarget;
int i;
/* Extended statistics were new in v10 */
@@ -7186,10 +7187,16 @@ getExtendedStatistics(Archive *fout)
query = createPQExpBuffer();
- appendPQExpBuffer(query, "SELECT tableoid, oid, stxname, "
- "stxnamespace, (%s stxowner) AS rolname "
- "FROM pg_catalog.pg_statistic_ext",
- username_subquery);
+ if (fout->remoteVersion < 130000)
+ appendPQExpBuffer(query, "SELECT tableoid, oid, stxname, "
+ "stxnamespace, (%s stxowner) AS rolname, (-1) AS stxstattarget "
+ "FROM pg_catalog.pg_statistic_ext",
+ username_subquery);
+ else
+ appendPQExpBuffer(query, "SELECT tableoid, oid, stxname, "
+ "stxnamespace, (%s stxowner) AS rolname, stxstattarget "
+ "FROM pg_catalog.pg_statistic_ext",
+ username_subquery);
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -7200,6 +7207,7 @@ getExtendedStatistics(Archive *fout)
i_stxname = PQfnumber(res, "stxname");
i_stxnamespace = PQfnumber(res, "stxnamespace");
i_rolname = PQfnumber(res, "rolname");
+ i_stattarget = PQfnumber(res, "stxstattarget");
statsextinfo = (StatsExtInfo *) pg_malloc(ntups * sizeof(StatsExtInfo));
@@ -7214,7 +7222,8 @@ getExtendedStatistics(Archive *fout)
findNamespace(fout,
atooid(PQgetvalue(res, i, i_stxnamespace)));
statsextinfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
-
+ statsextinfo[i].stattarget = atoi(PQgetvalue(res, i, i_stattarget));
+
/* Decide whether we want to dump it */
selectDumpableObject(&(statsextinfo[i].dobj), fout);
@@ -16501,6 +16510,19 @@ dumpStatisticsExt(Archive *fout, StatsExtInfo *statsextinfo)
/* Result of pg_get_statisticsobjdef is complete except for semicolon */
appendPQExpBuffer(q, "%s;\n", stxdef);
+ /*
+ * We only issue an ALTER STATISTICS statement if the stxstattarget entry
+ * for this statictics object is non-negative (i.e. it's not the default
+ * value).
+ */
+ if (statsextinfo->stattarget >= 0)
+ {
+ appendPQExpBuffer(q, "ALTER STATISTICS %s ",
+ fmtQualifiedDumpable(statsextinfo));
+ appendPQExpBuffer(q, "SET STATISTICS %d;\n",
+ statsextinfo->stattarget);
+ }
+
appendPQExpBuffer(delq, "DROP STATISTICS %s;\n",
fmtQualifiedDumpable(statsextinfo));
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index ccf2153fac..0e7c32c852 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -386,6 +386,7 @@ typedef struct _statsExtInfo
{
DumpableObject dobj;
char *rolname; /* name of owner, or empty string */
+ int stattarget; /* statistics target */
} StatsExtInfo;
typedef struct _ruleInfo
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 7cbccee103..a34fa8739b 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2513,6 +2513,17 @@ my %tests = (
unlike => { exclude_dump_test_schema => 1, },
},
+ 'ALTER STATISTICS extended_stats_options' => {
+ create_order => 98,
+ create_sql => 'ALTER STATISTICS dump_test.test_ext_stats_opts SET STATISTICS 1000',
+ regexp => qr/^
+ \QALTER STATISTICS dump_test.test_ext_stats_opts SET STATISTICS 1000;\E
+ /xms,
+ like =>
+ { %full_runs, %dump_test_schema_runs, section_post_data => 1, },
+ unlike => { exclude_dump_test_schema => 1, },
+ },
+
'CREATE SEQUENCE test_table_col1_seq' => {
regexp => qr/^
\QCREATE SEQUENCE dump_test.test_table_col1_seq\E
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 3f7001fb69..cebdd7b2a0 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1840,7 +1840,7 @@ psql_completion(const char *text, int start, int end)
/* ALTER STATISTICS <name> */
else if (Matches("ALTER", "STATISTICS", MatchAny))
- COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA");
+ COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA", "SET STATISTICS");
/* ALTER TRIGGER <name>, add ON */
else if (Matches("ALTER", "TRIGGER", MatchAny))
diff --git a/src/include/catalog/pg_statistic_ext.h b/src/include/catalog/pg_statistic_ext.h
index d8c5e0651e..54a88f4ec8 100644
--- a/src/include/catalog/pg_statistic_ext.h
+++ b/src/include/catalog/pg_statistic_ext.h
@@ -41,6 +41,7 @@ CATALOG(pg_statistic_ext,3381,StatisticExtRelationId)
Oid stxnamespace; /* OID of statistics object's namespace */
Oid stxowner; /* statistics object's owner */
+ int32 stxstattarget BKI_DEFAULT(-1); /* statistics target */
/*
* variable-length fields start here, but we allow direct access to
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index b4e7db67c3..1dc6dc2ca0 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -87,6 +87,7 @@ extern ObjectAddress AlterOperator(AlterOperatorStmt *stmt);
/* commands/statscmds.c */
extern ObjectAddress CreateStatistics(CreateStatsStmt *stmt);
+extern ObjectAddress AlterStatistics(AlterStatsStmt *stmt);
extern void RemoveStatisticsById(Oid statsOid);
extern void UpdateStatisticsForTypeChange(Oid statsOid,
Oid relationOid, int attnum,
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 4e2fb39105..57589d4fac 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -420,6 +420,7 @@ typedef enum NodeTag
T_CreateStatsStmt,
T_AlterCollationStmt,
T_CallStmt,
+ T_AlterStatsStmt,
/*
* TAGS FOR PARSE TREE NODES (parsenodes.h)
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 94ded3c135..f21ff8028a 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2789,6 +2789,18 @@ typedef struct CreateStatsStmt
bool if_not_exists; /* do nothing if stats name already exists */
} CreateStatsStmt;
+/* ----------------------
+ * Alter Statistics Statement
+ * ----------------------
+ */
+typedef struct AlterStatsStmt
+{
+ NodeTag type;
+ List *defnames; /* qualified name (list of Value strings) */
+ int stxstattarget; /* statistics target */
+ bool missing_ok; /* skip error if statistics object is missing */
+} AlterStatsStmt;
+
/* ----------------------
* Create Function Statement
* ----------------------
diff --git a/src/include/statistics/extended_stats_internal.h b/src/include/statistics/extended_stats_internal.h
index 8433c34f6d..fcf4e8ae0b 100644
--- a/src/include/statistics/extended_stats_internal.h
+++ b/src/include/statistics/extended_stats_internal.h
@@ -71,7 +71,7 @@ extern MVDependencies *statext_dependencies_deserialize(bytea *data);
extern MCVList *statext_mcv_build(int numrows, HeapTuple *rows,
Bitmapset *attrs, VacAttrStats **stats,
- double totalrows);
+ double totalrows, int stattarget);
extern bytea *statext_mcv_serialize(MCVList *mcv, VacAttrStats **stats);
extern MCVList *statext_mcv_deserialize(bytea *data);
diff --git a/src/include/statistics/statistics.h b/src/include/statistics/statistics.h
index cb7bc630e9..33abb22e44 100644
--- a/src/include/statistics/statistics.h
+++ b/src/include/statistics/statistics.h
@@ -100,6 +100,8 @@ extern MCVList *statext_mcv_load(Oid mvoid);
extern void BuildRelationExtStatistics(Relation onerel, double totalrows,
int numrows, HeapTuple *rows,
int natts, VacAttrStats **vacattrstats);
+extern int ComputeExtStatisticsRows(Relation onerel,
+ int natts, VacAttrStats **stats);
extern bool statext_is_kind_built(HeapTuple htup, char kind);
extern Selectivity dependencies_clauselist_selectivity(PlannerInfo *root,
List *clauses,
diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index 94b8a8f8b8..f4534e3f7c 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -98,11 +98,28 @@ CREATE STATISTICS ab1_a_b_stats ON a, b FROM ab1;
ANALYZE ab1;
WARNING: statistics object "public.ab1_a_b_stats" could not be computed for relation "public.ab1"
ALTER TABLE ab1 ALTER a SET STATISTICS -1;
+-- setting statistics target 0 skips the statistics, without printing any message, so check catalog
+ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0;
+ANALYZE ab1;
+SELECT stxname, stxdndistinct, stxddependencies, stxdmcv
+ FROM pg_statistic_ext s, pg_statistic_ext_data d
+ WHERE s.stxname = 'ab1_a_b_stats'
+ AND d.stxoid = s.oid;
+ stxname | stxdndistinct | stxddependencies | stxdmcv
+---------------+---------------+------------------+---------
+ ab1_a_b_stats | | |
+(1 row)
+
+ALTER STATISTICS ab1_a_b_stats SET STATISTICS -1;
-- partial analyze doesn't build stats either
ANALYZE ab1 (a);
WARNING: statistics object "public.ab1_a_b_stats" could not be computed for relation "public.ab1"
ANALYZE ab1;
DROP TABLE ab1;
+ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0;
+ERROR: statistics object "ab1_a_b_stats" does not exist
+ALTER STATISTICS IF EXISTS ab1_a_b_stats SET STATISTICS 0;
+NOTICE: statistics object "ab1_a_b_stats" does not exist, skipping
-- Verify supported object types for extended statistics
CREATE schema tststats;
CREATE TABLE tststats.t (a int, b int, c text);
diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index 4bc1536727..231d7cac83 100644
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -68,10 +68,20 @@ INSERT INTO ab1 SELECT a, a%23 FROM generate_series(1, 1000) a;
CREATE STATISTICS ab1_a_b_stats ON a, b FROM ab1;
ANALYZE ab1;
ALTER TABLE ab1 ALTER a SET STATISTICS -1;
+-- setting statistics target 0 skips the statistics, without printing any message, so check catalog
+ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0;
+ANALYZE ab1;
+SELECT stxname, stxdndistinct, stxddependencies, stxdmcv
+ FROM pg_statistic_ext s, pg_statistic_ext_data d
+ WHERE s.stxname = 'ab1_a_b_stats'
+ AND d.stxoid = s.oid;
+ALTER STATISTICS ab1_a_b_stats SET STATISTICS -1;
-- partial analyze doesn't build stats either
ANALYZE ab1 (a);
ANALYZE ab1;
DROP TABLE ab1;
+ALTER STATISTICS ab1_a_b_stats SET STATISTICS 0;
+ALTER STATISTICS IF EXISTS ab1_a_b_stats SET STATISTICS 0;
-- Verify supported object types for extended statistics
CREATE schema tststats;
On Saturday, July 27, 2019 7:06 AM(GMT+9), Tomas Vondra wrote:
On Fri, Jul 26, 2019 at 07:03:41AM +0000, Jamison, Kirk wrote:
On Sat, July 20, 2019 8:12 AM (GMT+9), Tomas Vondra wrote:
+ /* XXX What if the target is set to 0? Reset the statistic?
*/
This also makes me wonder. I haven't looked deeply into the code,
but since 0 is a valid value, I believe it should reset the stats.I agree resetting the stats after setting the target to 0 seems quite
reasonable. But that's not what we do for attribute stats, because in
that case we simply skip the attribute during the future ANALYZE runs
- we don't reset the stats, we keep the existing stats. So I've done
the same thing here, and I've removed the XXX comment.If we want to change that, I'd do it in a separate patch for both the
regular and extended stats.Hi, Tomas
Sorry for my late reply.
You're right. I have no strong opinion whether we'd want to change thatbehavior.
I've also confirmed the change in the patch where setting statistics
target 0 skips the statistics.OK, thanks.
Maybe only some minor nitpicks in the source code comments below:
1. "it's" should be "its":+ * Compute statistic target, based on what's set for the
statistic
+ * object itself, and for it's attributes.
2. Consistency whether you'd use either "statistic " or "statisticS ".
Ex. statistic target vs statisticS target, statistics object vs statisticobject, etc.
Attached is v4 of the patch, with a couple more improvements:
1) I've renamed the if_not_exists flag to missing_ok, because that's
more consistent with the "IF EXISTS" clause in the grammar (the old
flag was kinda the exact opposite), and I've added a NOTICE about the skip.+ bool missing_ok; /* do nothing if statistics does
not exist */
Confirmed. So we ignore if statistic does not exist, and skip the error.
Maybe to make it consistent with other data structures in
parsernodes.h, you can change the comment of missing_ok to:
/* skip error if statistics object does not exist */Thanks, I've fixed all those places in the attached v5.
I've confirmed the fix.
2) I've renamed ComputeExtStatsTarget to ComputeExtStatsRows, because
that's what the function was doing anyway (computing sample size).3) I've added a couple of regression tests to stats_ext.sql
Aside from that, I've cleaned up a couple of places and improved a
bunch of comments. Nothing huge.I have a question though regarding ComputeExtStatisticsRows.
I'm just curious with the value 300 when computing sample size.
Where did this value come from?+ /* compute sample size based on the statistic target */ + return (300 * result);Overall, the patch is almost already in good shape for commit.
I'll wait for the next update.That's how we compute number of rows to sample, based on the statistics target.
See std_typanalyze() in analyze.c, which also cites the paper where this comes
from.
Noted. Found it. Thank you for the reference.
There's just a small whitespace (extra space) below after running git diff --check.
src/bin/pg_dump/pg_dump.c:7226: trailing whitespace.
+
It would be better to post an updated patch,
but other than that, I've confirmed the fixes.
The patch passed the make-world and regression tests as well.
I've marked this as "ready for committer".
Regards,
Kirk Jamison
Hi,
From: Jamison, Kirk [mailto:k.jamison@jp.fujitsu.com]
Sent: Monday, July 29, 2019 10:53 AM
To: 'Tomas Vondra' <tomas.vondra@2ndquadrant.com>
Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; PostgreSQL Hackers
<pgsql-hackers@lists.postgresql.org>
Subject: RE: Multivariate MCV list vs. statistics targetOn Saturday, July 27, 2019 7:06 AM(GMT+9), Tomas Vondra wrote:
On Fri, Jul 26, 2019 at 07:03:41AM +0000, Jamison, Kirk wrote:
On Sat, July 20, 2019 8:12 AM (GMT+9), Tomas Vondra wrote:
+ /* XXX What if the target is set to 0? Reset the statistic?
*/
This also makes me wonder. I haven't looked deeply into the code,
but since 0 is a valid value, I believe it should reset the stats.I agree resetting the stats after setting the target to 0 seems
quite reasonable. But that's not what we do for attribute stats,
because in that case we simply skip the attribute during the future
ANALYZE runs
- we don't reset the stats, we keep the existing stats. So I've
done the same thing here, and I've removed the XXX comment.If we want to change that, I'd do it in a separate patch for both
the regular and extended stats.Hi, Tomas
Sorry for my late reply.
You're right. I have no strong opinion whether we'd want to change
thatbehavior.
I've also confirmed the change in the patch where setting statistics
target 0 skips the statistics.OK, thanks.
Maybe only some minor nitpicks in the source code comments below:
1. "it's" should be "its":+ * Compute statistic target, based on what's set for the
statistic
+ * object itself, and for it's attributes.
2. Consistency whether you'd use either "statistic " or "statisticS ".
Ex. statistic target vs statisticS target, statistics object vs
statisticobject, etc.
Attached is v4 of the patch, with a couple more improvements:
1) I've renamed the if_not_exists flag to missing_ok, because
that's more consistent with the "IF EXISTS" clause in the grammar
(the old flag was kinda the exact opposite), and I've added a NOTICEabout the skip.
+ bool missing_ok; /* do nothing if statistics does
not exist */
Confirmed. So we ignore if statistic does not exist, and skip the error.
Maybe to make it consistent with other data structures in
parsernodes.h, you can change the comment of missing_ok to:
/* skip error if statistics object does not exist */Thanks, I've fixed all those places in the attached v5.
I've confirmed the fix.
2) I've renamed ComputeExtStatsTarget to ComputeExtStatsRows,
because that's what the function was doing anyway (computing samplesize).
3) I've added a couple of regression tests to stats_ext.sql
Aside from that, I've cleaned up a couple of places and improved a
bunch of comments. Nothing huge.I have a question though regarding ComputeExtStatisticsRows.
I'm just curious with the value 300 when computing sample size.
Where did this value come from?+ /* compute sample size based on the statistic target */ + return (300 * result);Overall, the patch is almost already in good shape for commit.
I'll wait for the next update.That's how we compute number of rows to sample, based on the statistics
target.
See std_typanalyze() in analyze.c, which also cites the paper where
this comes from.Noted. Found it. Thank you for the reference.
There's just a small whitespace (extra space) below after running git diff
--check.src/bin/pg_dump/pg_dump.c:7226: trailing whitespace.
+It would be better to post an updated patch, but other than that, I've confirmed
the fixes.
The patch passed the make-world and regression tests as well.
I've marked this as "ready for committer".
The patch does not apply anymore.
In addition to the whitespace detected,
kindly rebase the patch as there were changes from recent commits
in the following files:
src/backend/commands/analyze.c
src/bin/pg_dump/pg_dump.c
src/bin/pg_dump/t/002_pg_dump.pl
src/test/regress/expected/stats_ext.out
src/test/regress/sql/stats_ext.sql
Regards,
Kirk Jamison
On Thu, Aug 1, 2019 at 12:16 PM Jamison, Kirk <k.jamison@jp.fujitsu.com> wrote:
On Saturday, July 27, 2019 7:06 AM(GMT+9), Tomas Vondra wrote:
On Fri, Jul 26, 2019 at 07:03:41AM +0000, Jamison, Kirk wrote:
Overall, the patch is almost already in good shape for commit.
I'll wait for the next update.
The patch passed the make-world and regression tests as well.
I've marked this as "ready for committer".The patch does not apply anymore.
Based on the above, it sounds like this patch is super close and the
only problem is bitrot, so I've set it back to Ready for Committer.
Over to Tomas to rebase and commit, or move to the next CF if that's
more appropriate.
--
Thomas Munro
https://enterprisedb.com
Hello.
At Thu, 1 Aug 2019 00:15:48 +0000, "Jamison, Kirk" <k.jamison@jp.fujitsu.com> wrote in <D09B13F772D2274BB348A310EE3027C6518F94@g01jpexmbkw24>
The patch does not apply anymore.
In addition to the whitespace detected,
kindly rebase the patch as there were changes from recent commits
in the following files:
src/backend/commands/analyze.c
src/bin/pg_dump/pg_dump.c
src/bin/pg_dump/t/002_pg_dump.pl
src/test/regress/expected/stats_ext.out
src/test/regress/sql/stats_ext.sql
The patch finally failed only for stats_ext.out, where 14ef15a222
is hitting. (for b2a3d706b8)
I looked through this patch and have some comments.
+++ b/src/backend/commands/statscmds.c
+#include "access/heapam.h"
..
+#include "utils/fmgroids.h"These don't seem needed.
+ Assert(stmt->missing_ok);
Perhaps we shouldn't Assert on this condition. Isn't it better we
just "elog(ERROR" here?
+ DeconstructQualifiedName(stmt->defnames, &schemaname, &statname);
Maybe we don't need detailed analysis that the function emits on
error. Couldn't we use NameListToString() instead? That reduces
the number of ereport()s and considerably simplify the code
around.
+ oldtup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(stxoid));
+
+ /* Must be owner of the existing statistics object */
+ if (!pg_statistics_object_ownercheck(stxoid, GetUserId()))This repeats the SearchSysCache twice in a quite short
duration. I suppose it'd be better that ACL (and validity) checks
done directly using oldtup.
+ /* replace the stxstattarget column */
+ repl_repl[Anum_pg_statistic_ext_stxstattarget - 1] = true;
+ repl_val[Anum_pg_statistic_ext_stxstattarget - 1] = Int32GetDatum(newtarget)We usually do this kind of work using SearchSysCacheCopyN(),
which simplifies the code around, too.
+++ b/src/backend/statistics/mcv.c* Maximum number of MCV items to store, based on the attribute with the
* largest stats target (and the number of groups we have available).
*/
- nitems = stats[0]->attr->attstattarget;
- for (i = 1; i < numattrs; i++)
- {
- if (stats[i]->attr->attstattarget > nitems)
- nitems = stats[i]->attr->attstattarget;
- }
+ nitems = stattarget;Maybe you forgot to modify the comment.
check_xact_readonly() returns false for this command. As the
result it emits a somewhat pointless error message.
=# alter statistics s1 set statistics 0;
ERROR: cannot assign TransactionIds during recovery
+++ b/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.hi_stxname = PQfnumber(res, "stxname");
i_stxnamespace = PQfnumber(res, "stxnamespace");
i_rolname = PQfnumber(res, "rolname");
+ i_stattarget = PQfnumber(res, "stxstattarget");
I'm not sure whether it is the convention here, but variable name
is different from column name only for the added line.
+++ b/src/bin/psql/tab-complete.c
- COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA");
+ COMPLETE_WITH("OWNER TO", "RENAME TO", "SET SCHEMA", "SET STATISTICS");ALTER STATISTICS s2 SET STATISTICS<tab> is suggested with
ext-stats names, but it's the place for target value.
+++ b/src/include/nodes/nodes.h
T_CallStmt,
+ T_AlterStatsStmt,I think it should be immediately below T_CreateStatsStmt.
+++ b/src/include/nodes/parsenodes.h
+ bool missing_ok; /* skip error if statistics object is missing */Should be very trivial, but many bool members especially
missing_ok have a comment having "?" at the end.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Thu, Aug 01, 2019 at 05:25:31PM +1200, Thomas Munro wrote:
On Thu, Aug 1, 2019 at 12:16 PM Jamison, Kirk <k.jamison@jp.fujitsu.com> wrote:
On Saturday, July 27, 2019 7:06 AM(GMT+9), Tomas Vondra wrote:
On Fri, Jul 26, 2019 at 07:03:41AM +0000, Jamison, Kirk wrote:
Overall, the patch is almost already in good shape for commit.
I'll wait for the next update.The patch passed the make-world and regression tests as well.
I've marked this as "ready for committer".The patch does not apply anymore.
Based on the above, it sounds like this patch is super close and the
only problem is bitrot, so I've set it back to Ready for Committer.
Over to Tomas to rebase and commit, or move to the next CF if that's
more appropriate.
I'll move it to the next CF. Aside from the issues pointed by Kyotaro-san
in his review, I still haven't made my mind about whether to base the use
statistics targets set for the attributes. That's what we're doing now,
but I'm not sure it's a good idea after adding separate statistics target.
I wonder what Dean's opinion on this is, as he added the current logic.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, 1 Aug 2019 at 11:30, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
I'll move it to the next CF. Aside from the issues pointed by Kyotaro-san
in his review, I still haven't made my mind about whether to base the use
statistics targets set for the attributes. That's what we're doing now,
but I'm not sure it's a good idea after adding separate statistics target.
I wonder what Dean's opinion on this is, as he added the current logic.
If this were being released in the same version as MCV stats first
appeared, I'd say that there's not much point basing the default
multivariate stats target on the per-column targets, when it has its
own knob to control it. However, since this won't be released for a
year, those per-column-based defaults will be in the field for that
long, and so I'd say that we shouldn't change the default when adding
this, otherwise users who don't use this new feature might be
surprised by the change in behaviour.
Regards,
Dean
On 2019-Aug-01, Tomas Vondra wrote:
I'll move it to the next CF. Aside from the issues pointed by Kyotaro-san
in his review, I still haven't made my mind about whether to base the use
statistics targets set for the attributes. That's what we're doing now,
but I'm not sure it's a good idea after adding separate statistics target.
I wonder what Dean's opinion on this is, as he added the current logic.
Latest patch no longer applies. Please update. And since you already
seem to have handled all review comments since it was Ready for
Committer, and you now know Dean's opinion on the remaining question,
please get it pushed.
Thanks
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Sep 03, 2019 at 02:38:56PM -0400, Alvaro Herrera wrote:
On 2019-Aug-01, Tomas Vondra wrote:
I'll move it to the next CF. Aside from the issues pointed by Kyotaro-san
in his review, I still haven't made my mind about whether to base the use
statistics targets set for the attributes. That's what we're doing now,
but I'm not sure it's a good idea after adding separate statistics target.
I wonder what Dean's opinion on this is, as he added the current logic.Latest patch no longer applies. Please update. And since you already
seem to have handled all review comments since it was Ready for
Committer, and you now know Dean's opinion on the remaining question,
please get it pushed.
OK, I've pushed this the patch, after some minor polishing.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hello,
I found a missing column description in the pg_statistic_ext catalog document for this new feature.
The attached patch adds a description of the 'stxstattarget' column to pg_statistic_ext catalog's document.
If there is a better explanation, please correct it.
Regards,
Noriyoshi Shinoda
-----Original Message-----
From: Tomas Vondra [mailto:tomas.vondra@2ndquadrant.com]
Sent: Wednesday, September 11, 2019 7:28 AM
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Thomas Munro <thomas.munro@gmail.com>; Jamison, Kirk <k.jamison@jp.fujitsu.com>; Dean Rasheed <dean.a.rasheed@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Multivariate MCV list vs. statistics target
On Tue, Sep 03, 2019 at 02:38:56PM -0400, Alvaro Herrera wrote:
On 2019-Aug-01, Tomas Vondra wrote:
I'll move it to the next CF. Aside from the issues pointed by
Kyotaro-san in his review, I still haven't made my mind about whether
to base the use statistics targets set for the attributes. That's
what we're doing now, but I'm not sure it's a good idea after adding separate statistics target.
I wonder what Dean's opinion on this is, as he added the current logic.Latest patch no longer applies. Please update. And since you already
seem to have handled all review comments since it was Ready for
Committer, and you now know Dean's opinion on the remaining question,
please get it pushed.
OK, I've pushed this the patch, after some minor polishing.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
pg_statistic_ext_doc_v1.diffapplication/octet-stream; name=pg_statistic_ext_doc_v1.diffDownload
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index c6f95fa..3095148 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -6453,6 +6453,17 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
</row>
<row>
+ <entry><structfield>stxstattarget</structfield></entry>
+ <entry><type>integer</type></entry>
+ <entry></entry>
+ <entry>
+ The statistic-gathering target for this statistics object specified by
+ <command>ALTER STATISTICS SET STATISTICS</command>;
+ -1 means to using the system default statistics target (<xref linkend="guc-default-statistics-target"/>).
+ </entry>
+ </row>
+
+ <row>
<entry><structfield>stxkeys</structfield></entry>
<entry><type>int2vector</type></entry>
<entry><literal><link linkend="catalog-pg-attribute"><structname>pg_attribute</structname></link>.attnum</literal></entry>
Hi,
On Wed, Mar 18, 2020 at 04:28:34AM +0000, Shinoda, Noriyoshi (PN Japan A&PS Delivery) wrote:
Hello,
I found a missing column description in the pg_statistic_ext catalog document for this new feature.
The attached patch adds a description of the 'stxstattarget' column to pg_statistic_ext catalog's document.
If there is a better explanation, please correct it.
Thanks for the report. Yes, this is clearly an omission. I think it
would be better to use wording similar to attstattarget, per the
attached patch. That is, without reference to ALTER STATISTICS and
better explaination of what positive/negative values do. Do you agree?
regards
Regards,
Noriyoshi Shinoda-----Original Message-----
From: Tomas Vondra [mailto:tomas.vondra@2ndquadrant.com]
Sent: Wednesday, September 11, 2019 7:28 AM
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Thomas Munro <thomas.munro@gmail.com>; Jamison, Kirk <k.jamison@jp.fujitsu.com>; Dean Rasheed <dean.a.rasheed@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Multivariate MCV list vs. statistics targetOn Tue, Sep 03, 2019 at 02:38:56PM -0400, Alvaro Herrera wrote:
On 2019-Aug-01, Tomas Vondra wrote:
I'll move it to the next CF. Aside from the issues pointed by
Kyotaro-san in his review, I still haven't made my mind about whether
to base the use statistics targets set for the attributes. That's
what we're doing now, but I'm not sure it's a good idea after adding separate statistics target.
I wonder what Dean's opinion on this is, as he added the current logic.Latest patch no longer applies. Please update. And since you already
seem to have handled all review comments since it was Ready for
Committer, and you now know Dean's opinion on the remaining question,
please get it pushed.OK, I've pushed this the patch, after some minor polishing.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
pg_statistic_ext_doc_v2.difftext/plain; charset=us-asciiDownload
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index c6f95fa688..64614b569c 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -6452,6 +6452,22 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
<entry>Owner of the statistics object</entry>
</row>
+ <row>
+ <entry><structfield>stxstattarget</structfield></entry>
+ <entry><type>int4</type></entry>
+ <entry></entry>
+ <entry>
+ <structfield>stxstattarget</structfield> controls the level of detail
+ of statistics accumulated for this statistics object by
+ <xref linkend="sql-analyze"/>.
+ A zero value indicates that no statistics should be collected.
+ A negative value says to use the system default statistics target.
+ Positive values <structfield>stxstattarget</structfield>
+ determine the target number of <quote>most common values</quote>
+ to collect.
+ </entry>
+ </row>
+
<row>
<entry><structfield>stxkeys</structfield></entry>
<entry><type>int2vector</type></entry>
Hi,
Thanks for the report. Yes, this is clearly an omission. I think it would be better to use wording >similar to attstattarget, per the attached patch. That is, without reference to ALTER STATISTICS and >better explaination of what positive/negative values do. Do you agree?
Thank you for your comment.
I agree with the text you suggested.
Regards,
Noriyoshi Shinoda
-----Original Message-----
From: Tomas Vondra [mailto:tomas.vondra@2ndquadrant.com]
Sent: Wednesday, March 18, 2020 9:36 PM
To: Shinoda, Noriyoshi (PN Japan A&PS Delivery) <noriyoshi.shinoda@hpe.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>; Thomas Munro <thomas.munro@gmail.com>; Jamison, Kirk <k.jamison@jp.fujitsu.com>; Dean Rasheed <dean.a.rasheed@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Multivariate MCV list vs. statistics target
Hi,
On Wed, Mar 18, 2020 at 04:28:34AM +0000, Shinoda, Noriyoshi (PN Japan A&PS Delivery) wrote:
Hello,
I found a missing column description in the pg_statistic_ext catalog document for this new feature.
The attached patch adds a description of the 'stxstattarget' column to pg_statistic_ext catalog's document.
If there is a better explanation, please correct it.
Thanks for the report. Yes, this is clearly an omission. I think it would be better to use wording similar to attstattarget, per the attached patch. That is, without reference to ALTER STATISTICS and better explaination of what positive/negative values do. Do you agree?
regards
Regards,
Noriyoshi Shinoda-----Original Message-----
From: Tomas Vondra [mailto:tomas.vondra@2ndquadrant.com]
Sent: Wednesday, September 11, 2019 7:28 AM
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Thomas Munro <thomas.munro@gmail.com>; Jamison, Kirk
<k.jamison@jp.fujitsu.com>; Dean Rasheed <dean.a.rasheed@gmail.com>;
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Multivariate MCV list vs. statistics targetOn Tue, Sep 03, 2019 at 02:38:56PM -0400, Alvaro Herrera wrote:
On 2019-Aug-01, Tomas Vondra wrote:
I'll move it to the next CF. Aside from the issues pointed by
Kyotaro-san in his review, I still haven't made my mind about
whether to base the use statistics targets set for the attributes.
That's what we're doing now, but I'm not sure it's a good idea after adding separate statistics target.
I wonder what Dean's opinion on this is, as he added the current logic.Latest patch no longer applies. Please update. And since you already
seem to have handled all review comments since it was Ready for
Committer, and you now know Dean's opinion on the remaining question,
please get it pushed.OK, I've pushed this the patch, after some minor polishing.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Mar 18, 2020 at 01:32:19PM +0000, Shinoda, Noriyoshi (PN Japan
A&PS Delivery) wrote:
Hi,
Thanks for the report. Yes, this is clearly an omission. I think it
would be better to use wording >similar to attstattarget, per the
attached patch. That is, without reference to ALTER STATISTICS andbetter explaination of what positive/negative values do. Do you
agree?Thank you for your comment. I agree with the text you suggested.
Thank you for the report, I've pushed the reworded fix.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
I think the docs are inconsistent with the commit message and the code
(d06215d03) and docs should be corrected, soemthing like so:
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index b135c89005..cd10a6a6fc 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -7302,7 +7302,8 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
of statistics accumulated for this statistics object by
<xref linkend="sql-analyze"/>.
A zero value indicates that no statistics should be collected.
- A negative value says to use the system default statistics target.
+ A negative value says to use the maximum of the statistics targets of
+ the referenced columns, if set, or the system default statistics target.
Positive values of <structfield>stxstattarget</structfield>
determine the target number of <quote>most common values</quote>
to collect.
diff --git a/doc/src/sgml/ref/alter_statistics.sgml b/doc/src/sgml/ref/alter_statistics.sgml
index be4c3f1f05..f2e8a93166 100644
--- a/doc/src/sgml/ref/alter_statistics.sgml
+++ b/doc/src/sgml/ref/alter_statistics.sgml
@@ -101,7 +101,8 @@ ALTER STATISTICS <replaceable class="parameter">name</replaceable> SET STATISTIC
The statistic-gathering target for this statistics object for subsequent
<xref linkend="sql-analyze"/> operations.
The target can be set in the range 0 to 10000; alternatively, set it
- to -1 to revert to using the system default statistics
+ to -1 to revert to using the maximum statistics target of the
+ referenced column's, if set, or the system default statistics
target (<xref linkend="guc-default-statistics-target"/>).
For more information on the use of statistics by the
<productname>PostgreSQL</productname> query planner, refer to
--
2.17.0