Обсуждение: misleading error message in DefineIndex

Поиск
Список
Период
Сортировка

misleading error message in DefineIndex

От
jian he
Дата:
hi.

while working on virtual generated column as partition key, I found below
errmsg the constraint type is hardcoded.

            if (!found)
            {
                Form_pg_attribute att;
                att = TupleDescAttr(RelationGetDescr(rel),
                                    key->partattrs[i] - 1);
                ereport(ERROR,
                        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                         errmsg("unique constraint on partitioned
table must include all partitioning columns"),
                         errdetail("%s constraint on table \"%s\"
lacks column \"%s\" which is part of the partition key.",
                                   constraint_type,
RelationGetRelationName(rel),
                                   NameStr(att->attname))));
            }

create table idxpart (a int4range, constraint xx exclude USING GIST (a
with = ), b int4range ) partition by range (b);
ERROR:  unique constraint on partitioned table must include all
partitioning columns
DETAIL:  EXCLUDE constraint on table "idxpart" lacks column "b" which
is part of the partition key.

In the example above , the primary error message "unique
constraint..." seems confusing.


--
jian
https://www.enterprisedb.com/



Re: misleading error message in DefineIndex

От
Daniel Gustafsson
Дата:
> On 17 Nov 2025, at 05:06, jian he <jian.universality@gmail.com> wrote:

> In the example above , the primary error message "unique
> constraint..." seems confusing.

Some of the internals does seem bleed through.  Do you want to work on a patch
for a suggestion on an improvement?  Maybe it could be possible to improve the
wording by incorporating constraint_type in some way?

--
Daniel Gustafsson




Re: misleading error message in DefineIndex

От
jian he
Дата:
On Mon, Nov 17, 2025 at 8:52 PM Daniel Gustafsson <daniel@yesql.se> wrote:
>
> Some of the internals does seem bleed through.  Do you want to work on a patch
> for a suggestion on an improvement?  Maybe it could be possible to improve the
> wording by incorporating constraint_type in some way?
>

I have changed this ereport:

ereport(ERROR,
        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
         errmsg("unique constraint on partitioned table must include
all partitioning columns"),
         errdetail("%s constraint on table \"%s\" lacks column \"%s\"
which is part of the partition key.",
                    constraint_type, RelationGetRelationName(rel),
                    NameStr(att->attname))));

to
+                        errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                        errmsg("%s constraint on partitioned table
must include all partitioning columns", constraint_type),
+                        errdetail("%s constraint on partitioned table
\"%s\" lacks column \"%s\" which is part of the partition key.",
+                                  constraint_type,
RelationGetRelationName(rel),
+                                  NameStr(att->attname)));

Вложения

Re: misleading error message in DefineIndex

От
Tom Lane
Дата:
jian he <jian.universality@gmail.com> writes:
> On Mon, Nov 17, 2025 at 8:52 PM Daniel Gustafsson <daniel@yesql.se> wrote:
>>
>> Some of the internals does seem bleed through.  Do you want to work on a patch
>> for a suggestion on an improvement?

> I have changed this ereport:
>          errmsg("unique constraint on partitioned table must include
> all partitioning columns"),
> to
> +                        errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> +                        errmsg("%s constraint on partitioned table
> must include all partitioning columns", constraint_type),

I pushed this patch with minor editorialization:

* I didn't agree with changing the errdetail message.  Yeah, adding
"partitioned" there could be argued to be an improvement, but it's
not essential given that the main message already specifies that we're
talking about a partitioned table.  So I judged that change not worth
the work it would impose on translators.

* I put in translator: comments so that translators don't need to
consult the source code to guess what the %s stands for.

* I put back the extra ereport parentheses, because removing them
mainly served to obscure what the patch was changing and what it
wasn't.

            regards, tom lane