Re: [HACKERS] Fixed redundant i18n strings in json
On Thu, Aug 7, 2014 at 8:20 AM, Jeff Janes jeff.ja...@gmail.com wrote: It needs to go into 9_4_STABLE as well. It is worth noticing that the buildfarm is completely in red because this patch was not backpatched to REL9_4_STABLE: http://buildfarm.postgresql.org/cgi-bin/show_status.pl Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixed redundant i18n strings in json
On Wed, Aug 6, 2014 at 7:20 PM, Jeff Janes jeff.ja...@gmail.com wrote: On Wed, Aug 6, 2014 at 8:35 AM, Robert Haas robertmh...@gmail.com wrote: On Tue, Aug 5, 2014 at 6:05 PM, Jeff Janes jeff.ja...@gmail.com wrote: I think you missed one of the regression tests, see attached Woops. Thanks, committed. Thanks. It needs to go into 9_4_STABLE as well. Sheesh, where is my brain? Done, thanks. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixed redundant i18n strings in json
Robert Haas robertmh...@gmail.com writes: On Sat, Aug 2, 2014 at 9:15 AM, Daniele Varrazzo daniele.varra...@gmail.com wrote: I'd definitely replace /arg/argument/. Furthermore I'd avoid the form argument 1: something is wrong: the string is likely to end up in sentences with other colons so I'd rather use something is wrong at argument 1. Is the patch attached better? Looks OK to me. I thought someone else might comment, but since no one has, committed. It looks to me like this is still wrong: if (nargs % 2 != 0) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), -errmsg(invalid number or arguments: object must be matched key value pairs))); +errmsg(invalid number or arguments), +errhint(Object must be matched key value pairs.))); Surely that was meant to read invalid number OF arguments. The errhint is only charitably described as English, as well. I'd suggest something like Arguments of json_build_object() must be pairs of keys and values. --- but maybe someone else can phrase that better. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixed redundant i18n strings in json
Tom Lane-2 wrote Robert Haas lt; robertmhaas@ gt; writes: On Sat, Aug 2, 2014 at 9:15 AM, Daniele Varrazzo lt; daniele.varrazzo@ gt; wrote: I'd definitely replace /arg/argument/. Furthermore I'd avoid the form argument 1: something is wrong: the string is likely to end up in sentences with other colons so I'd rather use something is wrong at argument 1. Is the patch attached better? Looks OK to me. I thought someone else might comment, but since no one has, committed. It looks to me like this is still wrong: if (nargs % 2 != 0) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), -errmsg(invalid number or arguments: object must be matched key value pairs))); +errmsg(invalid number or arguments), +errhint(Object must be matched key value pairs.))); Surely that was meant to read invalid number OF arguments. The errhint is only charitably described as English, as well. I'd suggest something like Arguments of json_build_object() must be pairs of keys and values. --- but maybe someone else can phrase that better. The user documentation is worth emulating here: http://www.postgresql.org/docs/9.4/interactive/functions-json.html errmsg(argument count must be divisible by 2) errhint(The argument list consists of alternating names and values) Note that I s/keys/names/ to match said documentation. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Fixed-redundant-i18n-strings-in-json-tp5813330p5814122.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixed redundant i18n strings in json
David G Johnston david.g.johns...@gmail.com writes: Tom Lane-2 wrote Surely that was meant to read invalid number OF arguments. The errhint is only charitably described as English, as well. I'd suggest something like Arguments of json_build_object() must be pairs of keys and values. --- but maybe someone else can phrase that better. The user documentation is worth emulating here: http://www.postgresql.org/docs/9.4/interactive/functions-json.html errmsg(argument count must be divisible by 2) errhint(The argument list consists of alternating names and values) Seems reasonable to me. Note that I s/keys/names/ to match said documentation. Hm. The docs aren't too consistent either: there are several other nearby places that say keys. Notably, the functions json[b]_object_keys() have that usage embedded in their names, where we can't readily change it. I'm inclined to think we should s/names/keys/ in the docs instead. Thoughts? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixed redundant i18n strings in json
On Thu, Aug 7, 2014 at 5:53 PM, Tom Lane t...@sss.pgh.pa.us wrote: David G Johnston david.g.johns...@gmail.com writes: Tom Lane-2 wrote Surely that was meant to read invalid number OF arguments. The errhint is only charitably described as English, as well. I'd suggest something like Arguments of json_build_object() must be pairs of keys and values. --- but maybe someone else can phrase that better. The user documentation is worth emulating here: http://www.postgresql.org/docs/9.4/interactive/functions-json.html errmsg(argument count must be divisible by 2) errhint(The argument list consists of alternating names and values) Seems reasonable to me. Note that I s/keys/names/ to match said documentation. Hm. The docs aren't too consistent either: there are several other nearby places that say keys. Notably, the functions json[b]_object_keys() have that usage embedded in their names, where we can't readily change it. I'm inclined to think we should s/names/keys/ in the docs instead. Thoughts? Agreed - have the docs match the common API term usage in our implementation. Not sure its worth a thorough hunt but at least fix them as they are noticed. David J.
Re: [HACKERS] Fixed redundant i18n strings in json
On Tue, Aug 5, 2014 at 6:05 PM, Jeff Janes jeff.ja...@gmail.com wrote: I think you missed one of the regression tests, see attached Woops. Thanks, committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixed redundant i18n strings in json
On Wed, Aug 6, 2014 at 8:35 AM, Robert Haas robertmh...@gmail.com wrote: On Tue, Aug 5, 2014 at 6:05 PM, Jeff Janes jeff.ja...@gmail.com wrote: I think you missed one of the regression tests, see attached Woops. Thanks, committed. Thanks. It needs to go into 9_4_STABLE as well. Cheers, Jeff
Re: [HACKERS] Fixed redundant i18n strings in json
On Sat, Aug 2, 2014 at 9:15 AM, Daniele Varrazzo daniele.varra...@gmail.com wrote: I'd definitely replace /arg/argument/. Furthermore I'd avoid the form argument 1: something is wrong: the string is likely to end up in sentences with other colons so I'd rather use something is wrong at argument 1. Is the patch attached better? Looks OK to me. I thought someone else might comment, but since no one has, committed. (As a note for the future, you forgot to update the regression test outputs; I took care of that before committing.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixed redundant i18n strings in json
On Tue, Aug 5, 2014 at 9:35 AM, Robert Haas robertmh...@gmail.com wrote: On Sat, Aug 2, 2014 at 9:15 AM, Daniele Varrazzo daniele.varra...@gmail.com wrote: I'd definitely replace /arg/argument/. Furthermore I'd avoid the form argument 1: something is wrong: the string is likely to end up in sentences with other colons so I'd rather use something is wrong at argument 1. Is the patch attached better? Looks OK to me. I thought someone else might comment, but since no one has, committed. (As a note for the future, you forgot to update the regression test outputs; I took care of that before committing.) I think you missed one of the regression tests, see attached Thanks, Jeff regression.diffs Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixed redundant i18n strings in json
I'd definitely replace /arg/argument/. Furthermore I'd avoid the form argument 1: something is wrong: the string is likely to end up in sentences with other colons so I'd rather use something is wrong at argument 1. Is the patch attached better? Cheers, -- Daniele On Thu, Jul 31, 2014 at 1:57 PM, Robert Haas robertmh...@gmail.com wrote: On Wed, Jul 30, 2014 at 2:59 PM, Daniele Varrazzo daniele.varra...@gmail.com wrote: Please find attached a small tweak to a couple of strings found while updating translations. The %d version is already used elsewhere in the same file. Probably not a bad idea, but aren't these strings pretty awful anyway? At a minimum, arg as an abbreviation for argument doesn't seem good. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c index 2f99908..0c55e9a 100644 --- a/src/backend/utils/adt/json.c +++ b/src/backend/utils/adt/json.c @@ -1910,7 +1910,7 @@ json_object_agg_transfn(PG_FUNCTION_ARGS) if (val_type == InvalidOid || val_type == UNKNOWNOID) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg(arg 1: could not determine data type))); + errmsg(could not determine data type for argument %d, 1))); add_json(arg, false, state, val_type, true); @@ -1934,7 +1934,7 @@ json_object_agg_transfn(PG_FUNCTION_ARGS) if (val_type == InvalidOid || val_type == UNKNOWNOID) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg(arg 2: could not determine data type))); + errmsg(could not determine data type for argument %d, 2))); add_json(arg, PG_ARGISNULL(2), state, val_type, false); @@ -1980,7 +1980,8 @@ json_build_object(PG_FUNCTION_ARGS) if (nargs % 2 != 0) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg(invalid number or arguments: object must be matched key value pairs))); + errmsg(invalid number or arguments), + errhint(Object must be matched key value pairs.))); result = makeStringInfo(); @@ -1994,7 +1995,8 @@ json_build_object(PG_FUNCTION_ARGS) if (PG_ARGISNULL(i)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg(arg %d: key cannot be null, i + 1))); + errmsg(argument %d cannot be null, i + 1), + errhint(Object keys should be text.))); val_type = get_fn_expr_argtype(fcinfo-flinfo, i); /* @@ -2016,7 +2018,8 @@ json_build_object(PG_FUNCTION_ARGS) if (val_type == InvalidOid || val_type == UNKNOWNOID) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg(arg %d: could not determine data type, i + 1))); + errmsg(could not determine data type for argument %d, + i + 1))); appendStringInfoString(result, sep); sep = , ; add_json(arg, false, result, val_type, true); @@ -2042,7 +2045,8 @@ json_build_object(PG_FUNCTION_ARGS) if (val_type == InvalidOid || val_type == UNKNOWNOID) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg(arg %d: could not determine data type, i + 2))); + errmsg(could not determine data type for argument %d, + i + 2))); add_json(arg, PG_ARGISNULL(i + 1), result, val_type, false); } @@ -2099,7 +2103,8 @@ json_build_array(PG_FUNCTION_ARGS) if (val_type == InvalidOid || val_type == UNKNOWNOID) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg(arg %d: could not determine data type, i + 1))); + errmsg(could not determine data type for argument %d, + i + 1))); appendStringInfoString(result, sep); sep = , ; add_json(arg, PG_ARGISNULL(i), result, val_type, false); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fixed redundant i18n strings in json
On Wed, Jul 30, 2014 at 2:59 PM, Daniele Varrazzo daniele.varra...@gmail.com wrote: Please find attached a small tweak to a couple of strings found while updating translations. The %d version is already used elsewhere in the same file. Probably not a bad idea, but aren't these strings pretty awful anyway? At a minimum, arg as an abbreviation for argument doesn't seem good. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Fixed redundant i18n strings in json
Please find attached a small tweak to a couple of strings found while updating translations. The %d version is already used elsewhere in the same file. -- Daniele diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c index 2f99908..2aa27cc 100644 --- a/src/backend/utils/adt/json.c +++ b/src/backend/utils/adt/json.c @@ -1910,7 +1910,7 @@ json_object_agg_transfn(PG_FUNCTION_ARGS) if (val_type == InvalidOid || val_type == UNKNOWNOID) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg(arg 1: could not determine data type))); + errmsg(arg %d: could not determine data type, 1))); add_json(arg, false, state, val_type, true); @@ -1934,7 +1934,7 @@ json_object_agg_transfn(PG_FUNCTION_ARGS) if (val_type == InvalidOid || val_type == UNKNOWNOID) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg(arg 2: could not determine data type))); + errmsg(arg %d: could not determine data type, 2))); add_json(arg, PG_ARGISNULL(2), state, val_type, false); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers