Re: [HACKERS] Same expression more than once in partition key

[Robert Haas]

On Fri, Jun 23, 2017 at 4:04 PM, Tom Lane  wrote:
> Peter Eisentraut  writes:
>> We also allow the same column more than once in an index.  We probably
>> don't have to be more strict here.
>
> There actually are valid uses for the same column more than once in
> an index, eg if you use a different operator class for each instance.
> I find it hard to envision a similar use-case in partitioning though.

Maybe you already realize this, but partitioning, like index creation,
allows an opclass to be specified:

rhaas=# create table foo (a text, b text) partition by range (a
text_ops, b text_pattern_ops);
CREATE TABLE

I don't really see anybody wanting to do that, but I don't really see
anyone wanting to do with an index either.

My inclination is to reject this patch as not solving any actual problem.

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

Re: [HACKERS] Same expression more than once in partition key

[Amit Langote]

On 2017/06/24 5:04, Tom Lane wrote:
> Peter Eisentraut  writes:
>> We also allow the same column more than once in an index.  We probably
>> don't have to be more strict here.
> 
> There actually are valid uses for the same column more than once in
> an index, eg if you use a different operator class for each instance.
> I find it hard to envision a similar use-case in partitioning though.

So, does this mean we don't need to apply Nagata-san's patch for now?

As far as the partitioning internals are concerned, I see no harm in
allowing a column to appear more than once in the partition key.  OTOH, I
too don't see a partitioning use-case which would require it.

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

Re: [HACKERS] Same expression more than once in partition key

[Tom Lane]

Peter Eisentraut  writes:
> We also allow the same column more than once in an index.  We probably
> don't have to be more strict here.

There actually are valid uses for the same column more than once in
an index, eg if you use a different operator class for each instance.
I find it hard to envision a similar use-case in partitioning though.

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

Re: [HACKERS] Same expression more than once in partition key

[Peter Eisentraut]

On 6/23/17 09:54, Tom Lane wrote:
>> However, I can use same expression more than once in partition key.
> 
> ... and so, I can't get excited about extending it to expressions.
> Where would you stop --- for example, are i and (i) equal?  What
> about (i) and (case when true then i else j end)?  In principle,
> the system could recognize both those identities, but it seems
> silly to expend code on it just for nagware purposes.

We also allow the same column more than once in an index.  We probably
don't have to be more strict here.

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

Re: [HACKERS] Same expression more than once in partition key

[Yugo Nagata]

On Fri, 23 Jun 2017 09:54:17 -0400
Tom Lane  wrote:

> Yugo Nagata  writes:
> > 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
> 
> AFAIK, that's just a simple test for obvious mistakes; it's not a
> property that's essential for correctness.

Thanks. This is what I want to know.

> 
> > However, I can use same expression more than once in partition key.
> 
> ... and so, I can't get excited about extending it to expressions.
> Where would you stop --- for example, are i and (i) equal?  What
> about (i) and (case when true then i else j end)?  In principle,
> the system could recognize both those identities, but it seems
> silly to expend code on it just for nagware purposes.
> 

Agreed.


>   regards, tom lane


-- 
Yugo Nagata 


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

Re: [HACKERS] Same expression more than once in partition key

[Tom Lane]

Yugo Nagata  writes:
> 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

AFAIK, that's just a simple test for obvious mistakes; it's not a
property that's essential for correctness.

> However, I can use same expression more than once in partition key.

... and so, I can't get excited about extending it to expressions.
Where would you stop --- for example, are i and (i) equal?  What
about (i) and (case when true then i else j end)?  In principle,
the system could recognize both those identities, but it seems
silly to expend code on it just for nagware purposes.

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

Re: [HACKERS] Same expression more than once in partition key

[Yugo Nagata]

On Fri, 23 Jun 2017 15:57:54 +0900
Yugo Nagata  wrote:

> Hi,
> 
> 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
> 
> However, I can use same expression more than once in partition key.
> 
> postgres=# create table t (i int) partition by range((i),(i));
> CREATE TABLE
> 
> Although this can be blocked by the attached parch, for example,
> the following is still possible.

I forgot to attach it.

> 
> postgres=# create table t (i int) partition by range((i),i);
> CREATE TABLE
> 
> Can these definition be a problem in the internal of partitioning?
> If not, we might not need to check column that apears more than once 
> in the partition key.
> 
> Regards,
> 
> 
> 
> -- 
> Yugo Nagata 


-- 
Yugo Nagata 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7d9c769..dc4540b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13171,6 +13171,19 @@ transformPartitionSpec(Relation rel, PartitionSpec *partspec, char *strategy)
 		PartitionElem *pelem = castNode(PartitionElem, lfirst(l));
 		ListCell   *lc;
 
+		if (pelem->expr)
+		{
+			/* Copy, to avoid scribbling on the input */
+			pelem = copyObject(pelem);
+
+			/* Now do parse transformation of the expression */
+			pelem->expr = transformExpr(pstate, pelem->expr,
+		EXPR_KIND_PARTITION_EXPRESSION);
+
+			/* we have to fix its collations too */
+			assign_expr_collations(pstate, pelem->expr);
+		}
+
 		/* Check for PARTITION BY ... (foo, foo) */
 		foreach(lc, newspec->partParams)
 		{
@@ -13183,19 +13196,11 @@ transformPartitionSpec(Relation rel, PartitionSpec *partspec, char *strategy)
 		 errmsg("column \"%s\" appears more than once in partition key",
 pelem->name),
 		 parser_errposition(pstate, pelem->location)));
-		}
-
-		if (pelem->expr)
-		{
-			/* Copy, to avoid scribbling on the input */
-			pelem = copyObject(pelem);
-
-			/* Now do parse transformation of the expression */
-			pelem->expr = transformExpr(pstate, pelem->expr,
-		EXPR_KIND_PARTITION_EXPRESSION);
-
-			/* we have to fix its collations too */
-			assign_expr_collations(pstate, pelem->expr);
+			else if (pelem->expr && pparam->expr && equal(pelem->expr, pparam->expr))
+ereport(ERROR,
+		(errcode(ERRCODE_DUPLICATE_COLUMN),
+		 errmsg("same expression appears more than once in partition key"),
+		 parser_errposition(pstate, pelem->location)));
 		}
 
 		newspec->partParams = lappend(newspec->partParams, pelem);

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