pg_statistic_ext.staenabled might not be the best column name
I'd been thinking that staenabled is not the most suitable column name
for storing the types of statistics that are defined for the extended
statistics. For me, this indicates that something can be disabled,
but there's no syntax for that, and even if there was, if we were to
enable/disable the kinds, we'd likely want to keep tabs on which kinds
were originally defined, otherwise it's more of an ADD and DROP than
an ENABLE/DISABLE.
"stakind" seems like a better name. I'd have personally gone with
"statype" but pg_statistic already thinks stakind is better.
A patch which changes this is attached
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
ext_stats_rename_staenabled.patchapplication/octet-stream; name=ext_stats_rename_staenabled.patchDownload
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 5883673..10ab169 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -4321,12 +4321,13 @@
</row>
<row>
- <entry><structfield>staenabled</structfield></entry>
+ <entry><structfield>stakind</structfield></entry>
<entry><type>char[]</type></entry>
<entry></entry>
<entry>
- An array with the modes of the enabled statistic types, encoded as
- <literal>d</literal> for ndistinct coefficients.
+ An array with the an element for each statistic kind set in the statistics.
+ <literal>d</literal> for ndistinct coefficients, <literal>f</literal>
+ for functional dependencies.
</entry>
</row>
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 46abadc..84f5c3f 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -62,12 +62,12 @@ CreateStatistics(CreateStatsStmt *stmt)
Oid relid;
ObjectAddress parentobject,
childobject;
- Datum types[2]; /* one for each possible type of statistics */
- int ntypes;
- ArrayType *staenabled;
+ Datum kinds[2]; /* one for each possible kind of statistics */
+ int nkinds;
+ ArrayType *stakind;
bool build_ndistinct;
bool build_dependencies;
- bool requested_type = false;
+ bool requested_kind = false;
Assert(IsA(stmt, CreateStatsStmt));
@@ -185,7 +185,7 @@ CreateStatistics(CreateStatsStmt *stmt)
stakeys = buildint2vector(attnums, numcols);
/*
- * Parse the statistics options. Currently only statistics types are
+ * Parse the statistics options. Currently only statistics kinds are
* recognized.
*/
build_ndistinct = false;
@@ -197,12 +197,12 @@ CreateStatistics(CreateStatsStmt *stmt)
if (strcmp(opt->defname, "ndistinct") == 0)
{
build_ndistinct = defGetBoolean(opt);
- requested_type = true;
+ requested_kind = true;
}
else if (strcmp(opt->defname, "dependencies") == 0)
{
build_dependencies = defGetBoolean(opt);
- requested_type = true;
+ requested_kind = true;
}
else
ereport(ERROR,
@@ -210,21 +210,21 @@ CreateStatistics(CreateStatsStmt *stmt)
errmsg("unrecognized STATISTICS option \"%s\"",
opt->defname)));
}
- /* If no statistic type was specified, build them all. */
- if (!requested_type)
+ /* If no statistic kind was specified, build them all. */
+ if (!requested_kind)
{
build_ndistinct = true;
build_dependencies = true;
}
- /* construct the char array of enabled statistic types */
- ntypes = 0;
+ /* construct the char array of statistics kinds */
+ nkinds = 0;
if (build_ndistinct)
- types[ntypes++] = CharGetDatum(STATS_EXT_NDISTINCT);
+ kinds[nkinds++] = CharGetDatum(STATS_EXT_NDISTINCT);
if (build_dependencies)
- types[ntypes++] = CharGetDatum(STATS_EXT_DEPENDENCIES);
- Assert(ntypes > 0);
- staenabled = construct_array(types, ntypes, CHAROID, 1, true, 'c');
+ kinds[nkinds++] = CharGetDatum(STATS_EXT_DEPENDENCIES);
+ Assert(nkinds > 0);
+ stakind = construct_array(kinds, nkinds, CHAROID, 1, true, 'c');
/*
* Everything seems fine, so let's build the pg_statistic_ext tuple.
@@ -236,7 +236,7 @@ CreateStatistics(CreateStatsStmt *stmt)
values[Anum_pg_statistic_ext_stanamespace - 1] = ObjectIdGetDatum(namespaceId);
values[Anum_pg_statistic_ext_staowner - 1] = ObjectIdGetDatum(GetUserId());
values[Anum_pg_statistic_ext_stakeys - 1] = PointerGetDatum(stakeys);
- values[Anum_pg_statistic_ext_staenabled - 1] = PointerGetDatum(staenabled);
+ values[Anum_pg_statistic_ext_stakind - 1] = PointerGetDatum(stakind);
/* no statistics build yet */
nulls[Anum_pg_statistic_ext_standistinct - 1] = true;
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index 4b3aa77..b2bb8d3 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -162,7 +162,7 @@ fetch_statentries_for_relation(Relation pg_statext, Oid relid)
bool isnull;
int i;
ArrayType *arr;
- char *enabled;
+ char *kind;
Form_pg_statistic_ext staForm;
entry = palloc0(sizeof(StatExtEntry));
@@ -174,21 +174,21 @@ fetch_statentries_for_relation(Relation pg_statext, Oid relid)
staForm->stakeys.values[i]);
}
- /* decode the staenabled char array into a list of chars */
+ /* decode the stakind char array into a list of chars */
datum = SysCacheGetAttr(STATEXTOID, htup,
- Anum_pg_statistic_ext_staenabled, &isnull);
+ Anum_pg_statistic_ext_stakind, &isnull);
Assert(!isnull);
arr = DatumGetArrayTypeP(datum);
if (ARR_NDIM(arr) != 1 ||
ARR_HASNULL(arr) ||
ARR_ELEMTYPE(arr) != CHAROID)
- elog(ERROR, "staenabled is not a 1-D char array");
- enabled = (char *) ARR_DATA_PTR(arr);
+ elog(ERROR, "stakind is not a 1-D char array");
+ kind = (char *) ARR_DATA_PTR(arr);
for (i = 0; i < ARR_DIMS(arr)[0]; i++)
{
- Assert((enabled[i] == STATS_EXT_NDISTINCT) ||
- (enabled[i] == STATS_EXT_DEPENDENCIES));
- entry->types = lappend_int(entry->types, (int) enabled[i]);
+ Assert((kind[i] == STATS_EXT_NDISTINCT) ||
+ (kind[i] == STATS_EXT_DEPENDENCIES));
+ entry->types = lappend_int(entry->types, (int) kind[i]);
}
result = lappend(result, entry);
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 5f11af2..19f3c8f 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1453,11 +1453,11 @@ pg_get_statisticsext_worker(Oid statextid, bool missing_ok)
int colno;
char *nsp;
ArrayType *arr;
- char *enabled;
+ char *kind;
Datum datum;
bool isnull;
- bool ndistinct_enabled;
- bool dependencies_enabled;
+ bool has_ndistinct;
+ bool has_dependencies;
int i;
statexttup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(statextid));
@@ -1479,43 +1479,43 @@ pg_get_statisticsext_worker(Oid statextid, bool missing_ok)
NameStr(statextrec->staname)));
/*
- * Lookup the staenabled column so that we know how to handle the WITH
+ * Lookup the stakind column so that we know how to handle the WITH
* clause.
*/
datum = SysCacheGetAttr(STATEXTOID, statexttup,
- Anum_pg_statistic_ext_staenabled, &isnull);
+ Anum_pg_statistic_ext_stakind, &isnull);
Assert(!isnull);
arr = DatumGetArrayTypeP(datum);
if (ARR_NDIM(arr) != 1 ||
ARR_HASNULL(arr) ||
ARR_ELEMTYPE(arr) != CHAROID)
- elog(ERROR, "staenabled is not a 1-D char array");
- enabled = (char *) ARR_DATA_PTR(arr);
+ elog(ERROR, "stakind is not a 1-D char array");
+ kind = (char *) ARR_DATA_PTR(arr);
- ndistinct_enabled = false;
- dependencies_enabled = false;
+ has_ndistinct = false;
+ has_dependencies = false;
for (i = 0; i < ARR_DIMS(arr)[0]; i++)
{
- if (enabled[i] == STATS_EXT_NDISTINCT)
- ndistinct_enabled = true;
- if (enabled[i] == STATS_EXT_DEPENDENCIES)
- dependencies_enabled = true;
+ if (kind[i] == STATS_EXT_NDISTINCT)
+ has_ndistinct = true;
+ if (kind[i] == STATS_EXT_DEPENDENCIES)
+ has_dependencies = true;
}
/*
- * If any option is disabled, then we'll need to append a WITH clause to
- * show which options are enabled. We omit the WITH clause on purpose
- * when all options are enabled, so a pg_dump/pg_restore will create all
- * statistics types on a newer postgres version, if the statistics had all
- * options enabled on the original version.
+ * If any of the supported statistic kinds were not specified then we'll
+ * need to append a WITH clause to show which ones were specified. We
+ * omit the WITH clause on purpose when all statistic kinds were
+ * specified, so a pg_dump/pg_restore will create all statistic kinds when
+ * performing the restore on a newer PostgreSQL version.
*/
- if (!ndistinct_enabled || !dependencies_enabled)
+ if (!has_ndistinct || !has_dependencies)
{
appendStringInfoString(&buf, " WITH (");
- if (ndistinct_enabled)
+ if (has_ndistinct)
appendStringInfoString(&buf, "ndistinct");
- else if (dependencies_enabled)
+ else if (has_dependencies)
appendStringInfoString(&buf, "dependencies");
appendStringInfoChar(&buf, ')');
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index e7c3d73..86a02fc 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -2350,8 +2350,8 @@ describeOneTableDetails(const char *schemaname,
" FROM pg_catalog.unnest(stakeys) s(attnum)\n"
" JOIN pg_catalog.pg_attribute a ON (starelid = a.attrelid AND\n"
" a.attnum = s.attnum AND NOT attisdropped)) AS columns,\n"
- " (staenabled @> '{d}') AS ndist_enabled,\n"
- " (staenabled @> '{f}') AS deps_enabled\n"
+ " (stakind @> '{d}') AS has_ndistinct,\n"
+ " (stakind @> '{f}') AS has_funcdeps\n"
"FROM pg_catalog.pg_statistic_ext stat "
"WHERE starelid = '%s'\n"
"ORDER BY 1;",
diff --git a/src/include/catalog/pg_statistic_ext.h b/src/include/catalog/pg_statistic_ext.h
index 0a1cc04..1130c32 100644
--- a/src/include/catalog/pg_statistic_ext.h
+++ b/src/include/catalog/pg_statistic_ext.h
@@ -43,7 +43,7 @@ CATALOG(pg_statistic_ext,3381)
int2vector stakeys; /* array of column keys */
#ifdef CATALOG_VARLEN
- char staenabled[1] BKI_FORCE_NOT_NULL; /* statistic types
+ char stakind[1] BKI_FORCE_NOT_NULL; /* statistic kinds
* requested to build */
pg_ndistinct standistinct; /* ndistinct coefficients (serialized) */
pg_dependencies stadependencies; /* dependencies (serialized) */
@@ -68,7 +68,7 @@ typedef FormData_pg_statistic_ext *Form_pg_statistic_ext;
#define Anum_pg_statistic_ext_stanamespace 3
#define Anum_pg_statistic_ext_staowner 4
#define Anum_pg_statistic_ext_stakeys 5
-#define Anum_pg_statistic_ext_staenabled 6
+#define Anum_pg_statistic_ext_stakind 6
#define Anum_pg_statistic_ext_standistinct 7
#define Anum_pg_statistic_ext_stadependencies 8
diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index b43208d..1bfde66 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -131,11 +131,11 @@ ERROR: duplicate column name in statistics definition
-- correct command
CREATE STATISTICS s10 ON (a, b, c) FROM ndistinct;
ANALYZE ndistinct;
-SELECT staenabled, standistinct
+SELECT stakind, standistinct
FROM pg_statistic_ext WHERE starelid = 'ndistinct'::regclass;
- staenabled | standistinct
-------------+------------------------------------------------------------------------------------------------
- {d,f} | [{(b 3 4), 301.000000}, {(b 3 6), 301.000000}, {(b 4 6), 301.000000}, {(b 3 4 6), 301.000000}]
+ stakind | standistinct
+---------+------------------------------------------------------------------------------------------------
+ {d,f} | [{(b 3 4), 301.000000}, {(b 3 6), 301.000000}, {(b 4 6), 301.000000}, {(b 3 4 6), 301.000000}]
(1 row)
-- Hash Aggregate, thanks to estimates improved by the statistic
@@ -197,11 +197,11 @@ INSERT INTO ndistinct (a, b, c, filler1)
cash_words(mod(i,33)::int::money)
FROM generate_series(1,10000) s(i);
ANALYZE ndistinct;
-SELECT staenabled, standistinct
+SELECT stakind, standistinct
FROM pg_statistic_ext WHERE starelid = 'ndistinct'::regclass;
- staenabled | standistinct
-------------+----------------------------------------------------------------------------------------------------
- {d,f} | [{(b 3 4), 2550.000000}, {(b 3 6), 800.000000}, {(b 4 6), 1632.000000}, {(b 3 4 6), 10000.000000}]
+ stakind | standistinct
+---------+----------------------------------------------------------------------------------------------------
+ {d,f} | [{(b 3 4), 2550.000000}, {(b 3 6), 800.000000}, {(b 4 6), 1632.000000}, {(b 3 4 6), 10000.000000}]
(1 row)
-- plans using Group Aggregate, thanks to using correct esimates
@@ -257,10 +257,10 @@ EXPLAIN (COSTS off)
(3 rows)
DROP STATISTICS s10;
-SELECT staenabled, standistinct
+SELECT stakind, standistinct
FROM pg_statistic_ext WHERE starelid = 'ndistinct'::regclass;
- staenabled | standistinct
-------------+--------------
+ stakind | standistinct
+---------+--------------
(0 rows)
-- dropping the statistics switches the plans to Hash Aggregate,
diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index 1b0018d..e12c433 100644
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -90,7 +90,7 @@ CREATE STATISTICS s10 ON (a, b, c) FROM ndistinct;
ANALYZE ndistinct;
-SELECT staenabled, standistinct
+SELECT stakind, standistinct
FROM pg_statistic_ext WHERE starelid = 'ndistinct'::regclass;
-- Hash Aggregate, thanks to estimates improved by the statistic
@@ -121,7 +121,7 @@ INSERT INTO ndistinct (a, b, c, filler1)
ANALYZE ndistinct;
-SELECT staenabled, standistinct
+SELECT stakind, standistinct
FROM pg_statistic_ext WHERE starelid = 'ndistinct'::regclass;
-- plans using Group Aggregate, thanks to using correct esimates
@@ -142,7 +142,7 @@ EXPLAIN (COSTS off)
DROP STATISTICS s10;
-SELECT staenabled, standistinct
+SELECT stakind, standistinct
FROM pg_statistic_ext WHERE starelid = 'ndistinct'::regclass;
-- dropping the statistics switches the plans to Hash Aggregate,
On Wed, Apr 12, 2017 at 9:36 AM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
I'd been thinking that staenabled is not the most suitable column name
for storing the types of statistics that are defined for the extended
statistics.
+1.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/12/2017 03:36 PM, David Rowley wrote:
"stakind" seems like a better name. I'd have personally gone with
"statype" but pg_statistic already thinks stakind is better.
+1 to stakind
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
On 04/12/2017 03:36 PM, David Rowley wrote:
"stakind" seems like a better name. I'd have personally gone with
"statype" but pg_statistic already thinks stakind is better.
+1 to stakind
I agree with that, but as long as we're rethinking column names here,
was it a good idea to use the same "sta" prefix in pg_statistic_ext
as in pg_statistic? I do not think there's anyplace else where we're
using the same table-identifying prefix in two different catalogs,
and it seems a little pointless to follow that convention at all if
we're not going to make it a unique prefix.
We could go with "ste" perhaps, or break the convention of 3-character
prefixes and go with "stae".
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/13/2017 03:28 PM, Tom Lane wrote:
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
On 04/12/2017 03:36 PM, David Rowley wrote:
"stakind" seems like a better name. I'd have personally gone with
"statype" but pg_statistic already thinks stakind is better.+1 to stakind
I agree with that, but as long as we're rethinking column names here,
was it a good idea to use the same "sta" prefix in pg_statistic_ext
as in pg_statistic? I do not think there's anyplace else where we're
using the same table-identifying prefix in two different catalogs,
and it seems a little pointless to follow that convention at all if
we're not going to make it a unique prefix.We could go with "ste" perhaps, or break the convention of 3-character
prefixes and go with "stae".
We have a bunch of > 3-character prefixes already: amop*, amproc*,
enum*, cast*. But I think I nevertheless like "ste" better.
That said, we also have two existing tables with the same prefix:
pg_constraint and pg_conversion. Both use "con" as the prefix. Yes, it
is a bit confusing, let's not to make the same mistake again.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4/13/17 08:37, Heikki Linnakangas wrote:
We could go with "ste" perhaps, or break the convention of 3-character
prefixes and go with "stae".We have a bunch of > 3-character prefixes already: amop*, amproc*,
enum*, cast*. But I think I nevertheless like "ste" better.
"stx" perhaps?
I would also be in favor of changing it to something other than "sta".
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 4/13/17 08:37, Heikki Linnakangas wrote:
We have a bunch of > 3-character prefixes already: amop*, amproc*,
enum*, cast*. But I think I nevertheless like "ste" better.
"stx" perhaps?
I would also be in favor of changing it to something other than "sta".
"stx" sounds pretty good to me --- it seems like e.g. "stxkind" is
more visibly distinct from "stakind" than "stekind" would be.
Any objections out there?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 4/13/17 08:37, Heikki Linnakangas wrote:
We have a bunch of > 3-character prefixes already: amop*, amproc*,
enum*, cast*. But I think I nevertheless like "ste" better."stx" perhaps?
I would also be in favor of changing it to something other than "sta".
"stx" sounds pretty good to me --- it seems like e.g. "stxkind" is
more visibly distinct from "stakind" than "stekind" would be.Any objections out there?
stx sounds good to me too. I'll see about a patch this afternoon.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Alvaro Herrera wrote:
Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 4/13/17 08:37, Heikki Linnakangas wrote:
We have a bunch of > 3-character prefixes already: amop*, amproc*,
enum*, cast*. But I think I nevertheless like "ste" better."stx" perhaps?
I would also be in favor of changing it to something other than "sta".
"stx" sounds pretty good to me --- it seems like e.g. "stxkind" is
more visibly distinct from "stakind" than "stekind" would be.Any objections out there?
stx sounds good to me too. I'll see about a patch this afternoon.
Took me a bit longer than I had hoped, but it's done now.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers