Re: Boolean partitions syntax

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Mark Dilger <hornschnorter(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Boolean partitions syntax
Date: 2018-02-02 09:39:34
Message-ID: 031be158-817a-d3d9-b97a-8ba998b608e2@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2018/01/29 14:57, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> writes:
>> Partition bound literals as captured gram.y don't have any type
>> information attached.
>
> Isn't that design broken by definition? TRUE is not the same thing
> as 't', nor as 'true'. Nor are 1 and '1' the same thing; it's true
> that in some contexts we'll let '1' convert to an integer 1, but the
> reverse is not true.

Hmm, I thought the following does convert an integer 1 to text value '1',
but maybe I'm missing your point.

create table foo (a text default 1);

> Moreover, this approach doesn't have any hope
> of ever extending to bound values that aren't bare literals.
>
> I think you are fixing this at the wrong level. Ideally the bound values
> ought to be expressions that get coerced to the partition column type.
> It's fine to require them to be constants for now, but not to invent
> an off-the-cuff set of syntactic restrictions that substitute for the
> semantic notion of "must be a constant".

I do remember the partitioning patch used to use a_expr for what today is:

partbound_datum:
Sconst { $$ = makeStringConst($1, @1); }
| NumericOnly { $$ = makeAConst($1, @1); }
| NULL_P { $$ = makeNullAConst(@1); }
;

but thought at the time that that allowed way too much stuff into the
partition bound syntax.

> That path will lead to nasty
> backwards compatibility issues whenever somebody tries to extend the
> feature.
>
> A concrete example of that is that the code currently accepts:
>
> regression=# create table textpart (a text) partition by list (a);
> CREATE TABLE
> regression=# create table textpart_t partition of textpart for values in (1);
> CREATE TABLE
>
> Since there's no implicit conversion from int to text, this seems
> pretty broken to me: there's no way for this behavior to be upward
> compatible to an implementation that treats the partition bound
> values as anything but text strings. We should fix that before the
> behavior gets completely set in concrete.

Most of the code does treat partition bound values as Node values doing
coercing before calling the input value good and failing upon not being
able to convert to the desired type for whatever reason.

create table b (a bool) partition by list (a);
create table bt partition of b for values in (1);
ERROR: specified value cannot be cast to type boolean for column "a"
LINE 1: create table bt partition of b for values in (1);

Can you say a bit more about the compatibility issues if we extend the syntax?

Thanks,
Amit

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Banck 2018-02-02 09:44:01 Re: [PoC PATCH] Parallel dump to /dev/null
Previous Message Stephen Frost 2018-02-02 09:02:04 Re: [HACKERS] make async slave to wait for lsn to be replayed