On 15/07/2017 17:22, Tom Lane wrote: > Julien Rouhaud <julien.rouh...@dalibo.com> writes: >> Since b8d7f053c5c2bf2a7e8734fe3327f6a8bc711755 (Andres in Cc), if you >> write queries which result in infinite recursion (or just too many >> nested function calls), execution ends with segfault instead of intended >> exhausted max_stack_depth: > > Yes. We discussed this before the patch went in [1].
Ah, I totally forgot about it, sorry. > I wanted to put > a stack depth check in ExecProcNode(), and still do. Andres demurred, > claiming that that was too much overhead, but didn't really provide a > credible alternative. The thread drifted off without any resolution, > but clearly we need to do something before 10.0 final. > I wanted to add an open item but I see you already did, thanks! >> Please find attached a trivial patch to fix this. I'm not sure >> ExecMakeTableFunctionResult() is the best or only place that needs to >> check the stack depth. > > I don't think that that's adequate at all. > > I experimented with a variant that doesn't go through > ExecMakeTableFunctionResult: > > [...] > This manages not to crash, but I think the reason it does not is purely > accidental: "SELECT so()" has a nontrivial targetlist so we end up running > ExecBuildProjectionInfo on that, meaning that a fresh expression > compilation happens at every nesting depth, and there are > check_stack_depth calls in expression compilation. Surely that's > something we'd try to improve someday. Or it could accidentally get > broken by unrelated changes in the way plpgsql sets up queries to be > executed. > > I still think that we really need to add a check in ExecProcNode(). > Even if there's an argument to be made that every recursion would > somewhere go through ExecMakeTableFunctionResult, very large/complex > queries could result in substantial stack getting chewed up before > we get to that --- and we don't have an infinite amount of stack slop. I was pretty sceptical about checking depth in ExecMakeTableFunctionResult() only vs ExecProcNode(), and this has finished convincing me so I definitely agree. -- Julien Rouhaud http://dalibo.com - http://dalibo.org -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers