Skip to content

Commit

Permalink
IDEA: Introduce a WITH based syntax for FOR-style iteration
Browse files Browse the repository at this point in the history
I dislike how `for` looks and feels so much "heavier" than `with`, due
to the `union` keyword and the need to parenthesize the body if it is
a query, and especially if you need to have *nested* `for`s.

So my idea is to introduce `<-` binding, which behaves like a `for`
binding.

So then you can do something like
```
with name <- {'Sully', 'Zhibo', 'Aljaž'}
insert Person { name := name }
```
I modified some tests you can look at to compare.

Is this a bad idea, and I'm just sleep-deprived?
Does anybody but me like this?
(Does anybody have any other ideas about lighter-weight `for`?)
  • Loading branch information
msullivan committed Feb 9, 2024
1 parent 3285fcb commit 563ba7e
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 47 deletions.
1 change: 1 addition & 0 deletions edb/edgeql/ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ class SortExpr(Base):
class AliasedExpr(Base):
alias: str
expr: Expr
monadic: bool = False


class ModuleAliasDecl(Base):
Expand Down
5 changes: 4 additions & 1 deletion edb/edgeql/codegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,10 @@ def _visit_offset_limit(

def visit_AliasedExpr(self, node: qlast.AliasedExpr) -> None:
self.write(ident_to_str(node.alias))
self.write(' := ')
if node.monadic:
self.write(' <- ')
else:
self.write(' := ')
self._block_ws(1)

self.visit(node.expr)
Expand Down
58 changes: 47 additions & 11 deletions edb/edgeql/compiler/stmt.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from typing import *

from collections import defaultdict
import functools
import textwrap

from edb import errors
Expand Down Expand Up @@ -72,17 +73,54 @@
def try_desugar(
expr: qlast.Query, *, ctx: context.ContextLevel
) -> Optional[irast.Set]:
with_desugar = try_with_monad_rewrite(expr, ctx=ctx)
if with_desugar:
return dispatch.compile(with_desugar, ctx=ctx)

new_syntax = desugar_group.try_group_rewrite(expr, aliases=ctx.aliases)
if new_syntax:
return dispatch.compile(new_syntax, ctx=ctx)
return None


@dispatch.compile.register(qlast.SelectQuery)
def compile_SelectQuery(
expr: qlast.SelectQuery, *, ctx: context.ContextLevel) -> irast.Set:
def try_with_monad_rewrite(
expr: qlast.Query, *, ctx: context.ContextLevel
) -> Optional[qlast.Query]:
if not expr.aliases:
return None
for i, alias in enumerate(expr.aliases):
# TODO: Should we make this less needlessly quadratic?
if isinstance(alias, qlast.AliasedExpr) and alias.monadic:
return qlast.ForQuery(
aliases=expr.aliases[:i],
from_desugaring=True,
iterator=alias.expr,
iterator_alias=alias.alias,
result=expr.replace(aliases=expr.aliases[i + 1:]),
)
return None


@dispatch.compile.register
def compile_Query(
expr: qlast.Query, *, ctx: context.ContextLevel) -> irast.Set:
if rewritten := try_desugar(expr, ctx=ctx):
return rewritten
return compile_query(expr, ctx=ctx)


@functools.singledispatch
def compile_query(
node: qlast.Query, *,
ctx: context.ContextLevel
) -> irast.Set:
raise NotImplementedError(
f'no EdgeQL compiler handler for {node.__class__}')


@compile_query.register
def compile_SelectQuery(
expr: qlast.SelectQuery, *, ctx: context.ContextLevel) -> irast.Set:

with ctx.subquery() as sctx:
stmt = irast.SelectStmt()
Expand Down Expand Up @@ -149,11 +187,9 @@ def compile_SelectQuery(
return result


@dispatch.compile.register(qlast.ForQuery)
@compile_query.register
def compile_ForQuery(
qlstmt: qlast.ForQuery, *, ctx: context.ContextLevel) -> irast.Set:
if rewritten := try_desugar(qlstmt, ctx=ctx):
return rewritten

with ctx.subquery() as sctx:
stmt = irast.SelectStmt(context=qlstmt.context)
Expand Down Expand Up @@ -284,7 +320,7 @@ def _make_group_binding(
return binding_set


@dispatch.compile.register(qlast.InternalGroupQuery)
@compile_query.register(qlast.InternalGroupQuery)
def compile_InternalGroupQuery(
expr: qlast.InternalGroupQuery, *, ctx: context.ContextLevel
) -> irast.Set:
Expand Down Expand Up @@ -411,7 +447,7 @@ def compile_InternalGroupQuery(
return result


@dispatch.compile.register(qlast.GroupQuery)
@compile_query.register(qlast.GroupQuery)
def compile_GroupQuery(
expr: qlast.GroupQuery, *, ctx: context.ContextLevel) -> irast.Set:
return dispatch.compile(
Expand All @@ -420,7 +456,7 @@ def compile_GroupQuery(
)


@dispatch.compile.register(qlast.InsertQuery)
@compile_query.register
def compile_InsertQuery(
expr: qlast.InsertQuery, *,
ctx: context.ContextLevel) -> irast.Set:
Expand Down Expand Up @@ -574,7 +610,7 @@ def compile_InsertQuery(
return result


@dispatch.compile.register(qlast.UpdateQuery)
@compile_query.register
def compile_UpdateQuery(
expr: qlast.UpdateQuery, *, ctx: context.ContextLevel) -> irast.Set:

Expand Down Expand Up @@ -681,7 +717,7 @@ def compile_UpdateQuery(
return result


@dispatch.compile.register(qlast.DeleteQuery)
@compile_query.register
def compile_DeleteQuery(
expr: qlast.DeleteQuery, *, ctx: context.ContextLevel) -> irast.Set:

Expand Down
10 changes: 10 additions & 0 deletions edb/edgeql/parser/grammar/expressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ def reduce_Identifier_ASSIGN_Expr(self, *kids):
self.val = qlast.AliasedExpr(alias=kids[0].val, expr=kids[2].val)


class MonadicExpr(Nonterm):
def reduce_Identifier_LANGBRACKET_MINUS_Expr(self, *kids):
self.val = qlast.AliasedExpr(
alias=kids[0].val, expr=kids[3].val, monadic=True)


class OptionallyAliasedExpr(Nonterm):
def reduce_AliasedExpr(self, *kids):
val = kids[0].val
Expand Down Expand Up @@ -389,6 +395,10 @@ def reduce_Identifier_AS_MODULE_ModuleName(self, *kids):
def reduce_AliasedExpr(self, *kids):
pass

@parsing.inline(0)
def reduce_MonadicExpr(self, *kids):
pass


class WithDecl(Nonterm):
@parsing.inline(0)
Expand Down
18 changes: 7 additions & 11 deletions tests/test_edgeql_for.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,17 +340,13 @@ async def test_edgeql_for_in_computable_02b(self):
SELECT User {
select_deck := ((
WITH cards := (
FOR letter IN {'I', 'B'}
UNION (
FOR copy IN {'1', '2'}
UNION (
SELECT User.deck {
name,
letter := letter ++ copy
}
FILTER User.deck.name[0] = letter
)
)
WITH letter <- {'I', 'B'},
copy <- {'1', '2'},
SELECT User.deck {
name,
letter := letter ++ copy
}
FILTER User.deck.name[0] = letter
)
SELECT cards ORDER BY .name THEN .letter
),)
Expand Down
46 changes: 22 additions & 24 deletions tests/test_edgeql_insert.py
Original file line number Diff line number Diff line change
Expand Up @@ -1272,19 +1272,19 @@ async def test_edgeql_insert_policy_cast(self):

async def test_edgeql_insert_for_01(self):
await self.con.execute(r'''
FOR x IN {3, 5, 7, 2}
UNION (INSERT InsertTest {
WITH x <- {3, 5, 7, 2}
INSERT InsertTest {
name := 'insert for 1',
l2 := x,
});
};
FOR Q IN (SELECT InsertTest{foo := 'foo' ++ <str> InsertTest.l2}
FILTER .name = 'insert for 1')
UNION (INSERT InsertTest {
WITH Q <- (SELECT InsertTest{foo := 'foo' ++ <str> InsertTest.l2}
FILTER .name = 'insert for 1')
INSERT InsertTest {
name := 'insert for 1',
l2 := 35 % Q.l2,
l3 := Q.foo,
});
};
''')

await self.assert_query_result(
Expand Down Expand Up @@ -1383,13 +1383,11 @@ async def test_edgeql_insert_for_04(self):
name := 'nested-insert-for',
l2 := 999,
subordinates := (
FOR x IN {('sub1', 'first'), ('sub2', 'second')}
UNION (
INSERT Subordinate {
name := x.0,
@comment := x.1,
}
)
WITH x <- {('sub1', 'first'), ('sub2', 'second')}
INSERT Subordinate {
name := x.0,
@comment := x.1,
}
)
};
''')
Expand Down Expand Up @@ -1417,9 +1415,9 @@ async def test_edgeql_insert_for_04(self):

async def test_edgeql_insert_for_06(self):
res = await self.con.query(r'''
FOR a in {"a", "b"} UNION (
FOR b in {"c", "d"} UNION (
INSERT Note {name := b}));
WITH a <- {"a", "b"},
b <- {"c", "d"},
INSERT Note {name := b};
''')
self.assertEqual(len(res), 4)

Expand All @@ -1433,9 +1431,9 @@ async def test_edgeql_insert_for_06(self):

async def test_edgeql_insert_for_07(self):
res = await self.con.query(r'''
FOR a in {"a", "b"} UNION (
FOR b in {a++"c", a++"d"} UNION (
INSERT Note {name := b}));
WITH a <- {"a", "b"},
b <- {a++"c", a++"d"},
INSERT Note {name := b};
''')
self.assertEqual(len(res), 4)

Expand All @@ -1449,10 +1447,10 @@ async def test_edgeql_insert_for_07(self):

async def test_edgeql_insert_for_08(self):
res = await self.con.query(r'''
FOR a in {"a", "b"} UNION (
FOR b in {"a", "b"} UNION (
FOR c in {a++b++"a", a++b++"b"} UNION (
INSERT Note {name := c})));
WITH a <- {"a", "b"},
b <- {"a", "b"},
c <- {a++b++"a", a++b++"b"},
INSERT Note {name := c};
''')
self.assertEqual(len(res), 8)

Expand Down

0 comments on commit 563ba7e

Please sign in to comment.