2014-09-01 12:33 GMT+02:00 Jeevan Chalke <jeevan.cha...@enterprisedb.com>:
> Hi Pavel, > > Patch does look good to me. And found no issues as such. > > However here are my optional suggestions: > > 1. Frankly, I did not like name of the function > "row_to_json_pretty_choosy". > Something like "row_to_json_pretty_ignore_nulls" seems better to me. > should be - I have no better name > > 2. To use ignore nulls feature, I have to always pass pretty flag. > Which seems weired. > > Since we do support named argument, can we avoid that? > No idea how much difficult it is. If we have a default arguments to this > function then we do not need one and two argument variations for this > function as well. And we can use named argument for omitting the required > one. Just a thought. > it needs a redesign of original implementation, we should to change API to use default values with named parameters but it doesn't help too much (although it can be readable little bit more) instead row_to_json(x, false, true) be row_ro_json(x, ignore_null := true) it is not too much work, but I need a names for parameters Regards Pavel > > Rest looks good to me. > > > Thanks > > -- > Jeevan B Chalke > Principal Software Engineer, Product Development > EnterpriseDB Corporation > The Enterprise PostgreSQL Company > >
