Re: using expression syntax for partition bounds

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp
Cc: pgsql-hackers(at)postgresql(dot)org, tgl(at)sss(dot)pgh(dot)pa(dot)us
Subject: Re: using expression syntax for partition bounds
Date: 2018-04-24 09:08:38
Message-ID: 20180424.180838.65007339.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks. I have almost missed this.

At Mon, 23 Apr 2018 11:44:14 +0900, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote in <e8c770eb-a850-6645-8310-f3eaea3a72fd(at)lab(dot)ntt(dot)co(dot)jp>
> On 2018/04/23 11:37, Amit Langote wrote:
> > I tried to update the patch to do things that way. I'm going to create a
> > new entry in the next CF titled "generalized expression syntax for
> > partition bounds" and add the patch there.
>
> Tweaked the commit message to credit all the authors.

+ any variable-free expression (subqueries, window functions, aggregate
+ functions, and set-returning functions are not allowed). The data type
+ of the partition bound expression must match the data type of the
+ corresponding partition key column.

Parititioning value is slimiar to column default expression in
the sense that it appeas within a DDL. The description for
DEFAULT is:

| The value is any variable-free expression (subqueries and
| cross-references to other columns in the current table are not
| allowed)

It actually refuses aggregates, window functions and SRFs but it
is not mentioned. Whichever we choose, they ought to have the
similar description.

> /*
> * Strip any top-level COLLATE clause, as they're inconsequential.
> * XXX - Should we add a WARNING here?
> */
> while (IsA(value, CollateExpr))
> value = (Node *) ((CollateExpr *) value)->arg;

Ok, I'll follow Tom's suggestion but collate is necessarilly
appears in this shape. For example ('a' collate "de_DE") || 'b')
has the collate node under the top ExprOp and we get a complaint
like following with it.

> ERROR: unrecognized node type: 123

123 is T_CollateExpr. The expression "value" (mmm..) reuires
eval_const_expressions to eliminate CollateExprs. It requires
assign_expr_collations to retrieve value's collation but we don't
do that since we ignore collation this in this case.

The following results in a strange-looking error.

> =# create table pt1 partition of p for values in (a);
> ERROR: column "a" does not exist
> LINE 1: create table pt1 partition of p for values in (a);

The p/pt1 actually have a column a.

The following results in similar error and it is wrong, too.

> =# create table pr1 partition of pr for values from (a + 1) to (a + 2);
> ERROR: column "a" does not exist
> LINE 1: create table pr1 partition of pr for values from (a + 1) to ...

Being similar but a bit different, the following command leads to
a SEGV since it creates PARTITION_RANGE_DATUM_VALUE with value =
NULL. Even if it leaves the original node, validateInfiniteBounds
rejects it and aborts.

> =# create table pr1 partition of pr for values from (hoge) to (hige);
(crash)

I fixed this using pre-columnref hook in the attached v3. This
behavles for columnrefs differently than DEFAULT.

The v3-2 does the same thing with DEFAULT. The behevior differs
whether the column exists or not.

> ERROR: cannot use column referencees in bound value
> ERROR: column "b" does not exist

I personally think that such similarity between DEFALUT and
partition bound (v3-2) is not required. But inserting the hook
(v3) doesn't look good for me.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
v3-0001-Allow-generalized-expression-syntax-for-partition.patch text/x-patch 19.9 KB
v3-2-0001-Allow-generalized-expression-syntax-for-partition.patch text/x-patch 21.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Konstantin Knizhnik 2018-04-24 09:09:28 Re: Built-in connection pooling
Previous Message Aleksander Alekseev 2018-04-24 09:07:35 Re: community bonding