Bug: Missing check_stack_depth() in GRAPH_TABLE rewriter

Started by SATYANARAYANA NARLAPURAM20 days ago4 messageshackers
Jump to latest
#1SATYANARAYANA NARLAPURAM
satyanarlapuram@gmail.com

Hi Hackers,

Two recursive functions, generate_setop_from_pathqueries() and
generate_queries_for_path_pattern_recurse() in GRAPH_TABLE rewriter are
missing check_stack_depth() calls. Consider a property graph with 400 edge
tables with a 2-hop pattern produce 160,000 path queries and therefore
160,000 recursion frames in
generate_setop_from_pathqueries(). This can easily exceed the OS stack
limit for some systems and crashes the backend.

I am proposing a stop gap fix of checking stack depth by calling
check_stack_depth for now but in future we may want to reduce the recursion
depth (for example use a balanced tree to reduce depth from O(N) to O(log
N).

Attached a patch for adding the check_stack_depth() check.

Repro:

CREATE TABLE sv (id int PRIMARY KEY);
INSERT INTO sv VALUES (1);

DO $$
BEGIN
FOR i IN 1..400 LOOP
EXECUTE format(
'CREATE TABLE se_%s (id int PRIMARY KEY, src int, dst int)', i);
END LOOP;
END$$;

DO $$
DECLARE
sql text;
edges text := '';
BEGIN
FOR i IN 1..400 LOOP
IF i > 1 THEN edges := edges || ', '; END IF;
edges := edges || format(
'se_%s KEY (id) SOURCE KEY (src) REFERENCES sv (id) '
'DESTINATION KEY (dst) REFERENCES sv (id)', i);
END LOOP;
EXECUTE 'CREATE PROPERTY GRAPH g VERTEX TABLES (sv KEY (id)) '
|| 'EDGE TABLES (' || edges || ')';
END$$;

SELECT * FROM GRAPH_TABLE(g
MATCH (a)-[e1]->(b)-[e2]->(c)
COLUMNS(a.id AS a_id))
LIMIT 1;

Thanks,
Satya

Attachments:

0001-graph-table-check-stack-depth.patchapplication/octet-stream; name=0001-graph-table-check-stack-depth.patchDownload+7-1
#2Ashutosh Bapat
ashutosh.bapat@enterprisedb.com
In reply to: SATYANARAYANA NARLAPURAM (#1)
Re: Bug: Missing check_stack_depth() in GRAPH_TABLE rewriter

On Sat, Apr 11, 2026 at 9:28 AM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

Hi Hackers,

Two recursive functions, generate_setop_from_pathqueries() and
generate_queries_for_path_pattern_recurse() in GRAPH_TABLE rewriter are missing check_stack_depth() calls. Consider a property graph with 400 edge tables with a 2-hop pattern produce 160,000 path queries and therefore 160,000 recursion frames in
generate_setop_from_pathqueries(). This can easily exceed the OS stack limit for some systems and crashes the backend.

I am proposing a stop gap fix of checking stack depth by calling check_stack_depth for now but in future we may want to reduce the recursion depth (for example use a balanced tree to reduce depth from O(N) to O(log N).

Attached a patch for adding the check_stack_depth() check.

Repro:

CREATE TABLE sv (id int PRIMARY KEY);
INSERT INTO sv VALUES (1);

DO $$
BEGIN
FOR i IN 1..400 LOOP
EXECUTE format(
'CREATE TABLE se_%s (id int PRIMARY KEY, src int, dst int)', i);
END LOOP;
END$$;

DO $$
DECLARE
sql text;
edges text := '';
BEGIN
FOR i IN 1..400 LOOP
IF i > 1 THEN edges := edges || ', '; END IF;
edges := edges || format(
'se_%s KEY (id) SOURCE KEY (src) REFERENCES sv (id) '
'DESTINATION KEY (dst) REFERENCES sv (id)', i);
END LOOP;
EXECUTE 'CREATE PROPERTY GRAPH g VERTEX TABLES (sv KEY (id)) '
|| 'EDGE TABLES (' || edges || ')';
END$$;

SELECT * FROM GRAPH_TABLE(g
MATCH (a)-[e1]->(b)-[e2]->(c)
COLUMNS(a.id AS a_id))
LIMIT 1;

Thanks for the report. I could reproduce the segfault on my laptop.
The attached patch fixes it and gives ERROR: stack depth limit
exceeded.

generate_queries_for_path_pattern_recurse() - has to work in a linear
fashion since the elements need to be processed in an order. Each
permutation of elements produces one query. These queries can be
arranged in a balanced tree as you suggest OR when constructing the
setop tree we could generate it in divide-and-conquer manner. However,
the tree will be flattened in the planner anyway (See
flatten_simple_union_all() and pull_up_simple_union_all()). Thus the
final planning will require a deeper stack anyway. The code complexity
doesn't seem to be worth it.

I also looked at a few commits that add check_stack_depth() to see if
we add tests for these scenarios. But I didn't find any. So no tests
added with this commit.

--
Best Wishes,
Ashutosh Bapat

Attachments:

v20260415-0001-Check-for-stack-overflow-when-rewriting-gr.patchtext/x-patch; charset=US-ASCII; name=v20260415-0001-Check-for-stack-overflow-when-rewriting-gr.patchDownload+7-1
#3Peter Eisentraut
peter_e@gmx.net
In reply to: Ashutosh Bapat (#2)
Re: Bug: Missing check_stack_depth() in GRAPH_TABLE rewriter

On 15.04.26 17:07, Ashutosh Bapat wrote:

Thanks for the report. I could reproduce the segfault on my laptop.
The attached patch fixes it and gives ERROR: stack depth limit
exceeded.

generate_queries_for_path_pattern_recurse() - has to work in a linear
fashion since the elements need to be processed in an order. Each
permutation of elements produces one query. These queries can be
arranged in a balanced tree as you suggest OR when constructing the
setop tree we could generate it in divide-and-conquer manner. However,
the tree will be flattened in the planner anyway (See
flatten_simple_union_all() and pull_up_simple_union_all()). Thus the
final planning will require a deeper stack anyway. The code complexity
doesn't seem to be worth it.

I also looked at a few commits that add check_stack_depth() to see if
we add tests for these scenarios. But I didn't find any. So no tests
added with this commit.

committed

(I moved the #include "miscadmin.h" to a more alphabetical position.)

#4Ashutosh Bapat
ashutosh.bapat@enterprisedb.com
In reply to: Peter Eisentraut (#3)
Re: Bug: Missing check_stack_depth() in GRAPH_TABLE rewriter

On Fri, Apr 24, 2026 at 11:54 AM Peter Eisentraut <peter@eisentraut.org> wrote:

On 15.04.26 17:07, Ashutosh Bapat wrote:

Thanks for the report. I could reproduce the segfault on my laptop.
The attached patch fixes it and gives ERROR: stack depth limit
exceeded.

generate_queries_for_path_pattern_recurse() - has to work in a linear
fashion since the elements need to be processed in an order. Each
permutation of elements produces one query. These queries can be
arranged in a balanced tree as you suggest OR when constructing the
setop tree we could generate it in divide-and-conquer manner. However,
the tree will be flattened in the planner anyway (See
flatten_simple_union_all() and pull_up_simple_union_all()). Thus the
final planning will require a deeper stack anyway. The code complexity
doesn't seem to be worth it.

I also looked at a few commits that add check_stack_depth() to see if
we add tests for these scenarios. But I didn't find any. So no tests
added with this commit.

committed

Thanks a lot for committing this and other fixes.

(I moved the #include "miscadmin.h" to a more alphabetical position.)

Didn't notice this. Sorry.

--
Best Wishes,
Ashutosh Bapat