Re: Tightening behaviour for non-immutable behaviour in immutable functions
От | Greg Stark |
---|---|
Тема | Re: Tightening behaviour for non-immutable behaviour in immutable functions |
Дата | |
Msg-id | CAM-w4HNiy8Q+4C2TxnfMsfTwSZ0N0=TWqjcAc2+tdRTL0QMB3Q@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Tightening behaviour for non-immutable behaviour in immutable functions (Greg Stark <stark@mit.edu>) |
Ответы |
Re: Tightening behaviour for non-immutable behaviour in immutable functions
(Greg Stark <stark@mit.edu>)
Re: Tightening behaviour for non-immutable behaviour in immutable functions (Andres Freund <andres@anarazel.de>) |
Список | pgsql-hackers |
On Mon, 13 Jun 2022 at 16:50, Greg Stark <stark@mit.edu> wrote: > > For that matter.... the gotchas are a bit .... convoluted.... > > Perhaps we should automatically fix up the current search patch and > attach it to functions where necessary for users instead of just > whingeing at them.... So I reviewed my own patch and.... it was completely broken... I fixed it to actually check the right variables. I also implemented the other idea above of actually fixing up search_path in proconfig for the user by default. I think this is going to be more practical. The problem with expecting the user to provide search_path is that they probably aren't today so the warnings would be firing for everything... Providing a fixed up search_path for users would give them a smoother upgrade process where we can give a warning only if the search_path is changed substantively which is much less likely. I'm still quite concerned about the performance impact of having search_path on so many functions. It causes a cache flush which could be pretty painful on a frequently called function such as one in an index expression during a data load.... The other issue is that having proconfig set does prevent these functions from being inlined which can be seen in the regression test as seen below. I'm not sure how big a problem this is for users. Inlining is important for many use cases I think. Maybe we can go ahead and inline things even if they have a proconfig if it matches the proconfig on the caller? Or maybe even check if we get the same objects from both search_paths? Of course this patch is still very WIP. Only one or the other function makes sense to keep. And I'm not opposed to having a GUC to enable/disable the enforcement or warnings. And the code itself needs to be cleaned up with parts of it moving to guc.c and/or namespace.c. Example of regression tests noticing that immutable functions with proconfig set become non-inlineable: diff -U3 /home/stark/src/postgresql/src/test/regress/expected/rangefuncs.out /home/stark/src/postgresql/src/test/regress/results/rangefuncs.out --- /home/stark/src/postgresql/src/test/regress/expected/rangefuncs.out 2022-01-17 12:01:54.958628564 -0500 +++ /home/stark/src/postgresql/src/test/regress/results/rangefuncs.out 2022-06-16 02:16:47.589703966 -0400 @@ -1924,14 +1924,14 @@ select * from array_to_set(array['one', 'two']) as t(f1 point,f2 text); ERROR: return type mismatch in function declared to return record DETAIL: Final statement returns integer instead of point at column 1. -CONTEXT: SQL function "array_to_set" during inlining +CONTEXT: SQL function "array_to_set" during startup explain (verbose, costs off) select * from array_to_set(array['one', 'two']) as t(f1 numeric(4,2),f2 text); - QUERY PLAN --------------------------------------------------------------- - Function Scan on pg_catalog.generate_subscripts i - Output: i.i, ('{one,two}'::text[])[i.i] - Function Call: generate_subscripts('{one,two}'::text[], 1) + QUERY PLAN +---------------------------------------------------- + Function Scan on public.array_to_set t + Output: f1, f2 + Function Call: array_to_set('{one,two}'::text[]) (3 rows) create temp table rngfunc(f1 int8, f2 int8); @@ -2064,11 +2064,12 @@ explain (verbose, costs off) select * from testrngfunc(); - QUERY PLAN --------------------------------------------------------- - Result - Output: 7.136178::numeric(35,6), 7.14::numeric(35,2) -(2 rows) + QUERY PLAN +------------------------------------- + Function Scan on public.testrngfunc + Output: f1, f2 + Function Call: testrngfunc() +(3 rows) select * from testrngfunc(); f1 | f2 -- greg
Вложения
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Sajti Zsolt ZoltánДата:
Сообщение: Global variable/memory context for PostgreSQL functions
Следующее
От: Mark DilgerДата:
Сообщение: Re: Modest proposal to extend TableAM API for controlling cluster commands