From d98742439bfe82a20cffcdda6d5a05fdd06b46ea Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Thu, 14 Mar 2024 11:06:49 +0100 Subject: [PATCH v5 1/3] Make stxstattarget nullable To match attstattarget change (commit 4f622503d6d). Reviewed-by: Tomas Vondra Discussion: https://www.postgresql.org/message-id/flat/4da8d211-d54d-44b9-9847-f2a9f1184c76@eisentraut.org --- doc/src/sgml/catalogs.sgml | 28 +++++++-------- 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, 93 insertions(+), 70 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 387a14b1869..2f091ad09d1 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -7657,6 +7657,19 @@ <structname>pg_statistic_ext</structname> Columns + + + stxkeys int2vector + (references pg_attribute.attnum) + + + An array of attribute numbers, indicating which table columns are + covered by this statistics object; + for example a value of 1 3 would + mean that the first and the third table columns are covered + + + stxstattarget int2 @@ -7666,7 +7679,7 @@ <structname>pg_statistic_ext</structname> Columns 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 + 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 @@ -7674,19 +7687,6 @@ <structname>pg_statistic_ext</structname> Columns - - - stxkeys int2vector - (references pg_attribute.attnum) - - - An array of attribute numbers, indicating which table columns are - covered by this statistics object; - for example a value of 1 3 would - mean that the first and the third table columns are covered - - - stxkind char[] diff --git a/doc/src/sgml/ref/alter_statistics.sgml b/doc/src/sgml/ref/alter_statistics.sgml index 73cc9e830de..c82a728a910 100644 --- a/doc/src/sgml/ref/alter_statistics.sgml +++ b/doc/src/sgml/ref/alter_statistics.sgml @@ -26,7 +26,7 @@ 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 @@ Parameters 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 a855f750c75..5f49479832d 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 2470265561a..ae6719329e7 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 c6e2f679fd5..3ad99fffe1d 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 f21bdc2ab9a..99fdf208dba 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 171e5916965..a5149ca823c 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 1ab80eb7cac..6433497bcd2 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 429989efd91..9cf6dae3d90 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202403132 +#define CATALOG_VERSION_NO 202403141 #endif diff --git a/src/include/catalog/pg_statistic_ext.h b/src/include/catalog/pg_statistic_ext.h index 85064086ec5..50a5c021edf 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 aadaf67f574..70a21df0fee 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; base-commit: 6b41ef03306f50602f68593d562cd73d5e39a9b9 -- 2.44.0