Horton

Thread

Commits

  1. Remove undocumented restriction against duplicate partition key columns.

  1. Problem on pg_dump RANGE partition with expressions

    Yugo Nagata <nagata@sraoss.co.jp> — 2018-07-12T07:59:39Z

    Hi,

During looking into other thread[1], I found a problem on pg_dump of range
partition table using expressions. When we create a range partitioned table, 
we cannot use a column more than　once in the partition key.
 
 postgres=# create table t (i int) partition by range(i,i);
 ERROR:  column "i" appears more than once in partition key

On the other hand, the following query using expression is allowed.

 postgres=# create table test (i int) partition by range(i,(i));
 CREATE TABLE

However, when we use pg_dump for this, we get

 CREATE TABLE public.test (
     i integer
 )
 PARTITION BY RANGE (i, i);

, and we cannot restore this due to the error.

I can consider three approaches to resolve this.


1) Allow to appear more than once in range partition key

I don't understand why there is this restriction. If we have no clear reason, 
can we rip out this restrition?

2) Forbid this kind of partition key

If we can make more checks and forbid such partition keys as RANGE(i,(i)), 
we can avoid the problem though it is a bit rigorous.

3) Treat expressions as it is

For some reasons, expressions like "(column)" or "(column COLLATE something)"
is treated like simple attributes in the current implementation
(in ComputePartitionAttr() ).

If we treat these expressions as it is, pg_dump result will be the same 
expression as when the partition table is created, and we can restore this 
successfully.

Could you give me your opinions aoubt which approach is appropriate?

Regards,

[1] https://www.postgresql.org/message-id/flat/20180712155808.49e712d8.nagata%40sraoss.co.jp#00bbfb5054c0a57f9a2fe48fae77b848

-- 
Yugo Nagata <nagata@sraoss.co.jp>

  2. Re: Problem on pg_dump RANGE partition with expressions

    Amit Langote <langote_amit_f8@lab.ntt.co.jp> — 2018-07-12T08:44:48Z

    Nagata-san,

On 2018/07/12 16:59, Yugo Nagata wrote:
> Hi,
> 
> During looking into other thread[1], I found a problem on pg_dump of range
> partition table using expressions. When we create a range partitioned table, 
> we cannot use a column more than　once in the partition key.
>  
>  postgres=# create table t (i int) partition by range(i,i);
>  ERROR:  column "i" appears more than once in partition key
> 
> On the other hand, the following query using expression is allowed.
> 
>  postgres=# create table test (i int) partition by range(i,(i));
>  CREATE TABLE
> 
> However, when we use pg_dump for this, we get
> 
>  CREATE TABLE public.test (
>      i integer
>  )
>  PARTITION BY RANGE (i, i);
> 
> , and we cannot restore this due to the error.

Oops.

> I can consider three approaches to resolve this.
> 
> 
> 1) Allow to appear more than once in range partition key
> 
> I don't understand why there is this restriction. If we have no clear reason, 
> can we rip out this restrition?

I can't recall exactly, but back when I wrote this code, I might have been
thinking that such a feature is useless and would actually just end up
being a foot gun for users.

Although, I'm open to ripping it out if it's seen as being overly restrictive.

> 2) Forbid this kind of partition key
> 
> If we can make more checks and forbid such partition keys as RANGE(i,(i)), 
> we can avoid the problem though it is a bit rigorous.

If there is a way for transformPartitionSpec to conclude that i and (i)
are in fact the same thing and error out, which is perhaps not that hard
to do, then maybe we could do that.  That is, if we don't want to rip the
error out as your proposed solution (1) above.

> 3) Treat expressions as it is
> 
> For some reasons, expressions like "(column)" or "(column COLLATE something)"
> is treated like simple attributes in the current implementation
> (in ComputePartitionAttr() ).
> 
> If we treat these expressions as it is, pg_dump result will be the same 
> expression as when the partition table is created, and we can restore this 
> successfully.

Hmm, I think it's better to simplify (column) into a simple column
reference instead of uselessly maintaining it as an expression.  There's
no other good reason to do this.

Thanks,
Amit

  3. Re: Problem on pg_dump RANGE partition with expressions

    Yugo Nagata <nagata@sraoss.co.jp> — 2018-07-13T09:45:02Z

    On Thu, 12 Jul 2018 17:44:48 +0900
Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:

> > 1) Allow to appear more than once in range partition key
> > 
> > I don't understand why there is this restriction. If we have no clear reason, 
> > can we rip out this restrition?
> 
> I can't recall exactly, but back when I wrote this code, I might have been
> thinking that such a feature is useless and would actually just end up
> being a foot gun for users.
> 
> Although, I'm open to ripping it out if it's seen as being overly restrictive.

I think this way is good to resolve this, because allowing columns to appear more
than once would be harmless at least with the current partitioning methods, 
range and hash, although this is useless. 

Actually, we currenly allow same expression apears more than once in partition key
like

 create table t (i int) partition by range((i+1),(i+1));

In the past, I proposed a patch to forbid this, but this is rejected
since there is harmless and no need to restrict this.

Attached is a patch to get rid of "appears more than once" restriction.

Regards,

-- 
Yugo Nagata <nagata@sraoss.co.jp>

  4. Re: Problem on pg_dump RANGE partition with expressions

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-07-19T19:42:49Z

    Yugo Nagata <nagata@sraoss.co.jp> writes:
> Attached is a patch to get rid of "appears more than once" restriction.

Pushed.  (Again, it'd have been helpful if you updated the regression
tests.)

			regards, tom lane