[Nikita Glukhov <n.gluhov@postgrespro.ru>]

Attached two small fixes for the previous committed patch:

1. I've noticed a difference in behavior between json_populate_record()
and  jsonb_populate_record() if we are trying to populate record from a
non-JSON-object: json function throws an error but jsonb function returns
a record with NULL fields. So I think it would be better to throw an error
in jsonb case too, but I'm not sure.

2. Also I've found a some kind of thinko in JsGetObjectSize() macro, but it
seems that this obvious mistake can not lead to incorrect behavior.

--
Nikita Glukhov
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

fix-thinko-in-JsObjectSize-macro.patch (text/x-patch, 833B) download | inline diff:
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index ab9a745..a98742f 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -308,12 +308,11 @@ typedef struct JsObject
 	((jsv)->is_json ? (jsv)->val.json.type == JSON_TOKEN_STRING \
 		: ((jsv)->val.jsonb && (jsv)->val.jsonb->type == jbvString))
 
-#define JsObjectSize(jso) \
+#define JsObjectIsEmpty(jso) \
 	((jso)->is_json \
-		? hash_get_num_entries((jso)->val.json_hash) \
-		: !(jso)->val.jsonb_cont || JsonContainerSize((jso)->val.jsonb_cont))
-
-#define JsObjectIsEmpty(jso) (JsObjectSize(jso) == 0)
+		? hash_get_num_entries((jso)->val.json_hash) == 0 \
+		: (!(jso)->val.jsonb_cont || \
+		   JsonContainerSize((jso)->val.jsonb_cont) == 0))
 
 #define JsObjectFree(jso) do { \
 		if ((jso)->is_json) \

fix-jsonb_populate_record-behavior-on-non-objects.patch (text/x-patch, 2.1K) download | inline diff:
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index a98742f..083982c 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -2683,7 +2683,20 @@ JsValueToJsObject(JsValue *jsv, JsObject *jso)
 			JsonContainerIsObject(jbv->val.binary.data))
 			jso->val.jsonb_cont = jbv->val.binary.data;
 		else
+		{
+			bool		is_scalar = IsAJsonbScalar(jbv) ||
+				(jbv->type == jbvBinary &&
+				 JsonContainerIsScalar(jbv->val.binary.data));
+
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg(is_scalar
+							? "cannot call %s on a scalar"
+							: "cannot call %s on an array",
+							"populate_composite")));
+
 			jso->val.jsonb_cont = NULL;
+		}
 	}
 }
 
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index f199eb4..7954703 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -2276,17 +2276,9 @@ SELECT jsa FROM jsonb_populate_record(NULL::jsbrec, '{"jsa": ["aaa", null, [1, 2
 (1 row)
 
 SELECT rec FROM jsonb_populate_record(NULL::jsbrec, '{"rec": 123}') q;
- rec  
-------
- (,,)
-(1 row)
-
+ERROR:  cannot call populate_composite on a scalar
 SELECT rec FROM jsonb_populate_record(NULL::jsbrec, '{"rec": [1, 2]}') q;
- rec  
-------
- (,,)
-(1 row)
-
+ERROR:  cannot call populate_composite on an array
 SELECT rec FROM jsonb_populate_record(NULL::jsbrec, '{"rec": {"a": "abc", "c": "01.02.2003", "x": 43.2}}') q;
                 rec                
 -----------------------------------
@@ -2303,11 +2295,7 @@ SELECT reca FROM jsonb_populate_record(NULL::jsbrec, '{"reca": 123}') q;
 ERROR:  expected json array
 HINT:  see the value of key "reca"
 SELECT reca FROM jsonb_populate_record(NULL::jsbrec, '{"reca": [1, 2]}') q;
-      reca       
------------------
- {"(,,)","(,,)"}
-(1 row)
-
+ERROR:  cannot call populate_composite on a scalar
 SELECT reca FROM jsonb_populate_record(NULL::jsbrec, '{"reca": [{"a": "abc", "b": 456}, null, {"c": "01.02.2003", "x": 43.2}]}') q;
                           reca                          
 --------------------------------------------------------