Re: [HACKERS] Fixed redundant i18n strings in json

2014-08-07 Thread Michael Paquier
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

2014-08-07 Thread Robert Haas
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

2014-08-07 Thread Tom Lane
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

2014-08-07 Thread David G Johnston
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

2014-08-07 Thread Tom Lane
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

2014-08-07 Thread David Johnston
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

2014-08-06 Thread Robert Haas
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

2014-08-06 Thread Jeff Janes
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

2014-08-05 Thread Robert Haas
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

2014-08-05 Thread Jeff Janes
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

2014-08-02 Thread Daniele Varrazzo
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

2014-07-31 Thread Robert Haas
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

2014-07-30 Thread Daniele Varrazzo
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