On 2022-08-30 Tu 17:25, Nikita Glukhov wrote:
>
>
>
>>>> Patches 0001-0006:
>>>>
>>>> Yeah, these add the overhead of an extra function call (typin() ->
>>>> typin_opt_error()) in possibly very common paths. Other than
>>>> refactoring *all* places that call typin() to use the new API, the
>>>> only other option seems to be to leave the typin() functions alone and
>>>> duplicate their code in typin_opt_error() versions for all the types
>>>> that this patch cares about. Though maybe, that's not necessarily a
>>>> better compromise than accepting the extra function call overhead.
>>> I think another possibility is to create a static inline function in the
>>> corresponding .c module (say boolin_impl() in bool.c), which is called
>>> by both the opt_error variant as well as the regular one. This would
>>> avoid the duplicate code as well as the added function-call overhead.
>> +1
> I always thought about such internal inline functions, I 've added them in v10.
>
>
A couple of questions about these:
1. Patch 5 changes the API of DecodeDateTime() and DecodeTimeOnly() by
adding an extra parameter bool *error. Would it be better to provide
_opt_error flavors of these?
2. Patch 6 changes jsonb_from_cstring so that it's no longer static
inline. Shouldn't we have a static inline function that can be called
from inside jsonb.c and is called by the extern function?
changing both of these things would be quite trivial and should not hold
anything up.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com