Re: Adding support for Default partition in partitioning

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: Jeevan Ladhe <jeevan(dot)ladhe(at)enterprisedb(dot)com>, Beena Emerson <memissemerson(at)gmail(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, Rahila Syed <rahilasyed90(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Adding support for Default partition in partitioning
Date: 2017-06-08 16:56:10
Message-ID: CA+TgmobkFaptsmQiP94sbAKTtDKS6Azz+P4Bw1FxzmNrnyVa0w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jun 7, 2017 at 5:47 AM, Ashutosh Bapat
<ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
> On Sat, Jun 3, 2017 at 2:11 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>
>> + errmsg("default partition contains row(s)
>> that would overlap with partition being created")));
>>
>> It doesn't really sound right to talk about rows overlapping with a
>> partition. Partitions can overlap with each other, but not rows.
>> Also, it's not really project style to use ambiguously plural forms
>> like "row(s)" in error messages. Maybe something like:
>>
>> new partition constraint for default partition \"%s\" would be
>> violated by some row
>
> Partition constraint is implementation detail here. We enforce
> partition bounds through constraints and we call such constraints as
> partition constraints. But a user may not necessarily understand this
> term or may interpret it different. Adding "new" adds to the confusion
> as the default partition is not new.

I see your point. We could say "updated partition constraint" instead
of "new partition constraint" to address that to some degree.

> My suggestion in an earlier mail
> was ""default partition contains rows that conflict with the partition
> bounds of "part_xyz"", with a note that we should use a better word
> than "conflict". So, Jeevan seems to have used overlap, which again is
> not correct. How about "default partition contains row/s which would
> fit the partition "part_xyz" being created or attached." with a hint
> to move those rows to the new partition's table in case of attach. I
> don't think hint would be so straight forward i.e. to create the table
> with SELECT INTO and then ATTACH.

The problem is that none of these actually sound very good. Neither
conflict nor overlap nor fit actually express the underlying idea very
clearly, at least IMHO. I'm not opposed to using some wording along
these lines if we can think of a clear way to word it, but I think my
wording is better than using some unclear word for this concept. I
can't immediately think of a way to adjust your wording so that it
seems completely clear.

> Also, the error code ERRCODE_CHECK_VIOLATION, which is an "integrity
> constraint violation" code, seems misleading. We aren't violating any
> integrity here. In fact I am not able to understand, how could adding
> an object violate integrity constraint. The nearest errorcode seems to
> be ERRCODE_INVALID_OBJECT_DEFINITION, which is also used for
> partitions with overlapping bounds.

I think that calling a constraint failure a check violation is not too
much of a stretch, even if it's technically a partition constraint
rather than a CHECK constraint. However, your proposal also seems
reasonable. I'm happy to go with whatever most people like best.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2017-06-08 17:00:26 Re: [PATCH] Fixed malformed error message on malformed SCRAM message.
Previous Message Robert Haas 2017-06-08 16:49:37 Re: Adding support for Default partition in partitioning