From 563ba7e38c417b27e8f8b93619c8b625e63b3e01 Mon Sep 17 00:00:00 2001 From: "Michael J. Sullivan" Date: Thu, 8 Feb 2024 21:41:11 -0800 Subject: [PATCH] IDEA: Introduce a WITH based syntax for FOR-style iteration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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`?) --- edb/edgeql/ast.py | 1 + edb/edgeql/codegen.py | 5 +- edb/edgeql/compiler/stmt.py | 58 +++++++++++++++++++----- edb/edgeql/parser/grammar/expressions.py | 10 ++++ tests/test_edgeql_for.py | 18 +++----- tests/test_edgeql_insert.py | 46 +++++++++---------- 6 files changed, 91 insertions(+), 47 deletions(-) diff --git a/edb/edgeql/ast.py b/edb/edgeql/ast.py index a936f104d59..db78d1db7c0 100644 --- a/edb/edgeql/ast.py +++ b/edb/edgeql/ast.py @@ -155,6 +155,7 @@ class SortExpr(Base): class AliasedExpr(Base): alias: str expr: Expr + monadic: bool = False class ModuleAliasDecl(Base): diff --git a/edb/edgeql/codegen.py b/edb/edgeql/codegen.py index 906bcdd2169..1a603ea2b84 100644 --- a/edb/edgeql/codegen.py +++ b/edb/edgeql/codegen.py @@ -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) diff --git a/edb/edgeql/compiler/stmt.py b/edb/edgeql/compiler/stmt.py index 827c6cb0925..f3c14d8ee30 100644 --- a/edb/edgeql/compiler/stmt.py +++ b/edb/edgeql/compiler/stmt.py @@ -24,6 +24,7 @@ from typing import * from collections import defaultdict +import functools import textwrap from edb import errors @@ -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() @@ -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) @@ -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: @@ -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( @@ -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: @@ -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: @@ -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: diff --git a/edb/edgeql/parser/grammar/expressions.py b/edb/edgeql/parser/grammar/expressions.py index 4e1a7923fd0..e698471c104 100644 --- a/edb/edgeql/parser/grammar/expressions.py +++ b/edb/edgeql/parser/grammar/expressions.py @@ -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 @@ -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) diff --git a/tests/test_edgeql_for.py b/tests/test_edgeql_for.py index f376d96424c..ec0d80f2ca8 100644 --- a/tests/test_edgeql_for.py +++ b/tests/test_edgeql_for.py @@ -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 ),) diff --git a/tests/test_edgeql_insert.py b/tests/test_edgeql_insert.py index 45ed2e186e6..d9dd7f76ef4 100644 --- a/tests/test_edgeql_insert.py +++ b/tests/test_edgeql_insert.py @@ -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' ++ InsertTest.l2} - FILTER .name = 'insert for 1') - UNION (INSERT InsertTest { + WITH Q <- (SELECT InsertTest{foo := 'foo' ++ 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( @@ -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, + } ) }; ''') @@ -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) @@ -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) @@ -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)