On 2023-Oct-12, Alexander Lakhin wrote: Hello,
> I've discovered that that commit added several recursive functions, and > some of them are not protected from stack overflow. True. I reproduced the first two, but didn't attempt to reproduce the third one -- patching all these to check for stack depth is cheap protection. I also patched ATAddCheckNNConstraint: > (ATAddCheckNNConstraint() is protected because it calls > AddRelationNewConstraints(), which in turn calls StoreRelCheck() -> > CreateConstraintEntry() -> recordDependencyOnSingleRelExpr() -> > find_expr_references_walker() -> expression_tree_walker() -> > expression_tree_walker() -> check_stack_depth().) because it seems uselessly risky to rely on depth checks that exist on completely unrelated pieces of code, when the function visibly recurses on itself. Especially so since the test cases that demonstrate crashes are so expensive to run, which means we're not going to detect it if at some point that other stack depth check stops being called for whatever reason. BTW probably the tests could be made much cheaper by running the server with a lower "ulimit -s" setting. I didn't try. I noticed one more crash while trying to "drop table" one of the hierarchies your scripts create. But it's a preexisting issue which needs a backpatched fix, and I think Egor already reported it in the other thread. Thank you -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "Industry suffers from the managerial dogma that for the sake of stability and continuity, the company should be independent of the competence of individual employees." (E. Dijkstra)
