Re: Revise some error messages in split partition code

From: Tender Wang <tndrwang(at)gmail(dot)com>
To: Richard Guo <guofenglinux(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Alexander Lakhin <exclusion(at)gmail(dot)com>
Subject: Re: Revise some error messages in split partition code
Date: 2024-04-10 13:32:32
Message-ID: CAHewXNnsfB8HBtz67-1k=cDwAKOPXrkXBQRk4MiNrV4eRkkssg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Richard Guo <guofenglinux(at)gmail(dot)com> 于2024年4月10日周三 19:32写道:

> I noticed some error messages in the split partition code that are not
> up to par. Such as:
>
> "new partitions not have value %s but split partition has"
>
> how about we revise it to:
>
> "new partitions do not have value %s but split partition does"
>
> Another one is:
>
> "any partition in the list should be DEFAULT because split partition is
> DEFAULT"
>
> how about we revise it to:
>
> "all partitions in the list should be DEFAULT because split partition is
> DEFAULT"
>
> Another problem I noticed is that in the test files partition_split.sql
> and partition_merge.sql, there are comments specifying the expected
> error messages for certain test queries. However, in some cases, the
> error message mentioned in the comment does not match the error message
> actually generated by the query. Such as:
>
> -- ERROR: invalid partitions order, partition "sales_mar2022" can not be
> merged
> -- (space between sections sales_jan2022 and sales_mar2022)
> ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_mar2022)
> INTO sales_jan_mar2022;
> ERROR: lower bound of partition "sales_mar2022" conflicts with upper
> bound of previous partition "sales_jan2022"
>
> I'm not sure if it's a good practice to specify the expected error
> message in the comment. But if we choose to do so, I think we at least
> need to ensure that the specified error message in the comment remains
> consistent with the error message produced by the query.
>
> Also there are some comments containing grammatical issues. Such as:
>
> -- no error: bounds of sales_noerror equals to lower and upper bounds of
> sales_dec2022 and sales_feb2022
>
> Attached is a patch to fix the issues I've observed. I suspect there
> may be more to be found.
>

Yeah. The day before yesterday I found some grammer errors from error
messages and code comments [1] .
Except those issues, @Alexander Lakhin <exclusion(at)gmail(dot)com> has found
some bugs [2]
I have some concerns that whether this patch is ready to commit.

[1]
https://www.postgresql.org/message-id/CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw%40mail.gmail.com
[2]
https://www.postgresql.org/message-id/dbc8b96c-3cf0-d1ee-860d-0e491da20485%40gmail.com

--
Tender Wang
OpenPie: https://en.openpie.com/

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Borisov 2024-04-10 13:42:51 Re: Table AM Interface Enhancements
Previous Message Robert Haas 2024-04-10 13:19:15 Re: Table AM Interface Enhancements