Documentation improvements for partitioning

On Fri, Feb 3, 2017 at 4:15 AM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
wrote:

Here are some patches to improve the documentation about partitioned
tables:

0001: Adds some details about partition_bound_spec to the CREATE TABLE
page, especially:

- a note about inclusivity of range partition bounds,
- a note about the UNBOUNDED literal in case of range partitioning,
- a note about the NULL literal in case of list partitioning,

I wonder if the above "note" should be added under the Notes section or
are they fine to be added as part of the description of PARTITION OF
clause. Also:

- in syntax synopsis, it appears now that any expression is OK to be used
for individual bound datum, but it's not true. Only literals are
allowed. So fixed that.
- added an example showing how to create partitions of a range
partitioned table with multiple columns in the partition key
- added PARTITION BY and PARTITION OF (FOR VALUES) as PostgreSQL
extensions in the compatibility section

0002: Adds details about partitioned tables to the DDL chapter (ddl.sgml)

- a new section named "Partitioned Tables" right next to the
Inheritance and Partitioning sections is created.
- examples are added to the existing Partitioning section using the new
partitioned tables. Old text about implementing table partitioning
using inheritance is kept, sort of as a still supported older
alternative.

0003: Add partitioning keywords to keywords.sgml

This is all I have for now. Any feedback is greatly appreciated. Adding
this to the next CF.

Thanks,
Amit

Patch applies.

Overall this looks really good. It goes a long way towards stating some of
the things I had to learn through experimentation.

I had to read a really long way into the patch before finding a blurb that
I felt wasn't completely clear:

+
+     <para>
+      <command>INSERT</command> statements with <literal>ON CONFLICT</>
+      clause are currently not allowed on partitioned tables, that is,
+      cause error when specified.
+     </para>

Here's some other tries at saying the same thing, none of which are
completely satisfying:

...ON CONFLICT clause are currently not allowed on partitioned tables and
will cause an error?
...ON CONFLICT clause are currently not allowed on partitioned tables and
will instead cause an error?
...ON CONFLICT clause will currently cause an error if used on a
partitioned table?

As far as additional issues to cover, this bit:

+     <listitem>
+      <para>
+       One cannot drop a <literal>NOT NULL</literal> constraint on a
+       partition's column, if the constraint is present in the parent
table.
+      </para>
+     </listitem>

Maybe we should add something about how one would go about dropping a NOT
NULL constraint (parent first then partitions?)

In reviewing this patch, do all our target formats make word spacing
irrelevant? i.e. is there any point in looking at the number of spaces
after a period, etc?

A final note, because I'm really familiar with partitioning on Postgres and
other databases, documentation which is clear to me might not be to someone
less familiar with partitioning. Maybe we want another reviewer for that?

Hi Corey,

On 2017/02/09 6:14, Corey Huinker wrote:

On Fri, Feb 3, 2017 at 4:15 AM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
wrote:

Here are some patches to improve the documentation about partitioned
tables:

Patch applies.

Overall this looks really good. It goes a long way towards stating some of
the things I had to learn through experimentation.

Thanks a lot for taking a look at it.

I had to read a really long way into the patch before finding a blurb that
I felt wasn't completely clear:

+
+     <para>
+      <command>INSERT</command> statements with <literal>ON CONFLICT</>
+      clause are currently not allowed on partitioned tables, that is,
+      cause error when specified.
+     </para>

Here's some other tries at saying the same thing, none of which are
completely satisfying:

...ON CONFLICT clause are currently not allowed on partitioned tables and
will cause an error?
...ON CONFLICT clause are currently not allowed on partitioned tables and
will instead cause an error?
...ON CONFLICT clause will currently cause an error if used on a
partitioned table?

The last one sounds better.

As far as additional issues to cover, this bit:

+     <listitem>
+      <para>
+       One cannot drop a <literal>NOT NULL</literal> constraint on a
+       partition's column, if the constraint is present in the parent
table.
+      </para>
+     </listitem>

Maybe we should add something about how one would go about dropping a NOT
NULL constraint (parent first then partitions?)

Dropping it on the parent will cause it to be dropped on the partitions as
well. About your point whether we should add a note about how to go about
dropping it in the partition, it seems to me it would be out of place
here; it's just saying that dropping NOT NULL constraint has a different
behavior with partitioned tables than regular inheritance. That note most
likely belongs in the ALTER TABLE reference page in the DROP NOT NULL
description, so created a patch for that (patch 0004 of the attached patches).

In reviewing this patch, do all our target formats make word spacing
irrelevant? i.e. is there any point in looking at the number of spaces
after a period, etc?

It seems to be a convention in the sources to include 2 spaces after a
period, which I just try to follow (both in the code comments and SGML).
I don't see that spaces are relevant as far as how the targets such as
HTML are rendered.

A final note, because I'm really familiar with partitioning on Postgres and
other databases, documentation which is clear to me might not be to someone
less familiar with partitioning. Maybe we want another reviewer for that?

More eyeballs will only help make this better.

Thanks,
Amit

On 10 February 2017 at 07:35, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

A final note, because I'm really familiar with partitioning on Postgres and
other databases, documentation which is clear to me might not be to someone
less familiar with partitioning. Maybe we want another reviewer for that?

More eyeballs will only help make this better.

Given that we already have partitioning feature committed, we really
need to have the docs committed as well.

Without claiming I'm happy about this, I think the best way to improve
the number of eyeballs on this is to commit these docs as is.

For me, the most important thing is understanding the feature, not
(yet) discussing what the docs should look like. This is especially
true if other patches reference the way partitioning works and nobody
can comment on those patches because they don't understand

Any issues with that?

--
Simon Riggs 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 2017/02/10 17:00, Simon Riggs wrote:

On 10 February 2017 at 07:35, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

A final note, because I'm really familiar with partitioning on Postgres and
other databases, documentation which is clear to me might not be to someone
less familiar with partitioning. Maybe we want another reviewer for that?

More eyeballs will only help make this better.

Given that we already have partitioning feature committed, we really
need to have the docs committed as well.

Without claiming I'm happy about this, I think the best way to improve
the number of eyeballs on this is to commit these docs as is.

For me, the most important thing is understanding the feature, not
(yet) discussing what the docs should look like. This is especially
true if other patches reference the way partitioning works and nobody
can comment on those patches because they don't understand

Any issues with that?

I agree that getting the proposed documentation changes committed would be
a step ahead. I saw in the logical replication thread that dealing with
partitioned tables without the docs explaining what they are has been
difficult. Hopefully the proposed documentation improvements help make
progress in that regard. Partitioned tables, at this point, have certain
limitations which affect its interaction with other features (old or new);
documenting those limitations will be helpful not only to the users
deciding whether to start using the new partitioned tables right away, but
also to the developers of other features who want to understand what
partitioned tables are.

Thanks,
Amit

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 10 February 2017 at 08:18, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

I agree that getting the proposed documentation changes committed would be
a step ahead.

Committed. I tested how it works and added documentation that matched
my experiences, correcting what you'd said and adding more information
for clarity as I went.

Few points from tests

* "ERROR: cannot create index on partitioned table "measurement_year_month""
is misleading because you can create indexes on partitions

* You can create additional CHECK (column is NOT NULL) as a check
constraint, even if you can't create a not null constraint

* The OID inheritance needs work - you shouldn't need to specify a
partition needs OIDS if the parent has it already.

* There's no tab completion, which prevents people from testing this
(maybe better now with some docs)

* ERROR: no partition of relation "measurement_year_month" found for row
DETAIL: Failing row contains (2016-12-02, 1, 1).
should not return the whole row, just the partition keys

* It's very weird you can't DROP a partitioned table. I think you need
to add dependencies.

* ERROR: TO must specify exactly one value per partitioning column
should say something more like "you specified one column value,
whereas the partitioning key requires two columns"

Good work so far, but there is still a very significant amount of work
to do. And as this feature evolves it must now contain full
documentation at every step, otherwise it won't be able to receive
full and fair review. So please make sure each new patch contains docs
changes for that patch.

--
Simon Riggs 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 2017/02/10 19:19, Simon Riggs wrote:

On 10 February 2017 at 08:18, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

I agree that getting the proposed documentation changes committed would be
a step ahead.

Committed. I tested how it works and added documentation that matched
my experiences, correcting what you'd said and adding more information
for clarity as I went.

Thanks for making the improvements to and committing the patch!

Few points from tests

* "ERROR: cannot create index on partitioned table "measurement_year_month""
is misleading because you can create indexes on partitions

Do you mean that this should not cause an error at all, but create the
specified index on partitions as part of running the command? Should the
code to handle that be part of this release?

Or do you simply mean that we should rewrite the error message and/or add
a HINT asking to create the index on partitions instead?

* You can create additional CHECK (column is NOT NULL) as a check
constraint, even if you can't create a not null constraint

Sorry but I am not exactly sure which "cannot create a not null
constraint" you are referring to here.

There are no restrictions on *creating* constraints on partitions, but
there are on dropping. Regular inheritance rules prevent dropping
inherited constraints (just the same for partitioned tables), of which
there are only named CHECK constraints at the moment. A new rule
introduced for partitions prevents dropping a column's NOT NULL constraint
if it's been "inherited" (i.e., the same constraint is present in the
parent partitioned table), although it's not in the same sense as for
CHECK constraints, because NOT NULL constraints are not tracked with
pg_constraints.

* The OID inheritance needs work - you shouldn't need to specify a
partition needs OIDS if the parent has it already.

That sounds right. It's better to keep the behavior same as for regular
inheritance. Will post a patch to get rid of the unnecessary check.

FWIW, the check was added to prevent the command from succeeding in the
case where WITHOUT OIDS has been specified for a partition and the parent
partitioned table has the OID column. Regular inheritance simply
*overrides* the WITHOUT OIDS specification, which might be seen as surprising.

* There's no tab completion, which prevents people from testing this
(maybe better now with some docs)

Will post a patch as well.

* ERROR: no partition of relation "measurement_year_month" found for row
DETAIL: Failing row contains (2016-12-02, 1, 1).
should not return the whole row, just the partition keys

I think that makes sense. Something along the lines of
BuildIndexValueDescription() for partition keys will be necessary. Will
post a patch.

* It's very weird you can't DROP a partitioned table. I think you need
to add dependencies.

Do you mean it should be possible to DROP a partitioned table without
needing to specify CASCADE? Currently, same thing happens for a
partitioned table as will for a inheritance parent.

* ERROR: TO must specify exactly one value per partitioning column
should say something more like "you specified one column value,
whereas the partitioning key requires two columns"

Should that be a DETAIL or HINT message?

Good work so far, but there is still a very significant amount of work
to do. And as this feature evolves it must now contain full
documentation at every step, otherwise it won't be able to receive
full and fair review. So please make sure each new patch contains docs
changes for that patch.

Agreed that comprehensive documentation of any new feature is crucial both
during development and after the feature is released.

Thanks,
Amit

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2017/02/13 14:21, Amit Langote wrote:

On 2017/02/10 19:19, Simon Riggs wrote:

* The OID inheritance needs work - you shouldn't need to specify a
partition needs OIDS if the parent has it already.

That sounds right. It's better to keep the behavior same as for regular
inheritance. Will post a patch to get rid of the unnecessary check.

FWIW, the check was added to prevent the command from succeeding in the
case where WITHOUT OIDS has been specified for a partition and the parent
partitioned table has the OID column. Regular inheritance simply
*overrides* the WITHOUT OIDS specification, which might be seen as surprising.

0001 of the attached patches takes care of this.

* There's no tab completion, which prevents people from testing this
(maybe better now with some docs)

Will post a patch as well.

...and 0002 for this.

* ERROR: no partition of relation "measurement_year_month" found for row
DETAIL: Failing row contains (2016-12-02, 1, 1).
should not return the whole row, just the partition keys

I think that makes sense. Something along the lines of
BuildIndexValueDescription() for partition keys will be necessary. Will
post a patch.

Let me spend a bit more time on this one.

Thanks,
Amit

On 2017/02/13 14:21, Amit Langote wrote:

On 2017/02/10 19:19, Simon Riggs wrote:

On 10 February 2017 at 08:18, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

I agree that getting the proposed documentation changes committed would be
a step ahead.

Committed. I tested how it works and added documentation that matched
my experiences, correcting what you'd said and adding more information
for clarity as I went.

Thanks for making the improvements to and committing the patch!

Since I had added this to CF 2017-03, I have marked it as committed.

https://commitfest.postgresql.org/13/983/

Thanks,
Amit

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Feb 10, 2017 at 3:00 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

Given that we already have partitioning feature committed, we really
need to have the docs committed as well.

Just for the record, it's not like there were no documentation changes
in the originally committed patch. In fact there were over 400 lines
of documentation:

doc/src/sgml/catalogs.sgml | 129 +++++++++++++++++++++++-
doc/src/sgml/ref/alter_table.sgml | 117 +++++++++++++++++++++-
doc/src/sgml/ref/create_foreign_table.sgml | 26 +++++
doc/src/sgml/ref/create_table.sgml | 154 +++++++++++++++++++++++++++++

The patch you committed approximately doubles the amount of
documentation for this feature, which is fine as far as it goes, but
the key points were all explained in the original commit. I have been
known to leave out documentation from commits from time to time and
fill it in after-the-fact, but that's not really what happened here.

Without claiming I'm happy about this, I think the best way to improve
the number of eyeballs on this is to commit these docs as is.

For me, the most important thing is understanding the feature, not
(yet) discussing what the docs should look like. This is especially
true if other patches reference the way partitioning works and nobody
can comment on those patches because they don't understand

Any issues with that?

There are a number of things that I think are awkward about the patch
as committed:

+      <listitem>
+       <para>
+        See last section for some general information:
+        <xref linkend="ddl-partitioned-tables">
+       </para>
+      </listitem>

I think we generally try to write the documentation in such a way as
to minimize backward references, and a backward reference to the
previous section seems particularly odd. We've now got section "5.10
Partitioned Tables" followed immediately by section "5.11
Partitioning", where the latter seems to think that you haven't read
the former.

I think that section 5.11 needs a much heavier rewrite than what it
got as part of this patch. It's a hodgepodge of the old content
(which explained how to fake partitioning when we didn't have an
explicit concept of partitioning) and new content that talks about how
the new way is different from the old way. But now that we have the
new way, I'm guessing that most people are going to use that and not
care about the old way any more. I'm not that it's even appropriate
to keep the lengthy explanation of how to fake table partitioning
using table inheritance and non-overlapping CHECK constraints, but if
we still want that stuff it should be de-emphasized more than it is
here. Probably the section should be retitled: in previous releases
we called this "Partitioning" because we had no explicit notion of
partitioning, but now that we do, it's confusing to have a section
called "Partitioning" that explains how to avoid using the
partitioning feature, which is more or less what this does. Or maybe
the section title should stay the same (or this should be merged into
the previous section?) but rewritten to change the emphasis.

--
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 Mon, Feb 13, 2017 at 5:57 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2017/02/13 14:21, Amit Langote wrote:

On 2017/02/10 19:19, Simon Riggs wrote:

* The OID inheritance needs work - you shouldn't need to specify a
partition needs OIDS if the parent has it already.

That sounds right. It's better to keep the behavior same as for regular
inheritance. Will post a patch to get rid of the unnecessary check.

FWIW, the check was added to prevent the command from succeeding in the
case where WITHOUT OIDS has been specified for a partition and the parent
partitioned table has the OID column. Regular inheritance simply
*overrides* the WITHOUT OIDS specification, which might be seen as surprising.

0001 of the attached patches takes care of this.

I think 0001 needs to remove this hunk of documentation:

<listitem>
<para>
If the partitioned table specified <literal>WITH OIDS</literal> then
each partition must also specify <literal>WITH OIDS</literal>. Oids
are not automatically inherited by partitions.
</para>
</listitem>

I think 0001 is better than the status quo, but I'm wondering whether
we should try to do something slightly different. Maybe it should
always work for the child table to specify neither WITH OIDS nor
WITHOUT OIDS, but if you do specify one of them then it has to be the
one that matches the parent partitioned table? With this patch, IIUC,
WITH OIDS is allowed only if the parent has the same, but WITHOUT OIDS
is allowed (but ignored) regardless of the parent setting.

--
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

Noticed some typos in the documentation. Here's patch to correct
those. Sorry, if it has been already taken care of.

On Wed, Feb 15, 2017 at 10:01 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Feb 13, 2017 at 5:57 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

On 2017/02/13 14:21, Amit Langote wrote:

On 2017/02/10 19:19, Simon Riggs wrote:

* The OID inheritance needs work - you shouldn't need to specify a
partition needs OIDS if the parent has it already.

That sounds right. It's better to keep the behavior same as for regular
inheritance. Will post a patch to get rid of the unnecessary check.

FWIW, the check was added to prevent the command from succeeding in the
case where WITHOUT OIDS has been specified for a partition and the parent
partitioned table has the OID column. Regular inheritance simply
*overrides* the WITHOUT OIDS specification, which might be seen as surprising.

0001 of the attached patches takes care of this.

I think 0001 needs to remove this hunk of documentation:

<listitem>
<para>
If the partitioned table specified <literal>WITH OIDS</literal> then
each partition must also specify <literal>WITH OIDS</literal>. Oids
are not automatically inherited by partitions.
</para>
</listitem>

I think 0001 is better than the status quo, but I'm wondering whether
we should try to do something slightly different. Maybe it should
always work for the child table to specify neither WITH OIDS nor
WITHOUT OIDS, but if you do specify one of them then it has to be the
one that matches the parent partitioned table? With this patch, IIUC,
WITH OIDS is allowed only if the parent has the same, but WITHOUT OIDS
is allowed (but ignored) regardless of the parent setting.

--
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

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On Wed, Feb 15, 2017 at 4:26 AM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:

Noticed some typos in the documentation. Here's patch to correct
those. Sorry, if it has been already taken care of.

Thanks. That is indeed nonstandard capitalization. Committed.

--
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 13 February 2017 at 05:21, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

Few points from tests

* "ERROR: cannot create index on partitioned table "measurement_year_month""
is misleading because you can create indexes on partitions

Do you mean that this should not cause an error at all, but create the
specified index on partitions as part of running the command? Should the
code to handle that be part of this release?

Sounds fairly basic to me. If you don't support this, then presumably
every ORM, pgAdmin etc will all be broken.

And 1000 people will need to write a script that does what we could do
easily in a loop internally.

At present you haven't even documented how you'd do this.

I see you might want to create an index on a subset of partitions, but
I expect that you've thought of that and have a proposal for syntax
that allows it. Though the default should be that CREATE INDEX just
works.

Or do you simply mean that we should rewrite the error message and/or add
a HINT asking to create the index on partitions instead?

You could

* It's very weird you can't DROP a partitioned table. I think you need
to add dependencies.

Do you mean it should be possible to DROP a partitioned table without
needing to specify CASCADE? Currently, same thing happens for a
partitioned table as will for a inheritance parent.

If we wanted them to act identically we wouldn't have any need for a
new feature at all, so clearly that doesn't make sense as an argument.

If I issue DROP TABLE elsewhere, it doesn't refuse to drop because it
has indexes, sequences etc on it. So why should it just because it has
partitions? Most especially if they were created automatically for me
in the first place. I might just understand that running ATTACH TABLE
might change that viewpoint.

I'm pretty sure DROP TABLE and CREATE INDEX are fairly basic
expectations from users about how tables should work, partitioned or
otherwise.

It leaves me asking what else is missing.

--
Simon Riggs 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 02/15/2017 06:10 AM, Simon Riggs wrote:

On 13 February 2017 at 05:21, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

If I issue DROP TABLE elsewhere, it doesn't refuse to drop because it
has indexes, sequences etc on it. So why should it just because it has
partitions?

Because partitions may have data.

JD

--
Command Prompt, Inc. http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Joshua D. Drake wrote:

On 02/15/2017 06:10 AM, Simon Riggs wrote:

On 13 February 2017 at 05:21, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

If I issue DROP TABLE elsewhere, it doesn't refuse to drop because it
has indexes, sequences etc on it. So why should it just because it has
partitions?

Because partitions may have data.

So would the table, were it not partitioned.

--
�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 Wed, Feb 15, 2017 at 9:37 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Joshua D. Drake wrote:

On 02/15/2017 06:10 AM, Simon Riggs wrote:

On 13 February 2017 at 05:21, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:

If I issue DROP TABLE elsewhere, it doesn't refuse to drop because it
has indexes, sequences etc on it. So why should it just because it has
partitions?

Because partitions may have data.

So would the table, were it not partitioned.

True. I think the question here is: do we want to view the dependency
between a partitioned table and a partition of that table as
DEPENDENCY_NORMAL or as DEPENDENCY_AUTO? With table inheritance, it's
always been "normal" and I'm not sure there's any good reason for
partitioning to make the opposite decision. The new partitioning
implementation provides a user experience that is overall smoother
than doing the same thing with inheritance, but it's not as if you can
ignore the fact that your partitioned tables have sub-objects that are
also tables.

--
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 Wed, Feb 15, 2017 at 9:10 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

* "ERROR: cannot create index on partitioned table "measurement_year_month""
is misleading because you can create indexes on partitions

Do you mean that this should not cause an error at all, but create the
specified index on partitions as part of running the command? Should the
code to handle that be part of this release?

Sounds fairly basic to me. If you don't support this, then presumably
every ORM, pgAdmin etc will all be broken.

I don't see why that should be the case.

And 1000 people will need to write a script that does what we could do
easily in a loop internally.

Now that is probably true.

At present you haven't even documented how you'd do this.

It's not that hard to figure it out, though. A HINT wouldn't be a bad
idea, certainly.

There are some really thorny problems with making index creation
cascade to all of the partitions. I think it's worth doing, but
there's a lot of stuff to think about before you go and start writing
code. Most obviously, if you can use a single CREATE INDEX statement
to create indexes on all of the partitions, then probably you ought to
also be able to use DROP INDEX to get rid of all of those indexes. In
other words, it should probably work a lot like what already happens
with constraints: constraints cascade from the parent down to the
children, but we still know which child object goes with which parent
object, so if the parent object is dropped we can get rid of all of
the children. I think we need something similar here, although if we
restrict it to the partitioning case and don't make it work with table
inheritance then it can be simpler since table partitioning doesn't
allow multiple inheritance. Presumably we'd want other index commands
like REINDEX to cascade similarly.

Also, it's not entirely clear what the semantics should be. If the
partitioning key is (a) and you ask for an index on (a, b), you could
conceivably omit a from the indexes created on partitions that only
cover a single value of a. (That case is easy to detect when list
partitioning is in use.) Should we try do that elimination, or just
do what the user asked for? Will users be unhappy if we try to do
this sort of column elimination but it only works in simple cases?
Think about the possibility that there are partitioning expressions
rather than partitioning columns before concluding we can make it work
in all cases. On the other hand, if you ask for a UNIQUE index on
(b), should we go ahead and create such an index on each partition,
ensuring uniqueness within each partition, or should we refuse to
proceed on the grounds that we can't be sure that such an index will
ensure global uniqueness? If you do the former, someone might find
the behavior surprising, but if you do the latter, you might annoy
people who know what they're asking for and want that thing but can't
get it. I suspect we want to eventually allow a user to ask for
either one, because eventually we'll probably have global indexes, and
then you really need a way to say whether you want a global index or a
partitioned non-global index. But that requires agreeing on syntax,
which is complicated and will probably involve a lot of bikeshedding
(as well it should - these are big decisions).

I think it would be a bad idea to try to fix this problem for v10.
One of the earlier versions of the patch allowed indexes on the parent
table as if it were just a regular empty table, which did not seem
useful. I asked him to disallow that so as to keep our options open
for the future. I see no reason why v11 or v12 can't fill in the
functionality in this area. Right now we're 2 weeks away from the
start of the last CommitFest, and that's not the time to go start
writing a complex patch for a feature that isn't even particularly
well-defined. If somebody really cared about this
make-an-index-for-everything-in-the-hierarchy problem, they could've
written a patch for that at any time in the last 5 years; it's not
strictly dependent on the new partitioning stuff. Nobody's done that,
and trying to throw together something now in the last couple of weeks
could easily end with us getting it wrong and then having to face the
unpleasant prospect of either leaving it broken or breaking backward
compatibility to fix it.

It leaves me asking what else is missing.

There is certainly a lot of room for improvement here but I don't
understand your persistent negativity about what's been done thus far.
I think it's pretty clearly a huge step forward, and I think Amit
deserves a ton of credit for making it happen. The improvements in
bulk loading performance alone are stupendous. You apparently have
the idea that somebody could have written an even larger patch that
solved even more problems at once, but this was already a really big
patch, and IMHO quite a good one.

--
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

Robert Haas wrote:

On Wed, Feb 15, 2017 at 9:37 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Joshua D. Drake wrote:

Because partitions may have data.

So would the table, were it not partitioned.

True. I think the question here is: do we want to view the dependency
between a partitioned table and a partition of that table as
DEPENDENCY_NORMAL or as DEPENDENCY_AUTO? With table inheritance, it's
always been "normal" and I'm not sure there's any good reason for
partitioning to make the opposite decision.

I think new-style partitioning is supposed to consider each partition as
an implementation detail of the table; the fact that you can manipulate
partitions separately does not really mean that they are their own
independent object. You don't stop to think "do I really want to drop
the TOAST table attached to this main table?" and attach a CASCADE
clause if so. You just drop the main table, and the toast one is
dropped automatically. I think new-style partitions should behave
equivalently.

You can make the partition an independent entity, but if you don't
explicitly take that step beforehand, I don't see why we should see it
that way implicitly.

The new partitioning
implementation provides a user experience that is overall smoother
than doing the same thing with inheritance, but it's not as if you can
ignore the fact that your partitioned tables have sub-objects that are
also tables.

Now that partitions are declarative, the underlying implementation could
change away from inheritance. It's now just an implementation artifact.

--
�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 Wed, Feb 15, 2017 at 11:34 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

I think new-style partitioning is supposed to consider each partition as
an implementation detail of the table; the fact that you can manipulate
partitions separately does not really mean that they are their own
independent object. You don't stop to think "do I really want to drop
the TOAST table attached to this main table?" and attach a CASCADE
clause if so. You just drop the main table, and the toast one is
dropped automatically. I think new-style partitions should behave
equivalently.

That's a reasonable point of view. I'd like to get some more opinions
on this topic. I'm happy to have us do whatever most people want, but
I'm worried that having table inheritance and table partitioning work
differently will be create confusion. I'm also suspicious that there
may be some implementation difficulties. On the hand, it does seem a
little silly to say that DROP TABLE partitioned_table should always
fail except in the degenerate case where there are no partitions, so
maybe changing it is for the best.

Now that partitions are declarative, the underlying implementation could
change away from inheritance. It's now just an implementation artifact.

I don't really agree with that. It's true that, for example, we could
decide to store the inheritance information for partitions someplace
other than pg_inherits, but from a user perspective these are always
going to be closely-related features. Also, there are quite a number
of ways in which it's very noticeable that the objects are separate.
They are dumped separately. They have separate indexes, and even if
we provide some facility to create a common indexing scheme across all
partitions automatically, you'll still be able to REINDEX or CLUSTER
one of those tables individually. They can have different storage
properties, like one can be UNLOGGED while another is not. They show
up in EXPLAIN plans. The partitioning structure affects what you can
and can't do with foreign keys. All of those are user-visible things
that make this look and feel like a collection of tables, not just a
single table that happens to have several relfilenodes under the hood.
I think that's actually a really important feature, not a design
defect.

As you may or may not know, EDB has had a proprietary implementation
of table partitioning since Advanced Server 9.1, and one of the things
we've learned from that implementation is that users really like to be
able to fiddle with the individual partitions. They want to things
like make them individually unlogged, rename them, vacuum them, add
contraints, add triggers, put them in different tablespaces, vary
indexing strategies, all the stuff that you normally do with
standalone tables. Any time one of those things didn't work quite
like it would for a standalone table, we got complaints. One of the
things that has become really clear over the five years that feature
has been out of the field is that users value the ability to do
different things with different child tables. Now that of course does
not mean that they don't want to be able to operate on the hierarchy
as a whole; we have numerous feature requests in that direction, too.
But, at least among the base of customers that have talked to us about
the proprietary partitioning feature in Advanced Server, wanting to
treat a partition as a table and do some arbitrary thing to it that
can be done to a table is extremely common. Of course, I can't
promise (and am not trying to argue) that the reaction among
PostgreSQL users generally will necessarily be similar to the
experience we've had with our Advanced Server customers, but this
experience has definitely caused me to lean in the direction of
wanting partitions to be first-class objects.

--
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