Re: pgsql: Allow UNIQUE indexes on partitioned tables

[Amit Langote]

On 2018/07/17 13:57, Alvaro Herrera wrote:
> On 2018-Feb-19, David G. Johnston wrote:
> 
>> As an aside, adding a link to "Data Definiton/Table Partitioning" from at
>> least CREATE TABLE ... PARTITION BY; and swapping "PARTITION BY" and
>> "PARTITION OF" in the Parameters section of that page - one must partition
>> by a table before one can partition it (and also the synopsis lists them in
>> the BY before OF order), would be helpful.
> 
> A little late, I have done these two changes.

A belated +1.

Thanks,
Amit

Re: pgsql: Allow UNIQUE indexes on partitioned tables

[Alvaro Herrera]

On 2018-Feb-19, David G. Johnston wrote:

> As an aside, adding a link to "Data Definiton/Table Partitioning" from at
> least CREATE TABLE ... PARTITION BY; and swapping "PARTITION BY" and
> "PARTITION OF" in the Parameters section of that page - one must partition
> by a table before one can partition it (and also the synopsis lists them in
> the BY before OF order), would be helpful.

A little late, I have done these two changes.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Allow UNIQUE indexes on partitioned tables

[Alvaro Herrera]

David G. Johnston wrote:

> Something like:
> 
> When establishing a unique constraint for a multi-level partition hierarchy
> all the "partition by" columns of the target partitioned table, as well as
> those of all its descendant partitioned tables, must be included in the
> constraint definition.

Yeah, that seems better to me.  Pushed.

> If I understand the above then the following failing test would be a worthy
> addition to memorialize the behavior of ALTER TABLE ATTACH under this
> constraint.
> 
> create table idxpart (a int primary key, b int) partition by range (a);
> create table idxpart1 (a int not null, b int, primary key (a, b)) partition
> by range (a, b);
> alter table idxpart attach partition idxpart1 for values from (1) to (1000);

Included this one too.


Thanks for reading!

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Allow UNIQUE indexes on partitioned tables

[Alvaro Herrera]

David Rowley wrote:
> On 20 February 2018 at 09:40, Alvaro Herrera  wrote:
> > Modified Files
> > --
> > doc/src/sgml/ddl.sgml |   9 +-
> 
> Attached is a very small fix to a small error this patch created in the docs.

Pushed, thanks.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Allow UNIQUE indexes on partitioned tables

[David G. Johnston]

On Tue, Feb 20, 2018 at 8:36 AM, Alvaro Herrera 
wrote:

> Many thanks for reading through it!
>
> David G. Johnston wrote:
> > I found the following change to be confusing.
>  [...]
> > I was expecting the doc for ADD CONSTRAINT USING INDEX to note the
> > limitation explicitly - in lieu of the above paragraph.
>
> Agreed.  I moved the note to ADD CONSTRAINT and added a different on to
> ADD CONSTRAINT USING INDEX.
>
> > Also, I cannot reason out what the following limitation means:
> >
> > /doc/src/sgml/ref/create_table.sgml
> > +  If any partitions are in turn partitioned, all columns of each
> > partition
> > +  key are considered at each level below the
> UNIQUE
> > +  constraint.
>
> I can see that being unclear.  I tried to be very concise, to avoid
> spending too many words on what is mostly a fringe feature; but that
> probably didn't work very well.  Wording suggestions welcome.
>
​[...]​


> then you may create a unique or PK constraint on t only if you include
> both columns (a,b).  You may not create a PK on t (a), which is a bit
> surprising since (b) is not part of the partition key of t directly,
> only of t_1.
>
> Of course, if you create a unique constraint on t_1 (i.e. it doesn't
> cover all of t) then you may use (b) alone -- that's what "each level
> below the UNIQUE constraint" supposed to convey.
>

Something like:

When establishing a unique constraint for a multi-level partition hierarchy
all the "partition by" columns of the target partitioned table, as well as
those of all its descendant partitioned tables, must be included in the
constraint definition.

If I understand the above then the following failing test would be a worthy
addition to memorialize the behavior of ALTER TABLE ATTACH under this
constraint.

create table idxpart (a int primary key, b int) partition by range (a);
create table idxpart1 (a int not null, b int, primary key (a, b)) partition
by range (a, b);
alter table idxpart attach partition idxpart1 for values from (1) to (1000);
​

>
> I have trouble coming up with a real-world example where you would run
> into this limitation in practice.
>

​Indeed
​
David J.

Re: pgsql: Allow UNIQUE indexes on partitioned tables

[Alvaro Herrera]

Many thanks for reading through it!

David G. Johnston wrote:
> I found the following change to be confusing.
 [...]
> I was expecting the doc for ADD CONSTRAINT USING INDEX to note the
> limitation explicitly - in lieu of the above paragraph.

Agreed.  I moved the note to ADD CONSTRAINT and added a different on to
ADD CONSTRAINT USING INDEX.

> Also, I cannot reason out what the following limitation means:
> 
> /doc/src/sgml/ref/create_table.sgml
> +  If any partitions are in turn partitioned, all columns of each
> partition
> +  key are considered at each level below the UNIQUE
> +  constraint.

I can see that being unclear.  I tried to be very concise, to avoid
spending too many words on what is mostly a fringe feature; but that
probably didn't work very well.  Wording suggestions welcome.  What this
means is that if you create a partition that is partitioned on a column
different from its parent, then a primary key that covers the whole
hierarchy (i.e. you're not just adding a PK to the partitioned
partition) must include all partition columns, not just the upper one.

Example:

create table t (a int, b int) partition by range (a);
create table t_1 partition of t for values from (0) to (1000) partition by 
range (b);

then you may create a unique or PK constraint on t only if you include
both columns (a,b).  You may not create a PK on t (a), which is a bit
surprising since (b) is not part of the partition key of t directly,
only of t_1.

Of course, if you create a unique constraint on t_1 (i.e. it doesn't
cover all of t) then you may use (b) alone -- that's what "each level
below the UNIQUE constraint" supposed to convey.

I have trouble coming up with a real-world example where you would run
into this limitation in practice.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Allow UNIQUE indexes on partitioned tables

[David G. Johnston]

I found the following change to be confusing.

/doc/src/sgml/ref/alter_table.sgml
+   
+Additional restrictions apply when unique indexes are applied to
+partitioned tables; see .
+   

That paragraph appears in the section covering "ALTER TABLE name ADD
table_constraint_using_index"

However, the code says:

/src/backend/commands/tablecmds.c
+   /*
+* Doing this on partitioned tables is not a simple feature to
implement,
+* so let's punt for now.
+*/
+   if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+   ereport(ERROR,
+   (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+errmsg("ALTER TABLE / ADD CONSTRAINT USING INDEX is not
supported on partitioned tables")));

I was expecting the doc for ADD CONSTRAINT USING INDEX to note the
limitation explicitly - in lieu of the above paragraph.

Also, I cannot reason out what the following limitation means:

/doc/src/sgml/ref/create_table.sgml
+  If any partitions are in turn partitioned, all columns of each
partition
+  key are considered at each level below the UNIQUE
+  constraint.


As an aside, adding a link to "Data Definiton/Table Partitioning" from at
least CREATE TABLE ... PARTITION BY; and swapping "PARTITION BY" and
"PARTITION OF" in the Parameters section of that page - one must partition
by a table before one can partition it (and also the synopsis lists them in
the BY before OF order), would be helpful.

David J.


On Mon, Feb 19, 2018 at 1:40 PM, Alvaro Herrera 
wrote:

> Allow UNIQUE indexes on partitioned tables
>
> If we restrict unique constraints on partitioned tables so that they
> must always include the partition key, then our standard approach to
> unique indexes already works --- each unique key is forced to exist
> within a single partition, so enforcing the unique restriction in each
> index individually is enough to have it enforced globally.  Therefore we
> can implement unique indexes on partitions by simply removing a few
> restrictions (and adding others.)
>
> Discussion: https://postgr.es/m/2017112921.hi6hg6pem2w2t36z@alvherre.
> pgsql
> Discussion: https://postgr.es/m/20171229230607.3iib6b62fn3uaf47@alvherre.
> pgsql
> Reviewed-by: Simon Riggs, Jesper Pedersen, Peter Eisentraut, Jaime
> Casanova, Amit Langote
>
> Branch
> --
> master
>
> Details
> ---
> https://git.postgresql.org/pg/commitdiff/eb7ed3f3063401496e4aa4bd68fa33
> f0be31a72f
>
> Modified Files
> --
> doc/src/sgml/ddl.sgml |   9 +-
> doc/src/sgml/ref/alter_table.sgml |  15 +-
> doc/src/sgml/ref/create_index.sgml|   5 +
> doc/src/sgml/ref/create_table.sgml|  18 +-
> src/backend/bootstrap/bootparse.y |   2 +
> src/backend/catalog/index.c   |  50 -
> src/backend/catalog/pg_constraint.c   |  76 +++
> src/backend/catalog/toasting.c|   4 +-
> src/backend/commands/indexcmds.c  | 125 +--
> src/backend/commands/tablecmds.c  |  71 ++-
> src/backend/parser/analyze.c  |   7 +
> src/backend/parser/parse_utilcmd.c|  31 +--
> src/backend/tcop/utility.c|   1 +
> src/bin/pg_dump/t/002_pg_dump.pl  |  65 ++
> src/include/catalog/index.h   |   5 +-
> src/include/catalog/pg_constraint_fn.h|   4 +-
> src/include/commands/defrem.h |   1 +
> src/include/parser/parse_utilcmd.h|   3 +-
> src/test/regress/expected/alter_table.out |   8 -
> src/test/regress/expected/create_index.out|   6 +
> src/test/regress/expected/create_table.out|  12 --
> src/test/regress/expected/indexing.out| 294
> +-
> src/test/regress/expected/insert_conflict.out |   2 +-
> src/test/regress/sql/alter_table.sql  |   2 -
> src/test/regress/sql/create_index.sql |   6 +
> src/test/regress/sql/create_table.sql |   8 -
> src/test/regress/sql/indexing.sql | 172 ++-
> 27 files changed, 907 insertions(+), 95 deletions(-)
>
>