Storm

Merge lp:~wallyworld/storm/support-recursive-with into lp:~launchpad-committers/storm/lp

  1. support-recursive-with
  2. Merge into lp
Proposed by Ian Booth
Status: Approved
Approved by: Ian Booth
Approved revision: 399
Proposed branch: lp:~wallyworld/storm/support-recursive-with
Merge into: lp:~launchpad-committers/storm/lp
Diff against target: 66 lines (+25/-3)
3 files modified
NEWS (+1/-0)
storm/expr.py (+10/-3)
tests/expr.py (+14/-0)
To merge this branch: bzr merge lp:~wallyworld/storm/support-recursive-with
Reviewer Review Type Date Requested Status
Free Ekanayaka (community) Approve
Launchpad Committers Pending
Review via email: mp+74327@code.launchpad.net

Commit message

Extend WITH expression to support recursion.

Description of the change

This branch extends with WITH expression to support recursion.

== Implementation ==

Add a new recursive attribute to With and change it's compiler to support it.

== Tests ==

There were no tests for With so I added some:
  expr.test_with()
  expr.test_with_recursive()

To post a comment you must log in.
lp:~wallyworld/storm/support-recursive-with updated
397. By Ian Booth

Merge from mainline

Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

Looks good to me with the 2 points below fixed. +1

[1]

+ def __init__(self, name, select, **kwargs):

Any particular reason for not writing this as:

+ def __init__(self, name, select, recursive=False):

?

It feels more explicit to me.

[2]

Please document the new recursive parameter in the class docstring, like:

class With(Expr):
     """Compiles to a simple (name AS expr) used in constructing WITH clauses.

     @param recursive: <description>
     """

and add a note about it in the NEWS file.

review: Approve
Reply
Revision history for this message
Ian Booth (wallyworld) wrote :

Hi

Thank you. I'll definitely make the changes you suggest below; I think
they are all excellent points.

On 08/11/11 21:17, Free Ekanayaka wrote:
> Review: Approve
>
> Looks good to me with the 2 points below fixed. +1
>
> [1]
>
> + def __init__(self, name, select, **kwargs):
>
> Any particular reason for not writing this as:
>
> + def __init__(self, name, select, recursive=False):
>
> ?
>
> It feels more explicit to me.
>
> [2]
>
> Please document the new recursive parameter in the class docstring, like:
>
> class With(Expr):
> """Compiles to a simple (name AS expr) used in constructing WITH clauses.
>
> @param recursive:<description>
> """
>
> and add a note about it in the NEWS file.
>

Reply
lp:~wallyworld/storm/support-recursive-with updated
398. By Ian Booth

Merge from main devel

399. By Ian Booth

Tweaks from code review

Unmerged revisions

399. By Ian Booth

Tweaks from code review

398. By Ian Booth

Merge from main devel

397. By Ian Booth

Merge from mainline

396. By Ian Booth

Extend WITH expression to support recursion

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2011-10-19 09:10:03 +0000
+++ NEWS 2011-11-09 01:59:22 +0000
@@ -3,6 +3,7 @@
33
4Improvements4Improvements
5------------5------------
6- Add support for generating recursive WITH clauses.
67
7Bug fixes8Bug fixes
8---------9---------
910
=== modified file 'storm/expr.py'
--- storm/expr.py 2011-10-19 09:10:03 +0000
+++ storm/expr.py 2011-11-09 01:59:22 +0000
@@ -1515,15 +1515,22 @@
1515from storm.expr import compile, Expr, SQL1515from storm.expr import compile, Expr, SQL
15161516
1517class With(Expr):1517class With(Expr):
1518 """Compiles to a simple (name AS expr) used in constructing WITH clauses."""1518 """Compiles to a simple (name AS expr) used in constructing WITH clauses.
15191519
1520 def __init__(self, name, select):1520 @param recursive: If true, a recursive WITH clause is generated.
1521 """
1522 __slots__ = ("name", "select", "recursive")
1523
1524 def __init__(self, name, select, recursive=False):
1521 self.name = name1525 self.name = name
1522 self.select = select1526 self.select = select
1527 self.recursive = recursive
15231528
1524@compile.when(With)1529@compile.when(With)
1525def compile_with(compile, with_expr, state):1530def compile_with(compile, with_expr, state):
1526 tokens = []1531 tokens = []
1532 if with_expr.recursive:
1533 tokens.append(" RECURSIVE ")
1527 tokens.append(compile(with_expr.name, state, token=True))1534 tokens.append(compile(with_expr.name, state, token=True))
1528 tokens.append(" AS ")1535 tokens.append(" AS ")
1529 tokens.append(compile(with_expr.select, state))1536 tokens.append(compile(with_expr.select, state))
15301537
=== modified file 'tests/expr.py'
--- tests/expr.py 2011-09-14 15:27:52 +0000
+++ tests/expr.py 2011-11-09 01:59:22 +0000
@@ -1839,6 +1839,20 @@
1839 self.assertEquals(select2.context, SELECT)1839 self.assertEquals(select2.context, SELECT)
1840 self.assertEquals(order_by.context, COLUMN_NAME)1840 self.assertEquals(order_by.context, COLUMN_NAME)
18411841
1842 def test_with(self):
1843 expr = With(elem2, Select(elem1))
1844 state = State()
1845 statement = compile(expr, state)
1846 self.assertEquals(statement, "elem2 AS (SELECT elem1)")
1847 self.assertEquals(state.parameters, [])
1848
1849 def test_with_recursive(self):
1850 expr = With(elem2, Select(elem1), recursive=True)
1851 state = State()
1852 statement = compile(expr, state)
1853 self.assertEquals(statement, " RECURSIVE elem2 AS (SELECT elem1)")
1854 self.assertEquals(state.parameters, [])
1855
1842 def test_except(self):1856 def test_except(self):
1843 expr = Except(Func1(), elem2, elem3)1857 expr = Except(Func1(), elem2, elem3)
1844 state = State()1858 state = State()

Subscribers

People subscribed via source and target branches