On Tue, Dec 21, 2021 at 2:47 PM Ashutosh Sharma <ashu.coe...@gmail.com> wrote:
> On Mon, Dec 20, 2021 at 7:04 PM Amit Langote <amitlangot...@gmail.com> wrote:
>> On Mon, Dec 13, 2021 at 11:37 PM Ashutosh Sharma <ashu.coe...@gmail.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


Reply via email to