Re: BUG #14666: Question on money type as the key of partitioned table

Thanks a lot for getting to fixing this bug before I did.

On 2017/05/29 6:53, Tom Lane wrote:
> tianbing(at)highgo(dot)com writes:
>> When I use the money type as the key to create the partition table as
>> follows:
>
>> postgres=# create table test(m money) partition by list(m);
>> CREATE TABLE
>> postgres=# create table test_1 partition of test for values in (10);
>> CREATE TABLE
>
>> Partition bounds without apostrophe can be createed, but it store the null
>> value, not '10' value.
>
> That's not actually what it's doing. A look into pg_class shows that
> while, for an integer partitioning column, you'd get something like this
> for relpartbound:
>
> test1p | {PARTITIONBOUND :strategy l :listdatums (
> {CONST :consttype 23 :consttypmod -1 :constcollid 0 :constlen 4 :constbyval true
> :constisnull false :location 54 :constvalue 4 [ 10 0 0 0 0 0 0 0 ]}) :lowerdatu
> ms <> :upperdatums <>}
>
> in the case at hand you'd get
>
> test2p | {PARTITIONBOUND :strategy l :listdatums (
> {FUNCEXPR :funcid 3811 :funcresulttype 790 :funcretset false :funcvariadic false
> :funcformat 2 :funccollid 0 :inputcollid 0 :args ({CONST :consttype 23 :constty
> pmod -1 :constcollid 0 :constlen 4 :constbyval true :constisnull false :location
> 54 :constvalue 4 [ 10 0 0 0 0 0 0 0 ]}) :location -1}) :lowerdatums <> :upperda
> tums <>}
>
> that is, what we have is a run-time coercion of integer to money.
> The partitioning code utterly fails to consider that what it might
> get from the partition list syntax is not a constant --- but since
> casts are not required to be immutable, it might not.

Oops, that's right. I had failed to consider that the cast step might
introduce a coercion function call.

> This is exacerbated by the fact that subsequent code naively assumes
> that the elements of PartitionBoundSpec.listdatums are Consts, without
> any checking. It's a wonder you don't get runtime crashes. (You might
> if the partition column type is pass-by-ref, I suspect.) And I'm
> unimpressed by the fact that this assumption is nowhere documented, too.
>
> What we need to do here (at least in the short term) is throw an error
> if we don't get a simple Const out of const-simplification. I'm not
> sure if we need a separate error message for that case, or if we can
> get away with just re-using the existing text about "specified value
> cannot be cast to type ...". The point here would be that the cast
> exists but is not immutable. Maybe use the same primary message
> but explain that in an errdetail?

The message as committed in 76a3df6e5e621928fbf0cddf347e16a62e9433ec looks
on point to me.

Thanks again.

Regards,
Amit