Hello 2013/1/19 Tom Lane <t...@sss.pgh.pa.us>: > Pavel Stehule <pavel.steh...@gmail.com> writes: >> 2013/1/18 Tom Lane <t...@sss.pgh.pa.us>: >>> The approach is also inherently seriously inefficient. ... > >> What is important - for this use case - there is simple and perfect >> possible optimization - in this case "non variadic manner call of >> variadic "any" function" all variadic parameters will share same type. >> Inside variadic function I have not information so this situation is >> in this moment, but just I can remember last used type - and I can >> reuse it, when parameter type is same like previous parameter. > >> So there no performance problem. > > Well, if we have to hack each variadic function to make it work well in > this scenario, that greatly weakens the major argument for the proposed > patch, namely that it provides a single-point fix for VARIADIC behavior. > > BTW, I experimented with lobotomizing array_in's caching of I/O function > lookup behavior, by deleting the if-test at arrayfuncs.c line 184. That > seemed to make it about 30% slower for a simple test involving > converting two-element float8 arrays. So while failing to cache this > stuff isn't the end of the world, arguing that it's not worth worrying > about is simply wrong. > >>> But large arrays have a worse problem: the approach flat out does >>> not work for arrays of more than FUNC_MAX_ARGS elements, because >>> there's no place to put their values in the FunctionCallInfo struct. >>> This last problem is, so far as I can see, unfixable within this >>> approach; surely we are not going to accept a feature that only works >>> for small arrays. So I am going to mark the CF item rejected not just >>> RWF. > >> disagree - non variadic manner call should not be used for walk around >> FUNC_MAX_ARGS limit. So there should not be passed big array. > > That's utter nonsense. Why wouldn't people expect concat(), for > example, to work for large (or even just moderate-sized) arrays? > > This problem *is* a show stopper for this patch. I suggested a way you > can fix it without having such a limitation. If you don't want to go > that way, well, it's not going to happen. > > I agree the prospect that each variadic-ANY function would have to deal > with this case for itself is a tad annoying. But there are only two of > them in the existing system, and it's not like a variadic-ANY function > isn't a pretty complicated beast anyway. > >> You propose now something, what you rejected four months ago. > > Well, at the time it wasn't apparent that this approach wouldn't work. > It is now, though.
so here is rewritten patch * there are no limit for array size that holds variadic arguments * iteration over mentioned array is moved to variadic function implementation * there is a new function get_fn_expr_arg_variadic, that returns true, when last argument has label VARIADIC - via FuncExpr node * fix all our variadic "any" functions - concat(), concat_ws() and format() * there is a small optimization - remember last used typOutput function and reuse it when possible * it doesn't change any previous test or documented behave I hope so almost all issues and questions are solved and there are agreement on implemented behave. Regards Pavel > > regards, tom lane
variadic_any_fix_20130121.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
