Re: type cache for concat functions

Hi

2017-08-24 19:24 GMT+02:00 Alexander Kuzmenkov <a(dot)kuzmenkov(at)postgrespro(dot)ru>:

> Hi Pavel,
>
> I tried applying your patch, it applies and compiles fine, check and
> checkworld pass.
>
> I ran a simple performance test, select concat(generate_series(1,100000),
> ... [x5 total]) vs select generate_series(1,100000)::text || ... .
> Operator || runs in 60 ms, while unpatched concat takes 90 and patched --
> 55 ms.
>
> About the code:
> * There seems to be an extra tab here:
> FmgrInfo *typcache;
> It's not aligned with the previous declaration with tab size 4.
>
> * You could allocate the cache and store it into the context inside
> rebuildConcatCache. It would return return the cache pointer, and the
> calling code would look cleaner -- just one line. This is a matter of taste
> though.
>
> * The nearby functions use snake_case, so it should be
> rebuild_concat_cache too.
>

all should be fixed.

Regards

Pavel

>
> Overall the patch looks good to me.
>
> --
> Alexander Kuzmenkov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>
>