Hi, On 2017-07-15 11:22:37 -0400, Tom Lane wrote: > The thread drifted off without any resolution, but clearly we need to > do something before 10.0 final.
Advertising
We need to do something, I'm less convinced that it's really v10 specific :/ > "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. Independent of $subject: What are you thinking of here? You want to avoid the ExecBuildProjectionInfo() in more cases - I'm unconvinced that's actually helpful. In my WIP JIT compilation patch the ExecBuildProjectionInfo() ends up being a good bit faster than paths avoiding it. > 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'm less convinced of that, due to the overhead argument. I think there's a couple ways around that however: 1) We could move ExecProcNode() to be callback based. The first time a node is executed a "wrapper" function is called that does the stack and potentially other checks. That also makes ExecProcNode() small enough to be inlined, which ends up being good for jump target prediction. I've done something similar for v11 for expression evaluation, getting rid of EEOP_*_FIRST duplication etc, and it seems to work well. The big disadvantage to that is that it's a bit invasive for v10, and very likely too invasive to backpatch. 2) I think there's some fair argument to be made that ExecInitNode()'s stack-space needs are similar enough to ExecProcNode()'s allowing us to put a check_stack_depth() into the former. That seems like it's required anyway, since in many cases that's going to trigger stack-depth exhaustion first anyway (unless we hit it in parse analysis, which also seems quite common). Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
