Re: [HACKERS] jsonb generator functions
On 12/08/2014 01:00 PM, Andrew Dunstan wrote: On 12/08/2014 04:21 AM, Alvaro Herrera wrote: Andrew Dunstan wrote: OK, here is a new patch version that * uses find_coercion_path() to find the cast function if any, as discussed elsewhere * removes calls to getTypeOutputInfo() except where required * honors a cast to json only for rendering both json and jsonb * adds processing for the date type that was previously missing in datum_to_jsonb Did this go anywhere? Not, yet. I hope to get to it this week. OK, here is a new version. The major change is that the aggregate final functions now clone the transition value rather than modifying it directly, avoiding a similar nearby error which Tom fixed recently. Also here is a patch factored out which applies the find_coercion_pathway change to json.c. I'm inclined to say we should backpatch this to 9.4 (and with a small change 9.3). Thoughts? cheers andrew -- 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] jsonb generator functions
On 12/12/2014 01:10 PM, Andrew Dunstan wrote: On 12/08/2014 01:00 PM, Andrew Dunstan wrote: On 12/08/2014 04:21 AM, Alvaro Herrera wrote: Andrew Dunstan wrote: OK, here is a new patch version that * uses find_coercion_path() to find the cast function if any, as discussed elsewhere * removes calls to getTypeOutputInfo() except where required * honors a cast to json only for rendering both json and jsonb * adds processing for the date type that was previously missing in datum_to_jsonb Did this go anywhere? Not, yet. I hope to get to it this week. OK, here is a new version. The major change is that the aggregate final functions now clone the transition value rather than modifying it directly, avoiding a similar nearby error which Tom fixed recently. Also here is a patch factored out which applies the find_coercion_pathway change to json.c. I'm inclined to say we should backpatch this to 9.4 (and with a small change 9.3). Thoughts? Er this time with patches. cheers andrew diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index da138e1..ef69b94 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -10245,9 +10245,10 @@ table2-mapping para xref linkend=functions-json-creation-table shows the functions that are - available for creating typejson/type values. - (Currently, there are no equivalent functions for typejsonb/, but you - can cast the result of one of these functions to typejsonb/.) + available for creating typejson/type and typejsonb/type values. + (There are no equivalent functions for typejsonb/, of the literalrow_to_json/ + and literalarray_to_json/ functions. However, the literalto_jsonb/ + function supplies much the same functionality as these functions would.) /para indexterm @@ -10268,6 +10269,18 @@ table2-mapping indexterm primaryjson_object/primary /indexterm + indexterm + primaryto_jsonb/primary + /indexterm + indexterm + primaryjsonb_build_array/primary + /indexterm + indexterm + primaryjsonb_build_object/primary + /indexterm + indexterm + primaryjsonb_object/primary + /indexterm table id=functions-json-creation-table titleJSON Creation Functions/title @@ -10282,17 +10295,18 @@ table2-mapping /thead tbody row + entryparaliteralto_json(anyelement)/literal + /paraparaliteralto_jsonb(anyelement)/literal + /para/entry entry - literalto_json(anyelement)/literal - /entry - entry - Returns the value as JSON. Arrays and composites are converted + Returns the value as typejson/ or typejsonb/. + Arrays and composites are converted (recursively) to arrays and objects; otherwise, if there is a cast from the type to typejson/type, the cast function will be used to - perform the conversion; otherwise, a JSON scalar value is produced. + perform the conversion; otherwise, a scalar value is produced. For any scalar type other than a number, a Boolean, or a null value, - the text representation will be used, properly quoted and escaped - so that it is a valid JSON string. + the text representation will be used, in such a fashion that it is a + valid typejson/ or typejsonb/ value. /entry entryliteralto_json('Fred said Hi.'::text)/literal/entry entryliteralFred said \Hi.\/literal/entry @@ -10321,9 +10335,9 @@ table2-mapping entryliteral{f1:1,f2:foo}/literal/entry /row row - entry - literaljson_build_array(VARIADIC any)/literal - /entry + entryparaliteraljson_build_array(VARIADIC any)/literal + /paraparaliteraljsonb_build_array(VARIADIC any)/literal + /para/entry entry Builds a possibly-heterogeneously-typed JSON array out of a variadic argument list. @@ -10332,9 +10346,9 @@ table2-mapping entryliteral[1, 2, 3, 4, 5]/literal/entry /row row - entry - literaljson_build_object(VARIADIC any)/literal - /entry + entryparaliteraljson_build_object(VARIADIC any)/literal + /paraparaliteraljsonb_build_object(VARIADIC any)/literal + /para/entry entry Builds a JSON object out of a variadic argument list. By convention, the argument list consists of alternating @@ -10344,9 +10358,9 @@ table2-mapping entryliteral{foo: 1, bar: 2}/literal/entry /row row - entry - literaljson_object(text[])/literal - /entry + entryparaliteraljson_object(text[])/literal + /paraparaliteraljsonb_object(text[])/literal + /para/entry entry Builds a JSON object out of a text array. The array must have either exactly one dimension with an even number of members, in which case @@ -10359,9 +10373,9 @@ table2-mapping entryliteral{a: 1, b: def, c:
Re: [HACKERS] jsonb generator functions
Andrew Dunstan and...@dunslane.net writes: Also here is a patch factored out which applies the find_coercion_pathway change to json.c. I'm inclined to say we should backpatch this to 9.4 (and with a small change 9.3). Thoughts? Meh. Maybe I'm just feeling gunshy because I broke something within the past 24 hours, but at this point (with 9.4.0 wrap only 3 days away) I'm inclined to avoid any 9.4 code churn that's not clearly necessary. You argued upthread that this change would not result in any behavioral changes in which cast method gets selected. If that's true, then we don't really need to back-patch; while if it turns out not to be true, we definitely don't want it in 9.3 and I'd argue it's too late for 9.4 also. In short, I think it's fine for the 9.4 JSON code to start diverging from HEAD at this point ... 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] jsonb generator functions
On 12/12/2014 01:55 PM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Also here is a patch factored out which applies the find_coercion_pathway change to json.c. I'm inclined to say we should backpatch this to 9.4 (and with a small change 9.3). Thoughts? Meh. Maybe I'm just feeling gunshy because I broke something within the past 24 hours, but at this point (with 9.4.0 wrap only 3 days away) I'm inclined to avoid any 9.4 code churn that's not clearly necessary. You argued upthread that this change would not result in any behavioral changes in which cast method gets selected. If that's true, then we don't really need to back-patch; while if it turns out not to be true, we definitely don't want it in 9.3 and I'd argue it's too late for 9.4 also. In short, I think it's fine for the 9.4 JSON code to start diverging from HEAD at this point ... Ok cheers andrew -- 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] jsonb generator functions
Andrew Dunstan wrote: OK, here is a new patch version that * uses find_coercion_path() to find the cast function if any, as discussed elsewhere * removes calls to getTypeOutputInfo() except where required * honors a cast to json only for rendering both json and jsonb * adds processing for the date type that was previously missing in datum_to_jsonb Did this go anywhere? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- 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] jsonb generator functions
On 12/08/2014 04:21 AM, Alvaro Herrera wrote: Andrew Dunstan wrote: OK, here is a new patch version that * uses find_coercion_path() to find the cast function if any, as discussed elsewhere * removes calls to getTypeOutputInfo() except where required * honors a cast to json only for rendering both json and jsonb * adds processing for the date type that was previously missing in datum_to_jsonb Did this go anywhere? Not, yet. I hope to get to it this week. cheers andrew -- 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] jsonb generator functions
On 10/28/2014 09:49 AM, Andrew Dunstan wrote: On 10/27/2014 05:57 PM, Alvaro Herrera wrote: Anyway this whole business of searching through the CASTSOURCETARGET syscache seems like it could be refactored. If I'm counting correctly, that block now appears four times (three in this patch, once in json.c). Can't we add a new function to (say) lsyscache and remove that? Twice, not three times in this patch, unless I'm going crazier than I thought. I can add a function to lsyscache along the lines of Oid get_cast_func(Oid from_type, Oid to_type) if you think it's worth it. OK, here is a new patch version that * uses find_coercion_path() to find the cast function if any, as discussed elsewhere * removes calls to getTypeOutputInfo() except where required * honors a cast to json only for rendering both json and jsonb * adds processing for the date type that was previously missing in datum_to_jsonb cheers andrew diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 7e5bcd9..fad0e79 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -10245,9 +10245,10 @@ table2-mapping para xref linkend=functions-json-creation-table shows the functions that are - available for creating typejson/type values. - (Currently, there are no equivalent functions for typejsonb/, but you - can cast the result of one of these functions to typejsonb/.) + available for creating typejson/type and typejsonb/type values. + (There are no equivalent functions for typejsonb/, of the literalrow_to_json/ + and literalarray_to_json/ functions. However, the literalto_jsonb/ + function supplies much the same functionality as these functions would.) /para indexterm @@ -10268,6 +10269,18 @@ table2-mapping indexterm primaryjson_object/primary /indexterm + indexterm + primaryto_jsonb/primary + /indexterm + indexterm + primaryjsonb_build_array/primary + /indexterm + indexterm + primaryjsonb_build_object/primary + /indexterm + indexterm + primaryjsonb_object/primary + /indexterm table id=functions-json-creation-table titleJSON Creation Functions/title @@ -10282,17 +10295,18 @@ table2-mapping /thead tbody row + entryparaliteralto_json(anyelement)/literal + /paraparaliteralto_jsonb(anyelement)/literal + /para/entry entry - literalto_json(anyelement)/literal - /entry - entry - Returns the value as JSON. Arrays and composites are converted + Returns the value as typejson/ or typejsonb/. + Arrays and composites are converted (recursively) to arrays and objects; otherwise, if there is a cast from the type to typejson/type, the cast function will be used to - perform the conversion; otherwise, a JSON scalar value is produced. + perform the conversion; otherwise, a scalar value is produced. For any scalar type other than a number, a Boolean, or a null value, - the text representation will be used, properly quoted and escaped - so that it is a valid JSON string. + the text representation will be used, in such a fashion that it is a + valid typejson/ or typejsonb/ value. /entry entryliteralto_json('Fred said Hi.'::text)/literal/entry entryliteralFred said \Hi.\/literal/entry @@ -10321,9 +10335,9 @@ table2-mapping entryliteral{f1:1,f2:foo}/literal/entry /row row - entry - literaljson_build_array(VARIADIC any)/literal - /entry + entryparaliteraljson_build_array(VARIADIC any)/literal + /paraparaliteraljsonb_build_array(VARIADIC any)/literal + /para/entry entry Builds a possibly-heterogeneously-typed JSON array out of a variadic argument list. @@ -10332,9 +10346,9 @@ table2-mapping entryliteral[1, 2, 3, 4, 5]/literal/entry /row row - entry - literaljson_build_object(VARIADIC any)/literal - /entry + entryparaliteraljson_build_object(VARIADIC any)/literal + /paraparaliteraljsonb_build_object(VARIADIC any)/literal + /para/entry entry Builds a JSON object out of a variadic argument list. By convention, the argument list consists of alternating @@ -10344,9 +10358,9 @@ table2-mapping entryliteral{foo: 1, bar: 2}/literal/entry /row row - entry - literaljson_object(text[])/literal - /entry + entryparaliteraljson_object(text[])/literal + /paraparaliteraljsonb_object(text[])/literal + /para/entry entry Builds a JSON object out of a text array. The array must have either exactly one dimension with an even number of members, in which case @@ -10359,9 +10373,9 @@ table2-mapping entryliteral{a: 1, b: def, c: 3.5}/literal/entry /row row - entry -
Re: [HACKERS] jsonb generator functions
Andrew Dunstan wrote: This bit: +/* + * Determine how we want to render values of a given type in datum_to_jsonb. + * + * Given the datatype OID, return its JsonbTypeCategory, as well as the type's + * output function OID. If the returned category is JSONBTYPE_CAST, we + * return the OID of the type-JSON cast function instead. + */ +static void +jsonb_categorize_type(Oid typoid, + JsonbTypeCategory * tcategory, + Oid *outfuncoid) +{ seems like it can return without having set the category and func OID, if there's no available cast. Callers don't seem to check for this condition; is this a bug? If not, why not? Maybe some extra comments are warranted. Right now, for the general case there, there are two syscache lookups rather than one. The fix is simple: just do the getTypeOutputInfo call inside each case inside the switch instead of once at the beginning, so that the general case can omit it; then there is just one syscache access in all the cases. json.c suffers from the same problem. Anyway this whole business of searching through the CASTSOURCETARGET syscache seems like it could be refactored. If I'm counting correctly, that block now appears four times (three in this patch, once in json.c). Can't we add a new function to (say) lsyscache and remove that? I'm just commenting on that part because the syscache.h/pg_cast.h inclusions look a bit out of place; it's certainly not a serious issue. I looked at what makes you include miscadmin.h. It's only USE_XSD_DATES as far as I can tell. I looked at how that might be fixed, and a quick patch that moves DateStyle, DateOrder and IntervalStyle (and associated definitions) to datetime.h seems to work pretty well ... except that initdb.c requires to know about some DATEORDER constants; but frontend code cannot include datetime.h because of Datum. So that idea crashed and burned until someone reorganizes the whole datetime code, which currently is pretty messy. I don't have any further comments on this patch, other than please add JsonbTypeCategory to pgindent/typedefs.list before doing your pgindent run. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] jsonb generator functions
On 10/27/2014 05:57 PM, Alvaro Herrera wrote: Andrew Dunstan wrote: This bit: +/* + * Determine how we want to render values of a given type in datum_to_jsonb. + * + * Given the datatype OID, return its JsonbTypeCategory, as well as the type's + * output function OID. If the returned category is JSONBTYPE_CAST, we + * return the OID of the type-JSON cast function instead. + */ +static void +jsonb_categorize_type(Oid typoid, + JsonbTypeCategory * tcategory, + Oid *outfuncoid) +{ seems like it can return without having set the category and func OID, if there's no available cast. Callers don't seem to check for this condition; is this a bug? If not, why not? Maybe some extra comments are warranted. Umm, no. The outfuncoid is set by the call to getTypeOutputInfo() and the category is set by every branch of the switch. We override the funcoid in the case where there's a cast to json or jsonb. I'll add a comment to that effect. Right now, for the general case there, there are two syscache lookups rather than one. The fix is simple: just do the getTypeOutputInfo call inside each case inside the switch instead of once at the beginning, so that the general case can omit it; then there is just one syscache access in all the cases. json.c suffers from the same problem. We only do more than one if it's not a builtin type, or an array or composite. So 99% of the time this won't even be called. Anyway this whole business of searching through the CASTSOURCETARGET syscache seems like it could be refactored. If I'm counting correctly, that block now appears four times (three in this patch, once in json.c). Can't we add a new function to (say) lsyscache and remove that? Twice, not three times in this patch, unless I'm going crazier than I thought. I can add a function to lsyscache along the lines of Oid get_cast_func(Oid from_type, Oid to_type) if you think it's worth it. cheers andrew -- 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] jsonb generator functions
On 10/15/2014 03:54 PM, Andrew Dunstan wrote: I checked a code, and I have only two small objection - a name jsonb_object_two_arg is not good - maybe json_object_keys_values ? It's consistent with the existing json_object_two_arg. In all cases I think I kept the names the same except for changing json to jsonb. Note that these _two_arg functions are not visible at the SQL level - they are only visible in the C code. I'm happy to be guided by others in changing or keeping these names. Next: there are no tests for to_jsonb function. Oh, my bad. I'll add some. Thank for the review. Here is a new patch that includes documentation and addresses all these issues, except that I didn't change the name of jsonb_object_two_arg to keep it consistent with the name of json_object_two_arg. I'm happy to change both if people feel it matters. cheers andrew diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 7e5bcd9..21ce293 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -10245,9 +10245,10 @@ table2-mapping para xref linkend=functions-json-creation-table shows the functions that are - available for creating typejson/type values. - (Currently, there are no equivalent functions for typejsonb/, but you - can cast the result of one of these functions to typejsonb/.) + available for creating typejson/type and typejsonb/type values. + (There are no equivalent functions for typejsonb/, of the literalrow_to_json/ + and literalarray_to_json/ functions. However, the literalto_jsonb/ + function supplies much the same functionality as these functions would.) /para indexterm @@ -10268,6 +10269,18 @@ table2-mapping indexterm primaryjson_object/primary /indexterm + indexterm + primaryto_jsonb/primary + /indexterm + indexterm + primaryjsonb_build_array/primary + /indexterm + indexterm + primaryjsonb_build_object/primary + /indexterm + indexterm + primaryjsonb_object/primary + /indexterm table id=functions-json-creation-table titleJSON Creation Functions/title @@ -10282,17 +10295,19 @@ table2-mapping /thead tbody row + entryparaliteralto_json(anyelement)/literal + /paraparaliteralto_jsonb(anyelement)/literal + /para/entry entry - literalto_json(anyelement)/literal - /entry - entry - Returns the value as JSON. Arrays and composites are converted + Returns the value as typejson/ or typejsonb/. + Arrays and composites are converted (recursively) to arrays and objects; otherwise, if there is a cast - from the type to typejson/type, the cast function will be used to - perform the conversion; otherwise, a JSON scalar value is produced. + from the type to typejson/type or typejsonb/ in the case of + literalto_jsonb/, the cast function will be used to + perform the conversion; otherwise, a scalar value is produced. For any scalar type other than a number, a Boolean, or a null value, - the text representation will be used, properly quoted and escaped - so that it is a valid JSON string. + the text representation will be used, in such a fashion that it is a + valid typejson/ or typejsonb/ value. /entry entryliteralto_json('Fred said Hi.'::text)/literal/entry entryliteralFred said \Hi.\/literal/entry @@ -10321,9 +10336,9 @@ table2-mapping entryliteral{f1:1,f2:foo}/literal/entry /row row - entry - literaljson_build_array(VARIADIC any)/literal - /entry + entryparaliteraljson_build_array(VARIADIC any)/literal + /paraparaliteraljsonb_build_array(VARIADIC any)/literal + /para/entry entry Builds a possibly-heterogeneously-typed JSON array out of a variadic argument list. @@ -10332,9 +10347,9 @@ table2-mapping entryliteral[1, 2, 3, 4, 5]/literal/entry /row row - entry - literaljson_build_object(VARIADIC any)/literal - /entry + entryparaliteraljson_build_object(VARIADIC any)/literal + /paraparaliteraljsonb_build_object(VARIADIC any)/literal + /para/entry entry Builds a JSON object out of a variadic argument list. By convention, the argument list consists of alternating @@ -10344,9 +10359,9 @@ table2-mapping entryliteral{foo: 1, bar: 2}/literal/entry /row row - entry - literaljson_object(text[])/literal - /entry + entryparaliteraljson_object(text[])/literal + /paraparaliteraljsonb_object(text[])/literal + /para/entry entry Builds a JSON object out of a text array. The array must have either exactly one dimension with an even number of members, in which case @@ -10359,9 +10374,9 @@ table2-mapping entryliteral{a: 1, b: def, c:
Re: [HACKERS] jsonb generator functions
Hi 2014-10-27 15:33 GMT+01:00 Andrew Dunstan and...@dunslane.net: On 10/15/2014 03:54 PM, Andrew Dunstan wrote: I checked a code, and I have only two small objection - a name jsonb_object_two_arg is not good - maybe json_object_keys_values ? It's consistent with the existing json_object_two_arg. In all cases I think I kept the names the same except for changing json to jsonb. Note that these _two_arg functions are not visible at the SQL level - they are only visible in the C code. I'm happy to be guided by others in changing or keeping these names. Next: there are no tests for to_jsonb function. Oh, my bad. I'll add some. Thank for the review. Here is a new patch that includes documentation and addresses all these issues, except that I didn't change the name of jsonb_object_two_arg to keep it consistent with the name of json_object_two_arg. I'm happy to change both if people feel it matters. I checked last patch jsonbmissingfunc5.patch and I have no any objections: 1. This jsonb API is consistent with current JSON API, so we surely would to this functionality 2. A implementation is clean without side effects and without impact on current code. 3. Patching and compilation are without any issues and warnings 4. Source code respects PostgreSQL coding rules 5. All regress tests was passed without problems 6. Documentation was build without problems 7. Patch contains necessary regress tests 8. Patch contains necessary documentation for new functions. Patch is ready for commiters Thank you for patch Regards Pavel cheers andrew
Re: [HACKERS] jsonb generator functions
2014-10-15 23:49 GMT+02:00 Andrew Dunstan and...@dunslane.net: On 10/15/2014 05:47 PM, Alvaro Herrera wrote: Andrew Dunstan wrote: If we really want to change the name of json_object_two_arg, it would probably be best to change it NOW in 9.4 before it gets out into a production release at all. Doesn't it require initdb? If so, I think it's too late now. yes, it is too heavy argument. ok Pavel Yeah, you're right, it would. OK, forget that. cheers andrew
Re: [HACKERS] jsonb generator functions
2014-10-13 17:22 GMT+02:00 Andrew Dunstan and...@dunslane.net: On 10/13/2014 09:37 AM, Andrew Dunstan wrote: On 09/26/2014 04:54 PM, Andrew Dunstan wrote: Here is a patch for the generator and aggregate functions for jsonb that we didn't manage to get done in time for 9.4. They are all equivalents of the similarly names json functions. Included are to_jsonb jsonb_build_object jsonb_build_array jsonb_object jsonb_agg jsonb_object_agg Still to come: documentation. Adding to the next commitfest. Revised patch to fix compiler warnings. And again, initializing an incompletely initialized variable, as found by Pavel Stehule. I checked a code, and I have only two small objection - a name jsonb_object_two_arg is not good - maybe json_object_keys_values ? Next: there are no tests for to_jsonb function. Regards Pavel cheers andrew -- 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] jsonb generator functions
On 10/15/2014 07:38 AM, Pavel Stehule wrote: 2014-10-13 17:22 GMT+02:00 Andrew Dunstan and...@dunslane.net mailto:and...@dunslane.net: On 10/13/2014 09:37 AM, Andrew Dunstan wrote: On 09/26/2014 04:54 PM, Andrew Dunstan wrote: Here is a patch for the generator and aggregate functions for jsonb that we didn't manage to get done in time for 9.4. They are all equivalents of the similarly names json functions. Included are to_jsonb jsonb_build_object jsonb_build_array jsonb_object jsonb_agg jsonb_object_agg Still to come: documentation. Adding to the next commitfest. Revised patch to fix compiler warnings. And again, initializing an incompletely initialized variable, as found by Pavel Stehule. I checked a code, and I have only two small objection - a name jsonb_object_two_arg is not good - maybe json_object_keys_values ? It's consistent with the existing json_object_two_arg. In all cases I think I kept the names the same except for changing json to jsonb. Note that these _two_arg functions are not visible at the SQL level - they are only visible in the C code. I'm happy to be guided by others in changing or keeping these names. Next: there are no tests for to_jsonb function. Oh, my bad. I'll add some. Thank for the review. cheers andrew -- 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] jsonb generator functions
On 10/15/2014 03:54 PM, I wrote: On 10/15/2014 07:38 AM, Pavel Stehule wrote: I checked a code, and I have only two small objection - a name jsonb_object_two_arg is not good - maybe json_object_keys_values ? It's consistent with the existing json_object_two_arg. In all cases I think I kept the names the same except for changing json to jsonb. Note that these _two_arg functions are not visible at the SQL level - they are only visible in the C code. I'm happy to be guided by others in changing or keeping these names. If we really want to change the name of json_object_two_arg, it would probably be best to change it NOW in 9.4 before it gets out into a production release at all. Thoughts? cheers andrew -- 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] jsonb generator functions
Andrew Dunstan wrote: If we really want to change the name of json_object_two_arg, it would probably be best to change it NOW in 9.4 before it gets out into a production release at all. Doesn't it require initdb? If so, I think it's too late now. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- 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] jsonb generator functions
On 10/15/2014 05:47 PM, Alvaro Herrera wrote: Andrew Dunstan wrote: If we really want to change the name of json_object_two_arg, it would probably be best to change it NOW in 9.4 before it gets out into a production release at all. Doesn't it require initdb? If so, I think it's too late now. Yeah, you're right, it would. OK, forget that. cheers andrew -- 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] jsonb generator functions
On 09/26/2014 04:54 PM, Andrew Dunstan wrote: Here is a patch for the generator and aggregate functions for jsonb that we didn't manage to get done in time for 9.4. They are all equivalents of the similarly names json functions. Included are to_jsonb jsonb_build_object jsonb_build_array jsonb_object jsonb_agg jsonb_object_agg Still to come: documentation. Adding to the next commitfest. Revised patch to fix compiler warnings. cheers andrew diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c index 2fd87fc..2712761 100644 --- a/src/backend/utils/adt/jsonb.c +++ b/src/backend/utils/adt/jsonb.c @@ -12,11 +12,20 @@ */ #include postgres.h +#include miscadmin.h +#include access/htup_details.h +#include access/transam.h +#include catalog/pg_cast.h +#include catalog/pg_type.h #include libpq/pqformat.h #include utils/builtins.h +#include utils/datetime.h +#include utils/lsyscache.h #include utils/json.h #include utils/jsonapi.h #include utils/jsonb.h +#include utils/syscache.h +#include utils/typcache.h typedef struct JsonbInState { @@ -24,6 +33,23 @@ typedef struct JsonbInState JsonbValue *res; } JsonbInState; +/* unlike with json categories, we need to treat json and jsonb differently */ +typedef enum /* type categories for datum_to_jsonb */ +{ + JSONBTYPE_NULL,/* null, so we didn't bother to identify */ + JSONBTYPE_BOOL,/* boolean (built-in types only) */ + JSONBTYPE_NUMERIC, /* numeric (ditto) */ + JSONBTYPE_TIMESTAMP, /* we use special formatting for timestamp */ + JSONBTYPE_TIMESTAMPTZ, /* ... and timestamptz */ + JSONBTYPE_JSON,/* JSON */ + JSONBTYPE_JSONB, /* JSONB */ + JSONBTYPE_ARRAY, /* array */ + JSONBTYPE_COMPOSITE, /* composite */ + JSONBTYPE_JSONCAST, /* something with an explicit cast to JSON */ + JSONBTYPE_JSONBCAST, /* something with an explicit cast to JSONB */ + JSONBTYPE_OTHER/* all else */ +} JsonbTypeCategory; + static inline Datum jsonb_from_cstring(char *json, int len); static size_t checkStringLen(size_t len); static void jsonb_in_object_start(void *pstate); @@ -33,6 +59,22 @@ static void jsonb_in_array_end(void *pstate); static void jsonb_in_object_field_start(void *pstate, char *fname, bool isnull); static void jsonb_put_escaped_value(StringInfo out, JsonbValue *scalarVal); static void jsonb_in_scalar(void *pstate, char *token, JsonTokenType tokentype); +static void jsonb_categorize_type(Oid typoid, + JsonbTypeCategory * tcategory, + Oid *outfuncoid); +static void composite_to_jsonb(Datum composite, JsonbInState *result); +static void array_dim_to_jsonb(JsonbInState *result, int dim, int ndims, int *dims, + Datum *vals, bool *nulls, int *valcount, + JsonbTypeCategory tcategory, Oid outfuncoid); +static void array_to_jsonb_internal(Datum array, JsonbInState *result); +static void jsonb_categorize_type(Oid typoid, + JsonbTypeCategory * tcategory, + Oid *outfuncoid); +static void datum_to_jsonb(Datum val, bool is_null, JsonbInState *result, + JsonbTypeCategory tcategory, Oid outfuncoid, + bool key_scalar); +static void add_jsonb(Datum val, bool is_null, JsonbInState *result, + Oid val_type, bool key_scalar); /* * jsonb type input function @@ -462,3 +504,1282 @@ JsonbToCString(StringInfo out, JsonbContainer *in, int estimated_len) return out-data; } + + +/* + * Determine how we want to render values of a given type in datum_to_jsonb. + * + * Given the datatype OID, return its JsonbTypeCategory, as well as the type's + * output function OID. If the returned category is JSONBTYPE_CAST, we + * return the OID of the type-JSON cast function instead. + */ +static void +jsonb_categorize_type(Oid typoid, + JsonbTypeCategory * tcategory, + Oid *outfuncoid) +{ + bool typisvarlena; + + /* Look through any domain */ + typoid = getBaseType(typoid); + + /* We'll usually need to return the type output function */ + getTypeOutputInfo(typoid, outfuncoid, typisvarlena); + + /* Check for known types */ + switch (typoid) + { + case BOOLOID: + *tcategory = JSONBTYPE_BOOL; + break; + + case INT2OID: + case INT4OID: + case INT8OID: + case FLOAT4OID: + case FLOAT8OID: + case NUMERICOID: + *tcategory = JSONBTYPE_NUMERIC; + break; + + case TIMESTAMPOID: + *tcategory = JSONBTYPE_TIMESTAMP; + break; + + case TIMESTAMPTZOID: + *tcategory = JSONBTYPE_TIMESTAMPTZ; + break; + + case JSONBOID: + *tcategory = JSONBTYPE_JSONB; + break; + + case JSONOID: + *tcategory = JSONBTYPE_JSON; + break; + + default: + /* Check for arrays and composites */ + if (OidIsValid(get_element_type(typoid))) +*tcategory = JSONBTYPE_ARRAY; + else if (type_is_rowtype(typoid)) +*tcategory = JSONBTYPE_COMPOSITE; + else + { +/* It's probably the general case ... */ +*tcategory = JSONBTYPE_OTHER; + +/* + * but let's look for a cast to json or jsonb, if it's not + * built-in + */ +
Re: [HACKERS] jsonb generator functions
On 10/13/2014 09:37 AM, Andrew Dunstan wrote: On 09/26/2014 04:54 PM, Andrew Dunstan wrote: Here is a patch for the generator and aggregate functions for jsonb that we didn't manage to get done in time for 9.4. They are all equivalents of the similarly names json functions. Included are to_jsonb jsonb_build_object jsonb_build_array jsonb_object jsonb_agg jsonb_object_agg Still to come: documentation. Adding to the next commitfest. Revised patch to fix compiler warnings. And again, initializing an incompletely initialized variable, as found by Pavel Stehule. cheers andrew diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c index 2fd87fc..33a19be 100644 --- a/src/backend/utils/adt/jsonb.c +++ b/src/backend/utils/adt/jsonb.c @@ -12,11 +12,20 @@ */ #include postgres.h +#include miscadmin.h +#include access/htup_details.h +#include access/transam.h +#include catalog/pg_cast.h +#include catalog/pg_type.h #include libpq/pqformat.h #include utils/builtins.h +#include utils/datetime.h +#include utils/lsyscache.h #include utils/json.h #include utils/jsonapi.h #include utils/jsonb.h +#include utils/syscache.h +#include utils/typcache.h typedef struct JsonbInState { @@ -24,6 +33,23 @@ typedef struct JsonbInState JsonbValue *res; } JsonbInState; +/* unlike with json categories, we need to treat json and jsonb differently */ +typedef enum /* type categories for datum_to_jsonb */ +{ + JSONBTYPE_NULL,/* null, so we didn't bother to identify */ + JSONBTYPE_BOOL,/* boolean (built-in types only) */ + JSONBTYPE_NUMERIC, /* numeric (ditto) */ + JSONBTYPE_TIMESTAMP, /* we use special formatting for timestamp */ + JSONBTYPE_TIMESTAMPTZ, /* ... and timestamptz */ + JSONBTYPE_JSON,/* JSON */ + JSONBTYPE_JSONB, /* JSONB */ + JSONBTYPE_ARRAY, /* array */ + JSONBTYPE_COMPOSITE, /* composite */ + JSONBTYPE_JSONCAST, /* something with an explicit cast to JSON */ + JSONBTYPE_JSONBCAST, /* something with an explicit cast to JSONB */ + JSONBTYPE_OTHER/* all else */ +} JsonbTypeCategory; + static inline Datum jsonb_from_cstring(char *json, int len); static size_t checkStringLen(size_t len); static void jsonb_in_object_start(void *pstate); @@ -33,6 +59,22 @@ static void jsonb_in_array_end(void *pstate); static void jsonb_in_object_field_start(void *pstate, char *fname, bool isnull); static void jsonb_put_escaped_value(StringInfo out, JsonbValue *scalarVal); static void jsonb_in_scalar(void *pstate, char *token, JsonTokenType tokentype); +static void jsonb_categorize_type(Oid typoid, + JsonbTypeCategory * tcategory, + Oid *outfuncoid); +static void composite_to_jsonb(Datum composite, JsonbInState *result); +static void array_dim_to_jsonb(JsonbInState *result, int dim, int ndims, int *dims, + Datum *vals, bool *nulls, int *valcount, + JsonbTypeCategory tcategory, Oid outfuncoid); +static void array_to_jsonb_internal(Datum array, JsonbInState *result); +static void jsonb_categorize_type(Oid typoid, + JsonbTypeCategory * tcategory, + Oid *outfuncoid); +static void datum_to_jsonb(Datum val, bool is_null, JsonbInState *result, + JsonbTypeCategory tcategory, Oid outfuncoid, + bool key_scalar); +static void add_jsonb(Datum val, bool is_null, JsonbInState *result, + Oid val_type, bool key_scalar); /* * jsonb type input function @@ -462,3 +504,1284 @@ JsonbToCString(StringInfo out, JsonbContainer *in, int estimated_len) return out-data; } + + +/* + * Determine how we want to render values of a given type in datum_to_jsonb. + * + * Given the datatype OID, return its JsonbTypeCategory, as well as the type's + * output function OID. If the returned category is JSONBTYPE_CAST, we + * return the OID of the type-JSON cast function instead. + */ +static void +jsonb_categorize_type(Oid typoid, + JsonbTypeCategory * tcategory, + Oid *outfuncoid) +{ + bool typisvarlena; + + /* Look through any domain */ + typoid = getBaseType(typoid); + + /* We'll usually need to return the type output function */ + getTypeOutputInfo(typoid, outfuncoid, typisvarlena); + + /* Check for known types */ + switch (typoid) + { + case BOOLOID: + *tcategory = JSONBTYPE_BOOL; + break; + + case INT2OID: + case INT4OID: + case INT8OID: + case FLOAT4OID: + case FLOAT8OID: + case NUMERICOID: + *tcategory = JSONBTYPE_NUMERIC; + break; + + case TIMESTAMPOID: + *tcategory = JSONBTYPE_TIMESTAMP; + break; + + case TIMESTAMPTZOID: + *tcategory = JSONBTYPE_TIMESTAMPTZ; + break; + + case JSONBOID: + *tcategory = JSONBTYPE_JSONB; + break; + + case JSONOID: + *tcategory = JSONBTYPE_JSON; + break; + + default: + /* Check for arrays and composites */ + if (OidIsValid(get_element_type(typoid))) +*tcategory = JSONBTYPE_ARRAY; + else if (type_is_rowtype(typoid)) +*tcategory = JSONBTYPE_COMPOSITE; + else + { +/* It's probably the general case ... */ +
[HACKERS] jsonb generator functions
Here is a patch for the generator and aggregate functions for jsonb that we didn't manage to get done in time for 9.4. They are all equivalents of the similarly names json functions. Included are to_jsonb jsonb_build_object jsonb_build_array jsonb_object jsonb_agg jsonb_object_agg Still to come: documentation. Adding to the next commitfest. cheers andrew diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c index 2fd87fc..6c23b75 100644 --- a/src/backend/utils/adt/jsonb.c +++ b/src/backend/utils/adt/jsonb.c @@ -12,11 +12,20 @@ */ #include postgres.h +#include miscadmin.h +#include access/htup_details.h +#include access/transam.h +#include catalog/pg_cast.h +#include catalog/pg_type.h #include libpq/pqformat.h #include utils/builtins.h +#include utils/datetime.h +#include utils/lsyscache.h #include utils/json.h #include utils/jsonapi.h #include utils/jsonb.h +#include utils/syscache.h +#include utils/typcache.h typedef struct JsonbInState { @@ -24,6 +33,23 @@ typedef struct JsonbInState JsonbValue *res; } JsonbInState; +/* unlike with json categories, we need to treat json and jsonb differently */ +typedef enum /* type categories for datum_to_jsonb */ +{ + JSONBTYPE_NULL,/* null, so we didn't bother to identify */ + JSONBTYPE_BOOL,/* boolean (built-in types only) */ + JSONBTYPE_NUMERIC, /* numeric (ditto) */ + JSONBTYPE_TIMESTAMP, /* we use special formatting for timestamp */ + JSONBTYPE_TIMESTAMPTZ, /* ... and timestamptz */ + JSONBTYPE_JSON,/* JSON */ + JSONBTYPE_JSONB, /* JSONB */ + JSONBTYPE_ARRAY, /* array */ + JSONBTYPE_COMPOSITE, /* composite */ + JSONBTYPE_JSONCAST, /* something with an explicit cast to JSON */ + JSONBTYPE_JSONBCAST, /* something with an explicit cast to JSONB */ + JSONBTYPE_OTHER/* all else */ +} JsonbTypeCategory; + static inline Datum jsonb_from_cstring(char *json, int len); static size_t checkStringLen(size_t len); static void jsonb_in_object_start(void *pstate); @@ -33,6 +59,22 @@ static void jsonb_in_array_end(void *pstate); static void jsonb_in_object_field_start(void *pstate, char *fname, bool isnull); static void jsonb_put_escaped_value(StringInfo out, JsonbValue *scalarVal); static void jsonb_in_scalar(void *pstate, char *token, JsonTokenType tokentype); +static void jsonb_categorize_type(Oid typoid, + JsonbTypeCategory * tcategory, + Oid *outfuncoid); +static void composite_to_jsonb(Datum composite, JsonbInState *result); +static void array_dim_to_jsonb(JsonbInState *result, int dim, int ndims, int *dims, + Datum *vals, bool *nulls, int *valcount, + JsonbTypeCategory tcategory, Oid outfuncoid); +static void array_to_jsonb_internal(Datum array, JsonbInState *result); +static void jsonb_categorize_type(Oid typoid, + JsonbTypeCategory * tcategory, + Oid *outfuncoid); +static void datum_to_jsonb(Datum val, bool is_null, JsonbInState *result, + JsonbTypeCategory tcategory, Oid outfuncoid, + bool key_scalar); +static void add_jsonb(Datum val, bool is_null, JsonbInState *result, + Oid val_type, bool key_scalar); /* * jsonb type input function @@ -462,3 +504,1278 @@ JsonbToCString(StringInfo out, JsonbContainer *in, int estimated_len) return out-data; } + + +/* + * Determine how we want to render values of a given type in datum_to_jsonb. + * + * Given the datatype OID, return its JsonbTypeCategory, as well as the type's + * output function OID. If the returned category is JSONBTYPE_CAST, we + * return the OID of the type-JSON cast function instead. + */ +static void +jsonb_categorize_type(Oid typoid, + JsonbTypeCategory * tcategory, + Oid *outfuncoid) +{ + bool typisvarlena; + + /* Look through any domain */ + typoid = getBaseType(typoid); + + /* We'll usually need to return the type output function */ + getTypeOutputInfo(typoid, outfuncoid, typisvarlena); + + /* Check for known types */ + switch (typoid) + { + case BOOLOID: + *tcategory = JSONBTYPE_BOOL; + break; + + case INT2OID: + case INT4OID: + case INT8OID: + case FLOAT4OID: + case FLOAT8OID: + case NUMERICOID: + *tcategory = JSONBTYPE_NUMERIC; + break; + + case TIMESTAMPOID: + *tcategory = JSONBTYPE_TIMESTAMP; + break; + + case TIMESTAMPTZOID: + *tcategory = JSONBTYPE_TIMESTAMPTZ; + break; + + case JSONBOID: + *tcategory = JSONBTYPE_JSONB; + break; + + case JSONOID: + *tcategory = JSONBTYPE_JSON; + break; + + default: + /* Check for arrays and composites */ + if (OidIsValid(get_element_type(typoid))) +*tcategory = JSONBTYPE_ARRAY; + else if (type_is_rowtype(typoid)) +*tcategory = JSONBTYPE_COMPOSITE; + else + { +/* It's probably the general case ... */ +*tcategory = JSONBTYPE_OTHER; + +/* + * but let's look for a cast to json or jsonb, if it's not + * built-in + */ +if (typoid = FirstNormalObjectId) +{ + HeapTuple tuple; + + tuple =
Re: [HACKERS] jsonb generator functions
On Fri, Sep 26, 2014 at 1:54 PM, Andrew Dunstan and...@dunslane.net wrote: Here is a patch for the generator and aggregate functions for jsonb that we didn't manage to get done in time for 9.4. That's cool, but I hope someone revisits adding a concatenate operator. That's a biggest omission IMHO. I'm not going to have time for that. -- Peter Geoghegan -- 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] jsonb generator functions
On 09/26/2014 05:00 PM, Peter Geoghegan wrote: On Fri, Sep 26, 2014 at 1:54 PM, Andrew Dunstan and...@dunslane.net wrote: Here is a patch for the generator and aggregate functions for jsonb that we didn't manage to get done in time for 9.4. That's cool, but I hope someone revisits adding a concatenate operator. That's a biggest omission IMHO. I'm not going to have time for that. This patch is the work that I have publicly promised to do. Dmitry Dolgov is in fact working on jsonb_concat(), and several other utility functions. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers