Re: Boolean partitions syntax - Mailing list pgsql-hackers

From Amit Langote
Subject Re: Boolean partitions syntax
Date
Msg-id 031be158-817a-d3d9-b97a-8ba998b608e2@lab.ntt.co.jp
Whole thread Raw
In response to Re: Boolean partitions syntax  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Boolean partitions syntax  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
List pgsql-hackers
On 2018/01/29 14:57, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.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



pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: [HACKERS] make async slave to wait for lsn to be replayed
Next
From: Michael Banck
Date:
Subject: Re: [PoC PATCH] Parallel dump to /dev/null