Allowing extended stats on foreign and partitioned tables
While reviewing extended stats I noticed that it was possible to
create extended stats on many object types, including sequences. I
mentioned that this should be disallowed. Statistics were then changed
to be only allowed on plain tables and materialized views.
This should be relaxed again to allow anything ANALYZE is allowed on.
The attached does this.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
ext_stats_on_analyzable_objects.patchapplication/octet-stream; name=ext_stats_on_analyzable_objects.patchDownload
diff --git a/doc/src/sgml/ref/create_statistics.sgml b/doc/src/sgml/ref/create_statistics.sgml
index dbe28d6..edbcf58 100644
--- a/doc/src/sgml/ref/create_statistics.sgml
+++ b/doc/src/sgml/ref/create_statistics.sgml
@@ -34,7 +34,7 @@ CREATE STATISTICS [ IF NOT EXISTS ] <replaceable class="PARAMETER">statistics_na
<para>
<command>CREATE STATISTICS</command> will create a new extended statistics
- object on the specified table.
+ object on the specified table, foreign table or materialized view.
The statistics will be created in the current database and
will be owned by the user issuing the command.
</para>
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 46abadc..2355e0e 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -106,10 +106,12 @@ CreateStatistics(CreateStatsStmt *stmt)
relid = RelationGetRelid(rel);
if (rel->rd_rel->relkind != RELKIND_RELATION &&
- rel->rd_rel->relkind != RELKIND_MATVIEW)
+ rel->rd_rel->relkind != RELKIND_MATVIEW &&
+ rel->rd_rel->relkind != RELKIND_FOREIGN_TABLE &&
+ rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("relation \"%s\" is not a table or materialized view",
+ errmsg("relation \"%s\" is not a table, foreign table, or materialized view",
RelationGetRelationName(rel))));
/*
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 65a2f23..57d034f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -6626,9 +6626,14 @@ getExtendedStatistics(Archive *fout, TableInfo tblinfo[], int numTables)
{
TableInfo *tbinfo = &tblinfo[i];
- /* Only plain tables and materialized views can have extended statistics. */
+ /*
+ * Only plain tables, materialized views, foreign tables and
+ * partitioned tables can have extended statistics.
+ */
if (tbinfo->relkind != RELKIND_RELATION &&
- tbinfo->relkind != RELKIND_MATVIEW)
+ tbinfo->relkind != RELKIND_MATVIEW &&
+ tbinfo->relkind != RELKIND_FOREIGN_TABLE &&
+ tbinfo->relkind != RELKIND_PARTITIONED_TABLE)
continue;
/*
This isn't exactly about this particular thread. But I noticed, that
after we introduced RELKIND_PARTITIONED_TABLE, we required to change a
number of conditions to include this relkind. We missed some places in
initial commits and fixed those later. I am wondering whether we
should creates macros clubbing relevant relkinds together based on the
purpose of the tests e.g. IS_RELKIND_HAS_STORAGE(). When a new relkind
is added, one can examine these macros to check whether the new
relkind fits in the given macro. If all those macros are placed
together, there is a high chance that we will not miss any place in
the initial commit itself.
For example, if we had a macro IS_RELKIND_WITH_STATS defined as
#define IS_RELKIND_WITH_STATS(relkind) \
((relkind) == RELKIND_RELATION || \
(relkind) == RELKIND_MATVIEW)
and CreateStatistics() and getExtendedStatistics() had following conditions
if (!IS_RELKIND_WITH_STATS(rel->rd_rel->relkind)) and if
(!IS_RELKIND_WITH_STATS(tabinfo->relkind)) resp. The patch would be
just adding
(relkind) == RELKIND_FOREIGN_TABLE || \
(relkind) == RELKIND_PARTITIONED_TABLE)
to that macro without requiring to find out where all we need to add
those two relkinds for statistics purposes.
On Mon, Apr 10, 2017 at 3:09 PM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
While reviewing extended stats I noticed that it was possible to
create extended stats on many object types, including sequences. I
mentioned that this should be disallowed. Statistics were then changed
to be only allowed on plain tables and materialized views.This should be relaxed again to allow anything ANALYZE is allowed on.
The attached does this.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database 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 Mon, Apr 10, 2017 at 09:39:22PM +1200, David Rowley wrote:
While reviewing extended stats I noticed that it was possible to
create extended stats on many object types, including sequences. I
mentioned that this should be disallowed. Statistics were then changed
to be only allowed on plain tables and materialized views.This should be relaxed again to allow anything ANALYZE is allowed on.
The attached does this.
[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Alvaro,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1]/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.
[1]: /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Noah Misch wrote:
On Mon, Apr 10, 2017 at 09:39:22PM +1200, David Rowley wrote:
While reviewing extended stats I noticed that it was possible to
create extended stats on many object types, including sequences. I
mentioned that this should be disallowed. Statistics were then changed
to be only allowed on plain tables and materialized views.This should be relaxed again to allow anything ANALYZE is allowed on.
The attached does this.
[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Alvaro,
since you committed the patch believed to have created it, you own this open
item.
I'm going to review David's patch and post a detailed review on
Wednesday 19th.
I think the patch is reasonable, but it's lacking new tests, and I'm
worried that the error message changes do not affect the existing tests
in the area.
--
�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
On 4/10/17 06:18, Ashutosh Bapat wrote:
This isn't exactly about this particular thread. But I noticed, that
after we introduced RELKIND_PARTITIONED_TABLE, we required to change a
number of conditions to include this relkind. We missed some places in
initial commits and fixed those later. I am wondering whether we
should creates macros clubbing relevant relkinds together based on the
purpose of the tests e.g. IS_RELKIND_HAS_STORAGE(). When a new relkind
is added, one can examine these macros to check whether the new
relkind fits in the given macro. If all those macros are placed
together, there is a high chance that we will not miss any place in
the initial commit itself.
I think this is worth a try.
--
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
On Sat, Apr 15, 2017 at 5:27 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 4/10/17 06:18, Ashutosh Bapat wrote:
This isn't exactly about this particular thread. But I noticed, that
after we introduced RELKIND_PARTITIONED_TABLE, we required to change a
number of conditions to include this relkind. We missed some places in
initial commits and fixed those later. I am wondering whether we
should creates macros clubbing relevant relkinds together based on the
purpose of the tests e.g. IS_RELKIND_HAS_STORAGE(). When a new relkind
is added, one can examine these macros to check whether the new
relkind fits in the given macro. If all those macros are placed
together, there is a high chance that we will not miss any place in
the initial commit itself.I think this is worth a try.
Thanks, will try to come up with something ASAP.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
David Rowley wrote:
While reviewing extended stats I noticed that it was possible to
create extended stats on many object types, including sequences. I
mentioned that this should be disallowed. Statistics were then changed
to be only allowed on plain tables and materialized views.This should be relaxed again to allow anything ANALYZE is allowed on.
The attached does this.
The error message was inconsistent in the case of indexes, because of
using heap_open instead of relation_open. That seemed gratuitous, so I
flipped it, added test for the whole thing and pushed.
Thanks for reporting.
--
�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
On 18 April 2017 at 09:01, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
David Rowley wrote:
While reviewing extended stats I noticed that it was possible to
create extended stats on many object types, including sequences. I
mentioned that this should be disallowed. Statistics were then changed
to be only allowed on plain tables and materialized views.This should be relaxed again to allow anything ANALYZE is allowed on.
The attached does this.
The error message was inconsistent in the case of indexes, because of
using heap_open instead of relation_open. That seemed gratuitous, so I
flipped it, added test for the whole thing and pushed.
Thanks for changing that and pushing this.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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