On 03/21/2017 06:28 PM, Dmitry Dolgov wrote: > > On 21 March 2017 at 03:03, Andrew Dunstan > <andrew.duns...@2ndquadrant.com > <mailto:andrew.duns...@2ndquadrant.com>> wrote: > > > > However, I think it should probably be broken up into a couple of > pieces - > > one for the generic json/jsonb transforms infrastructure (which probably > > needs some more comments) and one for the FTS functions that will > use it. > > Sure, here are two patches with separated functionality and a bit more > commentaries for the transform functions.
I'm not through looking at this. However, here are a few preliminary comments * we might need to rationalize the header locations a bit * iterate_json(b) and transform_json(b) are a bit too generally named. Really what they do is iterate over or transform string values in the json(b). They ignore / preserve the structure, keys, and non-string scalar values in the json(b). A general iterate or transform function would be called in effect with a stream of all the elements in the json, not just scalar strings. * Unless I'm missing something the iterate_json(b)_values return value is ignored. Instead of returning the state it looks to me like it should return nothing and be declared as void instead of void * * transform_jsonb and transform_json are somewhat asymmetrical. The latter should probably return a text* instead of a StringInfo, to be consistent with the former. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers