Hi Amit, On Mon, Jul 8, 2019 at 10:52 AM Amit Langote <amitlangot...@gmail.com> wrote: > > On Thu, Jul 4, 2019 at 6:52 PM Julien Rouhaud <rjuju...@gmail.com> wrote: > > On Tue, May 28, 2019 at 6:57 AM Amit Langote wrote: > > > >> Maybe like the attached? I'm not sure if we need to likewise be > > > >> concerned > > > >> about exec_sql_string() being handed multi-query strings. > > > > the whole extension sql script is passed to execute_sql_string(), so I > > think that it's a good thing to have similar workaround there. > > That makes sense, although it is perhaps much less likely for memory > usage explosion to occur in execute_sql_strings(), because the scripts > passed to execute_sql_strings() mostly contain utility statements and > rarely anything whose planning will explode in memory usage. > > Anyway, I've added similar handling in execute_sql_strings() for consistency.
Thanks! > Now I wonder if we'll need to consider another path which calls > pg_plan_queries() on a possibly multi-statement query -- > BuildCachedPlan()... I also thought about this when reviewing the patch, but AFAICS you can't provide a multi-statement query to BuildCachedPlan() using prepared statements and I'm not sure that SPI is worth the trouble. I'll mark this patch as ready for committer. > > > About the patch: > > > > - * Switch to appropriate context for constructing querytrees > > (again, > > - * these must outlive the execution context). > > + * Switch to appropriate context for constructing querytrees. > > + * Memory allocated during this construction is released before > > + * the generated plan is executed. > > > > The comment should mention query and plan trees, everything else seems ok > > to me. > > Okay, fixed. > > Attached updated patch. Thanks again. > > Regards, > Amit
