Skip to content

Commit

Permalink
Don't trigger recursion errors on medium-size set literals (#1785)
Browse files Browse the repository at this point in the history
Currently set literals get compiled into a chain of UNIONs, which
causes a big blowup in the nesting depth of the AST. As a result, we
trip a recursion error on set literals containing 60 one-element tuples,
which is a solidly medium-size input.

Fix this by compiling set literals into a tree of UNIONs instead of a
chain.

Fixes #475 to my satisfaction though others may disagree
  • Loading branch information
msullivan committed Sep 18, 2020
1 parent b90ff5d commit d0a5989
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 8 deletions.
15 changes: 7 additions & 8 deletions edb/edgeql/compiler/expr.py
Expand Up @@ -144,17 +144,16 @@ def compile_Set(
# a set literal is just sugar for a UNION
op = 'UNION'

# Turn it into a tree of UNIONs so we only blow up the nesting
# depth logarithmically.
# TODO: Introduce an N-ary operation that handles the whole thing?
mid = len(elements) // 2
ls, rs = elements[:mid], elements[mid:]
bigunion = qlast.BinOp(
left=elements[0],
right=elements[1],
left=qlast.Set(elements=ls) if len(ls) > 1 else ls[0],
right=qlast.Set(elements=rs) if len(rs) > 1 else rs[0],
op=op
)
for el in elements[2:]:
bigunion = qlast.BinOp(
left=bigunion,
right=el,
op=op
)
return dispatch.compile(bigunion, ctx=ctx)
else:
return setgen.new_empty_set(
Expand Down
18 changes: 18 additions & 0 deletions tests/test_edgeql_select.py
Expand Up @@ -6005,3 +6005,21 @@ async def test_edgeql_select_duplicate_definition_03(self):
todo
}
""")

async def test_edgeql_select_big_set_literal(self):
res = await self.con.query("""
SELECT {
(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,),
(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,),
(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,),
(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,),
(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,),
(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,),
(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,),
(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,),
(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,),
(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,),
};
""")

assert len(res) == 100

0 comments on commit d0a5989

Please sign in to comment.