From 012460ee93c304fbc7220e5b55d9d0577fc766ab Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Sun, 17 Mar 2024 12:22:05 +0100 Subject: [PATCH] Make stxstattarget nullable To match attstattarget change (commit 4f622503d6d). The logic inside CreateStatistics() is clarified a bit compared to that previous patch, and so here we also update ATExecSetStatistics() to match. Reviewed-by: Tomas Vondra Discussion: https://www.postgresql.org/message-id/flat/4da8d211-d54d-44b9-9847-f2a9f1184c76@eisentraut.org --- doc/src/sgml/catalogs.sgml | 34 +++++++++--------- doc/src/sgml/ref/alter_statistics.sgml | 11 +++--- src/backend/commands/statscmds.c | 46 ++++++++++++++++-------- src/backend/commands/tablecmds.c | 47 +++++++++++++------------ src/backend/parser/gram.y | 4 +-- src/backend/statistics/extended_stats.c | 4 ++- src/bin/pg_dump/pg_dump.c | 10 +++--- src/bin/psql/describe.c | 3 +- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_statistic_ext.h | 6 ++-- src/include/nodes/parsenodes.h | 2 +- 11 files changed, 96 insertions(+), 73 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 387a14b186..2f091ad09d 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -7657,23 +7657,6 @@ SCRAM-SHA-256$<iteration count>:&l - - - stxstattarget int2 - - - stxstattarget controls the level of detail - of statistics accumulated for this statistics object by - ANALYZE. - A zero value indicates that no statistics should be collected. - 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 stxstattarget - determine the target number of most common values - to collect. - - - stxkeys int2vector @@ -7687,6 +7670,23 @@ SCRAM-SHA-256$<iteration count>:&l + + + stxstattarget int2 + + + stxstattarget controls the level of detail + of statistics accumulated for this statistics object by + ANALYZE. + A zero value indicates that no statistics should be collected. + A null 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 stxstattarget + determine the target number of most common values + to collect. + + + stxkind char[] diff --git a/doc/src/sgml/ref/alter_statistics.sgml b/doc/src/sgml/ref/alter_statistics.sgml index 73cc9e830d..c82a728a91 100644 --- a/doc/src/sgml/ref/alter_statistics.sgml +++ b/doc/src/sgml/ref/alter_statistics.sgml @@ -26,7 +26,7 @@ PostgreSQL documentation ALTER STATISTICS name OWNER TO { new_owner | CURRENT_ROLE | CURRENT_USER | SESSION_USER } ALTER STATISTICS name RENAME TO new_name ALTER STATISTICS name SET SCHEMA new_schema -ALTER STATISTICS name SET STATISTICS new_target +ALTER STATISTICS name SET STATISTICS { new_target | DEFAULT } @@ -101,10 +101,11 @@ ALTER STATISTICS name SET STATISTIC The statistic-gathering target for this statistics object for subsequent ANALYZE operations. - The target can be set in the range 0 to 10000; alternatively, set it - to -1 to revert to using the maximum of the statistics target of the - referenced columns, if set, or the system default statistics - target (). + The target can be set in the range 0 to 10000. Set it to + DEFAULT to revert to using the system default + statistics target (). + (Setting to a value of -1 is an obsolete way spelling to get the same + outcome.) For more information on the use of statistics by the PostgreSQL query planner, refer to . diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c index a855f750c7..5f49479832 100644 --- a/src/backend/commands/statscmds.c +++ b/src/backend/commands/statscmds.c @@ -495,9 +495,9 @@ 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] = Int16GetDatum(-1); values[Anum_pg_statistic_ext_stxowner - 1] = ObjectIdGetDatum(stxowner); values[Anum_pg_statistic_ext_stxkeys - 1] = PointerGetDatum(stxkeys); + nulls[Anum_pg_statistic_ext_stxstattarget - 1] = true; values[Anum_pg_statistic_ext_stxkind - 1] = PointerGetDatum(stxkind); values[Anum_pg_statistic_ext_stxexprs - 1] = exprsDatum; @@ -606,23 +606,36 @@ AlterStatistics(AlterStatsStmt *stmt) bool repl_null[Natts_pg_statistic_ext]; bool repl_repl[Natts_pg_statistic_ext]; ObjectAddress address; - int newtarget = stmt->stxstattarget; + int newtarget; + bool newtarget_default; - /* Limit statistics target to a sane range */ - if (newtarget < -1) + /* -1 was used in previous versions for the default setting */ + if (stmt->stxstattarget && intVal(stmt->stxstattarget) != -1) { - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("statistics target %d is too low", - newtarget))); + newtarget = intVal(stmt->stxstattarget); + newtarget_default = false; } - else if (newtarget > MAX_STATISTICS_TARGET) + else + newtarget_default = true; + + if (!newtarget_default) { - newtarget = MAX_STATISTICS_TARGET; - ereport(WARNING, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("lowering statistics target to %d", - newtarget))); + /* Limit statistics target to a sane range */ + if (newtarget < 0) + { + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("statistics target %d is too low", + newtarget))); + } + else if (newtarget > MAX_STATISTICS_TARGET) + { + newtarget = MAX_STATISTICS_TARGET; + ereport(WARNING, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("lowering statistics target to %d", + newtarget))); + } } /* lookup OID of the statistics object */ @@ -673,7 +686,10 @@ AlterStatistics(AlterStatsStmt *stmt) /* replace the stxstattarget column */ repl_repl[Anum_pg_statistic_ext_stxstattarget - 1] = true; - repl_val[Anum_pg_statistic_ext_stxstattarget - 1] = Int16GetDatum(newtarget); + if (!newtarget_default) + repl_val[Anum_pg_statistic_ext_stxstattarget - 1] = Int16GetDatum(newtarget); + else + repl_null[Anum_pg_statistic_ext_stxstattarget - 1] = true; newtup = heap_modify_tuple(oldtup, RelationGetDescr(rel), repl_val, repl_null, repl_repl); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 2470265561..ae6719329e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8712,6 +8712,7 @@ static ObjectAddress ATExecSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newValue, LOCKMODE lockmode) { int newtarget; + bool newtarget_default; Relation attrelation; HeapTuple tuple, newtuple; @@ -8733,35 +8734,35 @@ ATExecSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newVa (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot refer to non-index column by number"))); - if (newValue) + /* -1 was used in previous versions for the default setting */ + if (newValue && intVal(newValue) != -1) { newtarget = intVal(newValue); + newtarget_default = false; } else + newtarget_default = true; + + if (!newtarget_default) { /* - * -1 was used in previous versions to represent the default setting + * Limit target to a sane range */ - newtarget = -1; - } - - /* - * 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 > MAX_STATISTICS_TARGET) - { - newtarget = MAX_STATISTICS_TARGET; - ereport(WARNING, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("lowering statistics target to %d", - newtarget))); + if (newtarget < 0) + { + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("statistics target %d is too low", + newtarget))); + } + else if (newtarget > MAX_STATISTICS_TARGET) + { + newtarget = MAX_STATISTICS_TARGET; + ereport(WARNING, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("lowering statistics target to %d", + newtarget))); + } } attrelation = table_open(AttributeRelationId, RowExclusiveLock); @@ -8815,7 +8816,7 @@ ATExecSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newVa /* Build new tuple. */ memset(repl_null, false, sizeof(repl_null)); memset(repl_repl, false, sizeof(repl_repl)); - if (newtarget != -1) + if (!newtarget_default) repl_val[Anum_pg_attribute_attstattarget - 1] = newtarget; else repl_null[Anum_pg_attribute_attstattarget - 1] = true; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index c6e2f679fd..3ad99fffe1 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -4610,7 +4610,7 @@ stats_param: ColId *****************************************************************************/ AlterStatsStmt: - ALTER STATISTICS any_name SET STATISTICS SignedIconst + ALTER STATISTICS any_name SET STATISTICS set_statistics_value { AlterStatsStmt *n = makeNode(AlterStatsStmt); @@ -4619,7 +4619,7 @@ AlterStatsStmt: n->stxstattarget = $6; $$ = (Node *) n; } - | ALTER STATISTICS IF_P EXISTS any_name SET STATISTICS SignedIconst + | ALTER STATISTICS IF_P EXISTS any_name SET STATISTICS set_statistics_value { AlterStatsStmt *n = makeNode(AlterStatsStmt); diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c index f21bdc2ab9..99fdf208db 100644 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -454,13 +454,15 @@ 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, staForm->stxkeys.values[i]); } + datum = SysCacheGetAttr(STATEXTOID, htup, Anum_pg_statistic_ext_stxstattarget, &isnull); + entry->stattarget = isnull ? -1 : DatumGetInt16(datum); + /* decode the stxkind char array into a list of chars */ datum = SysCacheGetAttrNotNull(STATEXTOID, htup, Anum_pg_statistic_ext_stxkind); diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 171e591696..a5149ca823 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -7580,7 +7580,7 @@ getExtendedStatistics(Archive *fout) if (fout->remoteVersion < 130000) appendPQExpBufferStr(query, "SELECT tableoid, oid, stxname, " - "stxnamespace, stxowner, stxrelid, (-1) AS stxstattarget " + "stxnamespace, stxowner, stxrelid, NULL AS stxstattarget " "FROM pg_catalog.pg_statistic_ext"); else appendPQExpBufferStr(query, "SELECT tableoid, oid, stxname, " @@ -7613,7 +7613,10 @@ getExtendedStatistics(Archive *fout) statsextinfo[i].rolname = getRoleName(PQgetvalue(res, i, i_stxowner)); statsextinfo[i].stattable = findTableByOid(atooid(PQgetvalue(res, i, i_stxrelid))); - statsextinfo[i].stattarget = atoi(PQgetvalue(res, i, i_stattarget)); + if (PQgetisnull(res, i, i_stattarget)) + statsextinfo[i].stattarget = -1; + else + statsextinfo[i].stattarget = atoi(PQgetvalue(res, i, i_stattarget)); /* Decide whether we want to dump it */ selectDumpableStatisticsObject(&(statsextinfo[i]), fout); @@ -17062,8 +17065,7 @@ dumpStatisticsExt(Archive *fout, const StatsExtInfo *statsextinfo) /* * We only issue an ALTER STATISTICS statement if the stxstattarget entry - * for this statistics object is non-negative (i.e. it's not the default - * value). + * for this statistics object is not the default value. */ if (statsextinfo->stattarget >= 0) { diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 1ab80eb7ca..6433497bcd 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -2804,7 +2804,8 @@ describeOneTableDetails(const char *schemaname, PQgetvalue(result, i, 1)); /* Show the stats target if it's not default */ - if (strcmp(PQgetvalue(result, i, 8), "-1") != 0) + if (!PQgetisnull(result, i, 8) && + strcmp(PQgetvalue(result, i, 8), "-1") != 0) appendPQExpBuffer(&buf, "; STATISTICS %s", PQgetvalue(result, i, 8)); diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 9cf6dae3d9..aec49079c1 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202403141 +#define CATALOG_VERSION_NO 202403171 #endif diff --git a/src/include/catalog/pg_statistic_ext.h b/src/include/catalog/pg_statistic_ext.h index 85064086ec..50a5c021ed 100644 --- a/src/include/catalog/pg_statistic_ext.h +++ b/src/include/catalog/pg_statistic_ext.h @@ -43,15 +43,15 @@ CATALOG(pg_statistic_ext,3381,StatisticExtRelationId) * object's namespace */ Oid stxowner BKI_LOOKUP(pg_authid); /* statistics object's owner */ - int16 stxstattarget BKI_DEFAULT(-1); /* statistics target */ /* - * variable-length fields start here, but we allow direct access to - * stxkeys + * variable-length/nullable fields start here, but we allow direct access + * to stxkeys */ int2vector stxkeys BKI_FORCE_NOT_NULL; /* array of column keys */ #ifdef CATALOG_VARLEN + int16 stxstattarget BKI_DEFAULT(_null_) BKI_FORCE_NULL; /* statistics target */ char stxkind[1] BKI_FORCE_NOT_NULL; /* statistics kinds requested * to build */ pg_node_tree stxexprs; /* A list of expression trees for stats diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index aadaf67f57..70a21df0fe 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3269,7 +3269,7 @@ typedef struct AlterStatsStmt { NodeTag type; List *defnames; /* qualified name (list of String) */ - int stxstattarget; /* statistics target */ + Node *stxstattarget; /* statistics target */ bool missing_ok; /* skip error if statistics object is missing */ } AlterStatsStmt;