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)


row_ro_json(x, ignore_null := true)

it is not too much work, but I need a names for parameters



> Rest looks good to me.
> Thanks
> --
> Jeevan B Chalke
> Principal Software Engineer, Product Development
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company

Reply via email to