Re: Multi-Column List Partitioning

On Tue, Dec 21, 2021 at 2:47 PM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
> On Mon, Dec 20, 2021 at 7:04 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>> On Mon, Dec 13, 2021 at 11:37 PM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>> >
>> > Hi,
>> >
>> > Is this okay?
>> >
>> > postgres=# CREATE TABLE t1 (a int, b int) PARTITION BY LIST ( a, a, a );
>> > CREATE TABLE
>> >
>> > postgres=# CREATE TABLE t1_1 PARTITION OF t1 FOR VALUES IN ((1, 2, 3), (4, 5, 6));
>> > CREATE TABLE
>> >
>> > postgres=# \d t1
>> > Partitioned table "public.t1"
>> > Column | Type | Collation | Nullable | Default
>> > --------+---------+-----------+----------+---------
>> > a | integer | | |
>> > b | integer | | |
>> > Partition key: LIST (a, a, a)
>> > Number of partitions: 1 (Use \d+ to list them.)
>>
>> I'd say it's not okay for a user to expect this to work sensibly, and
>> I don't think it would be worthwhile to write code to point that out
>> to the user if that is what you were implying.
>
> OK. As you wish.

Actually, we *do* have some code in check_new_partition_bound() to
point it out if an empty range is specified for a partition, something
that one (or a DDL script) may accidentally do:

/*
* First check if the resulting range would be empty with
* specified lower and upper bounds...
*/
cmpval = partition_rbound_cmp(key->partnatts,
key->partsupfunc,
key->partcollation,
lower->datums, lower->kind,
true, upper);
Assert(cmpval != 0);
if (cmpval > 0)
{
/* Point to problematic key in the lower datums list. */
PartitionRangeDatum *datum = list_nth(spec->lowerdatums,
cmpval - 1);

ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("empty range bound specified for
partition \"%s\"",
relname),
errdetail("Specified lower bound %s is
greater than or equal to upper bound %s.",

get_range_partbound_string(spec->lowerdatums),

get_range_partbound_string(spec->upperdatums)),
parser_errposition(pstate, datum->location)));
}

So one may wonder why we don't catch and point out more such user
mistakes, like the one in your example. It may not be hard to
implement a proof that the partition bound definition a user entered
results in a self-contradictory partition constraint using the
facilities given in predtest.c. (The empty-range proof seemed simple
enough to implement as the above block of code.) I don't however see
why we should do that for partition constraints if we don't do the
same for CHECK constraints; for example, the following definition,
while allowed, is not very useful:

create table foo (a int check (a = 1 and a = 2));
\d foo
Table "public.foo"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
a | integer | | |
Check constraints:
"foo_a_check" CHECK (a = 1 AND a = 2)

Maybe partitioning should be looked at differently than the free-form
CHECK constraints, but I'm not so sure. Or if others insist that it
may be worthwhile to improve the user experience in such cases, we
could do that as a separate patch than the patch to implement
multi-column list partitioning.

--
Amit Langote
EDB: http://www.enterprisedb.com