Обсуждение: Some leftovers of recent message cleanup?
Hello.
While I was examining message translation for PG14, I found some
messages that would need to be fixed.
0001 is a fix for perhaps-leftovers of the recent message cleanups
related to "positive integer"(fd90f6ba7a).
0002 is a fix for a maybe-mistake in message convention of a recent
fix in ECPG of linked-connection (about trailing period and
lower-cased commad names)
0003 is a fix for remaining "positive" use, which are in doubt in
worthiness to fix..
Please find the attached.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From 3bf651b5224915045c824eee48a5c3441432ccca Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Thu, 19 Aug 2021 16:53:24 +0900
Subject: [PATCH 1/3] Additional message fix related to "positive" wordings
While fd90f6ba7a intended to eliminate the usage of "positive integer"
and alike, it forgot to fix the same messages other places. Complete
the work.
---
src/backend/parser/parse_utilcmd.c | 2 +-
src/backend/utils/adt/tsquery.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 675e400839..3b656f2f13 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -4035,7 +4035,7 @@ transformPartitionBound(ParseState *pstate, Relation parent,
if (spec->modulus <= 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("modulus for hash partition must be a positive integer")));
+ errmsg("modulus for hash partition must be an integer greater than zero")));
Assert(spec->remainder >= 0);
diff --git a/src/backend/utils/adt/tsquery.c b/src/backend/utils/adt/tsquery.c
index b2ca0d2f8a..ded919b39b 100644
--- a/src/backend/utils/adt/tsquery.c
+++ b/src/backend/utils/adt/tsquery.c
@@ -196,7 +196,7 @@ parse_phrase_operator(TSQueryParserState pstate, int16 *distance)
else if (errno == ERANGE || l < 0 || l > MAXENTRYPOS)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("distance in phrase operator should not be greater than %d",
+ errmsg("distance in phrase operator must be an integer value between zero and %d
inclusive",
MAXENTRYPOS)));
else
{
--
2.27.0
From 75a03d39c83b080aca505a343e3a8381ef7703c4 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Thu, 19 Aug 2021 16:53:54 +0900
Subject: [PATCH 2/3] Message cleanup.
---
src/interfaces/ecpg/preproc/ecpg.header | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/interfaces/ecpg/preproc/ecpg.header b/src/interfaces/ecpg/preproc/ecpg.header
index efb2fb3a38..df58f1535c 100644
--- a/src/interfaces/ecpg/preproc/ecpg.header
+++ b/src/interfaces/ecpg/preproc/ecpg.header
@@ -596,7 +596,7 @@ check_declared_list(const char *name)
{
if (connection)
if (connection && strcmp(ptr->connection, connection) != 0)
- mmerror(PARSE_ERROR, ET_WARNING, "connection %s is overwritten with %s by declare statement %s.",
connection,ptr->connection, name);
+ mmerror(PARSE_ERROR, ET_WARNING, "connection %s is overwritten with %s by DECLARE statement %s",
connection,ptr->connection, name);
connection = mm_strdup(ptr -> connection);
return true;
}
--
2.27.0
From 8f4f993de671f0af594614b1a3e3218ee3d5fe70 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Thu, 19 Aug 2021 16:21:02 +0900
Subject: [PATCH 3/3] Exterminate usages of "positive"
Additional fix for "positive number"s.
---
doc/src/sgml/ref/create_function.sgml | 4 ++--
src/backend/commands/functioncmds.c | 4 ++--
src/backend/tsearch/wparser_def.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/ref/create_function.sgml b/doc/src/sgml/ref/create_function.sgml
index 7e6d52c7dc..1e10ac14c4 100644
--- a/doc/src/sgml/ref/create_function.sgml
+++ b/doc/src/sgml/ref/create_function.sgml
@@ -464,7 +464,7 @@ CREATE [ OR REPLACE ] FUNCTION
<listitem>
<para>
- A positive number giving the estimated execution cost for the function,
+ A number greater than zero giving the estimated execution cost for the function,
in units of <xref linkend="guc-cpu-operator-cost"/>. If the function
returns a set, this is the cost per returned row. If the cost is
not specified, 1 unit is assumed for C-language and internal functions,
@@ -480,7 +480,7 @@ CREATE [ OR REPLACE ] FUNCTION
<listitem>
<para>
- A positive number giving the estimated number of rows that the planner
+ A number greater than zero giving the estimated number of rows that the planner
should expect the function to return. This is only allowed when the
function is declared to return a set. The default assumption is
1000 rows.
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 79d875ab10..80f9f1a22b 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -829,7 +829,7 @@ compute_function_attributes(ParseState *pstate,
if (*procost <= 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("COST must be positive")));
+ errmsg("COST must be greater than zero")));
}
if (rows_item)
{
@@ -837,7 +837,7 @@ compute_function_attributes(ParseState *pstate,
if (*prorows <= 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("ROWS must be positive")));
+ errmsg("ROWS must be greater than zero")));
}
if (support_item)
*prosupport = interpret_func_support(support_item);
diff --git a/src/backend/tsearch/wparser_def.c b/src/backend/tsearch/wparser_def.c
index 559dff6355..36b24d3aea 100644
--- a/src/backend/tsearch/wparser_def.c
+++ b/src/backend/tsearch/wparser_def.c
@@ -2598,7 +2598,7 @@ prsd_headline(PG_FUNCTION_ARGS)
if (min_words <= 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("MinWords should be positive")));
+ errmsg("MinWords should be greater than zero")));
if (shortword < 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
--
2.27.0
On 2021/08/19 17:03, Kyotaro Horiguchi wrote: > Hello. > > While I was examining message translation for PG14, I found some > messages that would need to be fixed. > > 0001 is a fix for perhaps-leftovers of the recent message cleanups > related to "positive integer"(fd90f6ba7a). There are still other many messages using "positive" and "negative" keywords. We should also fix them at all? BTW, we discussed this before at [1] and concluded that at first we should focus on the fix of the ambiguous "non-negative" and let "positive" and "negative" keywords as they are. [1] https://www.postgresql.org/message-id/CALj2ACV7KDM8R=SrDbxvxT5yMDi+BMWVVV_UwmmHiii1pumr8Q@mail.gmail.com > 0002 is a fix for a maybe-mistake in message convention of a recent > fix in ECPG of linked-connection (about trailing period and > lower-cased commad names) LGTM. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
At Thu, 19 Aug 2021 20:29:42 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in > On 2021/08/19 17:03, Kyotaro Horiguchi wrote: > > Hello. > > While I was examining message translation for PG14, I found some > > messages that would need to be fixed. > > 0001 is a fix for perhaps-leftovers of the recent message cleanups > > related to "positive integer"(fd90f6ba7a). > > There are still other many messages using "positive" and "negative" > keywords. > We should also fix them at all? I'm not sure, or no if anything. My main point here is not to avoid use of such kind of words, but reducing variations of the effectively the same message from the view of translator burden. The two messages in 0001 are in that category. I noticed those messages accidentally. I don't think they are the only instance of such divergence, but I'm not going to do a comprehensive examination of such divergences.. (Or do I need to check that more comprehensively?) > BTW, we discussed this before at [1] and concluded that at first > we should focus on the fix of the ambiguous "non-negative" and > let "positive" and "negative" keywords as they are. > > [1] > https://www.postgresql.org/message-id/CALj2ACV7KDM8R=SrDbxvxT5yMDi+BMWVVV_UwmmHiii1pumr8Q@mail.gmail.com Understood. I agree that point and 0003 is worthless as I anticipated. I misunderstood that "positive" were included in the do-not-use list. > > 0002 is a fix for a maybe-mistake in message convention of a recent > > fix in ECPG of linked-connection (about trailing period and > > lower-cased commad names) > > LGTM. Thanks. So I excluded 0003 and added a fix for regression tests affected by 0001. (I'm a bit surprised that 0002 doesn't break regression tests.) regards. -- Kyotaro Horiguchi NTT Open Source Software Center From a1336831633e1ca8cae17d36829fab8272c9a7fe Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Thu, 19 Aug 2021 16:53:24 +0900 Subject: [PATCH v2 1/2] Merge diverged messages fd90f6ba7a forgot to fix other identical messages to the messages it fixed. Fix them to reduce translator burden. --- src/backend/parser/parse_utilcmd.c | 2 +- src/backend/utils/adt/tsquery.c | 2 +- src/test/regress/expected/alter_table.out | 2 +- src/test/regress/expected/create_table.out | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 675e400839..3b656f2f13 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -4035,7 +4035,7 @@ transformPartitionBound(ParseState *pstate, Relation parent, if (spec->modulus <= 0) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), - errmsg("modulus for hash partition must be a positive integer"))); + errmsg("modulus for hash partition must be an integer greater than zero"))); Assert(spec->remainder >= 0); diff --git a/src/backend/utils/adt/tsquery.c b/src/backend/utils/adt/tsquery.c index b2ca0d2f8a..ded919b39b 100644 --- a/src/backend/utils/adt/tsquery.c +++ b/src/backend/utils/adt/tsquery.c @@ -196,7 +196,7 @@ parse_phrase_operator(TSQueryParserState pstate, int16 *distance) else if (errno == ERANGE || l < 0 || l > MAXENTRYPOS) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("distance in phrase operator should not be greater than %d", + errmsg("distance in phrase operator must be an integer value between zero and %d inclusive", MAXENTRYPOS))); else { diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 8dcb00ac67..22091119c8 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -4107,7 +4107,7 @@ ALTER TABLE hash_parted ATTACH PARTITION hpart_5 FOR VALUES WITH (MODULUS 4, REM -- check that the table being attach is with valid modulus and remainder value CREATE TABLE fail_part(LIKE hash_parted); ALTER TABLE hash_parted ATTACH PARTITION fail_part FOR VALUES WITH (MODULUS 0, REMAINDER 1); -ERROR: modulus for hash partition must be a positive integer +ERROR: modulus for hash partition must be an integer greater than zero ALTER TABLE hash_parted ATTACH PARTITION fail_part FOR VALUES WITH (MODULUS 8, REMAINDER 8); ERROR: remainder for hash partition must be less than modulus ALTER TABLE hash_parted ATTACH PARTITION fail_part FOR VALUES WITH (MODULUS 3, REMAINDER 2); diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 96bf426d98..f4e559a1a3 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -934,7 +934,7 @@ LINE 1: ...LE fail_part PARTITION OF hash_parted2 FOR VALUES WITH (MODU... ^ -- modulus must be greater than zero CREATE TABLE fail_part PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 0, REMAINDER 1); -ERROR: modulus for hash partition must be a positive integer +ERROR: modulus for hash partition must be an integer greater than zero -- remainder must be greater than or equal to zero and less than modulus CREATE TABLE fail_part PARTITION OF hash_parted2 FOR VALUES WITH (MODULUS 8, REMAINDER 8); ERROR: remainder for hash partition must be less than modulus -- 2.27.0 From f5097bd47cb42f9eed34ade79f7434366fbdd0ee Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Thu, 19 Aug 2021 16:53:54 +0900 Subject: [PATCH v2 2/2] Message cleanup. --- src/interfaces/ecpg/preproc/ecpg.header | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/interfaces/ecpg/preproc/ecpg.header b/src/interfaces/ecpg/preproc/ecpg.header index efb2fb3a38..df58f1535c 100644 --- a/src/interfaces/ecpg/preproc/ecpg.header +++ b/src/interfaces/ecpg/preproc/ecpg.header @@ -596,7 +596,7 @@ check_declared_list(const char *name) { if (connection) if (connection && strcmp(ptr->connection, connection) != 0) - mmerror(PARSE_ERROR, ET_WARNING, "connection %s is overwritten with %s by declare statement %s.", connection,ptr->connection, name); + mmerror(PARSE_ERROR, ET_WARNING, "connection %s is overwritten with %s by DECLARE statement %s", connection,ptr->connection, name); connection = mm_strdup(ptr -> connection); return true; } -- 2.27.0
On 2021/08/20 11:53, Kyotaro Horiguchi wrote:
> At Thu, 19 Aug 2021 20:29:42 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
>> On 2021/08/19 17:03, Kyotaro Horiguchi wrote:
>>> Hello.
>>> While I was examining message translation for PG14, I found some
>>> messages that would need to be fixed.
>>> 0001 is a fix for perhaps-leftovers of the recent message cleanups
>>> related to "positive integer"(fd90f6ba7a).
>>
>> There are still other many messages using "positive" and "negative"
>> keywords.
>> We should also fix them at all?
>
> I'm not sure, or no if anything. My main point here is not to avoid
> use of such kind of words, but reducing variations of the effectively
> the same message from the view of translator burden. The two messages
> in 0001 are in that category. I noticed those messages accidentally. I
> don't think they are the only instance of such divergence, but I'm not
> going to do a comprehensive examination of such divergences.. (Or do I
> need to check that more comprehensively?)
Understood.
- errmsg("modulus for hash partition must be a positive integer")));
+ errmsg("modulus for hash partition must be an integer greater than zero")));
"an integer greater" should be "an integer value greater" because
we use that words in other similar log messages like "modulus for
hash partition must be an integer value greater than zero"
in src/backend/partitioning/partbounds.c?
I'm thinking to back-patch this to v11 where hash partitioning
was supported. On the other hand, the following change should
be back-patched to all supported versions.
- errmsg("distance in phrase operator should not be greater than %d",
+ errmsg("distance in phrase operator must be an integer value between zero and %d
inclusive",
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
At Fri, 20 Aug 2021 19:36:02 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
>
>
> On 2021/08/20 11:53, Kyotaro Horiguchi wrote:
> > At Thu, 19 Aug 2021 20:29:42 +0900, Fujii Masao
> > <masao.fujii@oss.nttdata.com> wrote in
> >> On 2021/08/19 17:03, Kyotaro Horiguchi wrote:
> >>> Hello.
> >>> While I was examining message translation for PG14, I found some
> >>> messages that would need to be fixed.
> >>> 0001 is a fix for perhaps-leftovers of the recent message cleanups
> >>> related to "positive integer"(fd90f6ba7a).
> >>
> >> There are still other many messages using "positive" and "negative"
> >> keywords.
> >> We should also fix them at all?
> > I'm not sure, or no if anything. My main point here is not to avoid
> > use of such kind of words, but reducing variations of the effectively
> > the same message from the view of translator burden. The two messages
> > in 0001 are in that category. I noticed those messages accidentally. I
> > don't think they are the only instance of such divergence, but I'm not
> > going to do a comprehensive examination of such divergences.. (Or do I
> > need to check that more comprehensively?)
>
>
> Understood.
>
> - errmsg("modulus for hash partition must be a positive
> - integer")));
> + errmsg("modulus for hash partition must be an integer greater than
> zero")));
>
> "an integer greater" should be "an integer value greater" because
> we use that words in other similar log messages like "modulus for
> hash partition must be an integer value greater than zero"
> in src/backend/partitioning/partbounds.c?
Ugh... Of course. I thought I did that way but the actual file is
incorrect... Fixed that in the attached.
> I'm thinking to back-patch this to v11 where hash partitioning
> was supported. On the other hand, the following change should
If I'm not missing something, back to 13, where the "word change" took
place? For 11 and 12, we need to change the *both* messages.
v11, 12
./src/backend/parser/parse_utilcmd.c
On 2021/08/23 15:27, Kyotaro Horiguchi wrote: > So this patch is meaningful only for v13 and later, too. Not sure what > to do for v12 and earlier but doesn't seem to be in much necessity. Probably I failed to get your point... I was thinking that your 0001 patch can be back-patched to v11. And also the attached patch can be applied to v10 and v9.6. If we do this, the log messages can be consistent between versions. But you think that we should not change the log messages in v12 or before, for some reasons? I pushed your 0002 patch. Thanks! Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Вложения
At Wed, 25 Aug 2021 10:05:25 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in > > > On 2021/08/23 15:27, Kyotaro Horiguchi wrote: > > So this patch is meaningful only for v13 and later, too. Not sure what > > to do for v12 and earlier but doesn't seem to be in much necessity. > > Probably I failed to get your point... I was thinking that > your 0001 patch can be back-patched to v11. And also > the attached patch can be applied to v10 and v9.6. If we do this, > the log messages can be consistent between versions. > But you think that we should not change the log messages > in v12 or before, for some reasons? Sigh. The message of partbounds.c of v11,12 cited there was just wrong. So, you're right that it is simply applicable to v11 and later. Sorry for the confusion. > I pushed your 0002 patch. Thanks! Thanks for committing! regares. -- Kyotaro Horiguchi NTT Open Source Software Center
On 2021/08/25 10:50, Kyotaro Horiguchi wrote: > Sigh. The message of partbounds.c of v11,12 cited there was just > wrong. So, you're right that it is simply applicable to v11 and > later. Sorry for the confusion. I separated the patch into two, to make the commit message of each change clear. And I pushed them. Thanks! Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION