Skip to content

Commit

Permalink
Fix interaction of implicit limit with explicit OFFSET (#7509)
Browse files Browse the repository at this point in the history
Currently, we will inject a LIMIT at any exposed SELECT, which results
in us applying implicit LIMIT before OFFSET, which is wrong.

Drop the injection inside of SelectStmt compilation and inject the
LIMIT at the top-level instead directly

Fixes #3627.
  • Loading branch information
msullivan authored Jul 2, 2024
1 parent b198c16 commit 62649b7
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 13 deletions.
4 changes: 4 additions & 0 deletions edb/edgeql/compiler/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,10 @@ def compile_ast_to_ir(

ctx = stmtctx_mod.init_context(schema=schema, options=options)

if isinstance(tree, qlast.Expr) and ctx.implicit_limit:
tree = qlast.SelectQuery(result=tree, implicit=True)
tree.limit = qlast.Constant.integer(ctx.implicit_limit)

if not script_info:
script_info = stmtctx_mod.preprocess_script([tree], ctx=ctx)

Expand Down
8 changes: 0 additions & 8 deletions edb/edgeql/compiler/stmt.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,6 @@ def compile_SelectQuery(
)
)

if (
(ctx.expr_exposed or sctx.stmt is ctx.toplevel_stmt)
and ctx.implicit_limit
and expr.limit is None
and not ctx.inhibit_implicit_limit
):
expr.limit = qlast.Constant.integer(ctx.implicit_limit)

stmt.result = compile_result_clause(
expr.result,
view_scls=ctx.view_scls,
Expand Down
5 changes: 0 additions & 5 deletions edb/edgeql/compiler/viewgen.py
Original file line number Diff line number Diff line change
Expand Up @@ -1534,7 +1534,6 @@ def _normalize_view_ptr_expr(

if (
(ctx.expr_exposed or ctx.stmt is ctx.toplevel_stmt)
and not qlexpr.limit
and ctx.implicit_limit
and not base_is_singleton
):
Expand Down Expand Up @@ -1659,10 +1658,6 @@ def _normalize_view_ptr_expr(
if (
(ctx.expr_exposed or ctx.stmt is ctx.toplevel_stmt)
and ctx.implicit_limit
and isinstance(qlexpr, (
qlast.SelectQuery, qlast.DeleteQuery, qlast.ShapeElement
))
and not qlexpr.limit
):
qlexpr = qlast.SelectQuery(result=qlexpr, implicit=True)
qlexpr.limit = qlast.Constant.integer(ctx.implicit_limit)
Expand Down
117 changes: 117 additions & 0 deletions tests/test_edgeql_scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -4177,3 +4177,120 @@ async def test_edgeql_scope_mat_issue_6060(self):
''',
[{"minCost": 1}, {"minCost": 1}, {"minCost": 1}, {"minCost": 2}],
)

async def test_edgeql_scope_implicit_limit_01(self):
# implicit limit should interact correctly with offset
await self.assert_query_result(
r'''
select Card { name } order by .name offset 3
''',
[
{"name": "Dwarf"},
{"name": "Giant eagle"},
{"name": "Giant turtle"},
{"name": "Golem"},
],
implicit_limit=4,
)

await self.assert_query_result(
r'''
with W := Card
select W { name } order by .name offset 3;
''',
[
{"name": "Dwarf"},
{"name": "Giant eagle"},
{"name": "Giant turtle"},
{"name": "Golem"},
],
implicit_limit=4,
)

await self.assert_query_result(
r'''
select Card { name } order by .name offset 3 limit 2
''',
[
{"name": "Dwarf"},
{"name": "Giant eagle"},
],
implicit_limit=4,
)

await self.assert_query_result(
r'''
select User { deck: {name} order by .name offset 3 }
filter .name = 'Carol';
''',
[
{
"deck": [
{"name": "Giant eagle"},
{"name": "Giant turtle"},
{"name": "Golem"},
]
}
],
implicit_limit=3,
)

async def test_edgeql_scope_implicit_limit_02(self):
# explicit limit shouldn't override implicit
await self.assert_query_result(
r'''
select User { deck: {name} order by .name offset 3 limit 100}
filter .name = 'Carol';
''',
[
{
"deck": [
{"name": "Giant eagle"},
{"name": "Giant turtle"},
{"name": "Golem"},
]
}
],
implicit_limit=3,
)

await self.assert_query_result(
r'''
select User { cards := (
select .deck {name} order by .name offset 3 limit 100)}
filter .name = 'Carol';
''',
[
{
"cards": [
{"name": "Giant eagle"},
{"name": "Giant turtle"},
{"name": "Golem"},
]
}
],
implicit_limit=3,
)

await self.assert_query_result(
r'''
select Card { name } order by .name offset 3 limit 100
''',
[
{"name": "Dwarf"},
{"name": "Giant eagle"},
{"name": "Giant turtle"},
{"name": "Golem"},
],
implicit_limit=4,
)

await self.assert_query_result(
r'''
select Card { name } order by .name offset 3 limit 1
''',
[
{"name": "Dwarf"},
],
implicit_limit=4,
)

0 comments on commit 62649b7

Please sign in to comment.