Re: [PATCH] Generalized JSON output functions

On Fri, Jul 10, 2015 at 5:16 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
wrote:

>
>> Well, one could call it premature pessimization due to dynamic call
>> overhead.
>>
>> IMO, the fact that json_out_init_context() sets the value callback to
>> json_out_value is an implementation detail, the other parts of code should
>> not rely on. And for the Explain output, there definitely going to be
>> *some* code between context initialization and output callbacks: these are
>> done in a number of different functions.
>>
>
> Again - it is necessary? Postgres still use modular code, not OOP code. I
> can understand the using of this technique, when I need a possibility to
> change behave. But these function are used for printing JSON, not printing
> any others.
>

No, it's not strictly necessary.

For me it's not about procedural- vs. object- style, but rather about being
able to override/extend the behavior consistently. And for that I would
prefer that if I override the value callback in a JSON output context, that
it would be called for every value being printed, not only for some of them.

Thank you for pointing out the case of Explain format, I've totally
overlooked it in my first version. Trying to apply the proposed approach
in the explain printing code led me to reorganize things slightly. I've
added explicit functions for printing keys vs. values, thus no need to
expose that key_scalar param anymore. There are now separate before/after
key and before/after value functions as well, but I believe it makes for a
cleaner code.

The most of the complexity is still in the code that decides whether or not
to put spaces (between the values or for indentation) and newlines at
certain points. Should we decide to unify the style we emit ourselves,
this could be simplified, while still leaving room for great flexibility if
overridden by an extension, for example.

Have a nice weekend.
--
Alex