Merge lp:~lifeless/storm/with into lp:storm

Proposed by Robert Collins
Status: Work in progress
Proposed branch: lp:~lifeless/storm/with
Merge into: lp:storm
Diff against target: 269 lines (+127/-12)
4 files modified
storm/expr.py (+30/-3)
storm/store.py (+62/-8)
test (+1/-1)
tests/store/postgres.py (+34/-0)
To merge this branch: bzr merge lp:~lifeless/storm/with
Reviewer Review Type Date Requested Status
Gustavo Niemeyer Needs Fixing
Review via email: mp+52630@code.launchpad.net

Description of the change

Implement WITH support - enough for Launchpads needs.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

[1]

199 + result = self.store.with_(SQL("tmp as (select 20 as id)")).using(Foo
200 + ).find(Foo.title, SQL("foo.id in (select id from tmp)"));
201 + self.assertEqual(list(result), [u'Title 20'])

We'll have to think a bit more about the interface here. The proposed one seems
hard to reason about and will certainly yield confusion regarding the distinction
between with_() and using(), which are undistinguishable from a developer
standpoint.

review: Needs Fixing
Revision history for this message
Robert Collins (lifeless) wrote :

On Thu, Mar 10, 2011 at 3:16 AM, Gustavo Niemeyer <email address hidden> wrote:
> Review: Needs Fixing
> [1]
>
> 199     +        result = self.store.with_(SQL("tmp as (select 20 as id)")).using(Foo
> 200     +            ).find(Foo.title, SQL("foo.id in (select id from tmp)"));
> 201     +        self.assertEqual(list(result), [u'Title 20'])
>
> We'll have to think a bit more about the interface here.  The proposed one seems
> hard to reason about and will certainly yield confusion regarding the distinction
> between with_() and using(), which are undistinguishable from a developer
> standpoint.

we could rename using to from_ (another keyword :( ...) or perhaps
store.use_with and store.use_from or something like that.

If this patch will break any existing storm features, just let me know
and I'll fix that as a priority. I'm also happy to do modest changes
to make it fit better into storm.

We're going to run a fork in LP for the moment - we need this feature
to solve performance problems.

Cheers,
Rob

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

[1]

> we could rename using to from_ (another keyword :( ...) or perhaps
> store.use_with and store.use_from or something like that.

Doesn't seem significantly better.

The WITH clause seems somewhat similar in essence to an alias, except
the definition of the alias is more than a simple rename. This means
that pretty much every location in Storm which could accept a table
can theoretically be defined with the help of a WITH clause.

I'm wondering if we might come up with a "With" type that could
optionally be defined name-less (similar to how Alias works), and be
used anywhere a real table can be.

> We're going to run a fork in LP for the moment - we need this feature
> to solve performance problems.

Please don't. We know what will happen next.

Launchpad has survived for years without this feature, so please
wait a bit longer and let's actually sort out the right way to do it.

You'll not want to go around patching code once we determine the way
that is going to be integrated, and will effectively be living with a
fork forever.

Revision history for this message
Robert Collins (lifeless) wrote :

On Fri, Mar 11, 2011 at 2:16 AM, Gustavo Niemeyer <email address hidden> wrote:
> [1]
>
>> we could rename using to from_ (another keyword :( ...) or perhaps
>> store.use_with and store.use_from or something like that.
>
> Doesn't seem significantly better.
>
> The WITH clause seems somewhat similar in essence to an alias, except
> the definition of the alias is more than a simple rename.  This means
> that pretty much every location in Storm which could accept a table
> can theoretically be defined with the help of a WITH clause.
>
> I'm wondering if we might come up with a "With" type that could
> optionally be defined name-less (similar to how Alias works), and be
> used anywhere a real table can be.

That would be beautiful - its outside my understanding of the plumbing
- the compiler state in particular - to make that work properly, but I
can certainly add a couple of (possibly) failing tests.

>> We're going to run a fork in LP for the moment - we need this feature
>> to solve performance problems.
>
> Please don't.  We know what will happen next.
>
> Launchpad has survived for years without this feature, so please
> wait a bit longer and let's actually sort out the right way to do it.

Launchpad has been terribly slow for years, for a number of reasons,
but one of the key things in fixing the performance issues has been
accepting that we need to solve the problem using the least-wrong
approach that is ready to hand, and *then* refactor to make the
solution have our ideal characteristics. Using this approach we've
unblocked years of inertia and made substantial performance gains for
Launchpad.

> You'll not want to go around patching code once we determine the way
> that is going to be integrated, and will effectively be living with a
> fork forever.

We'll have a couple of callsites at most, and it should be straight
forward to update them. Launchpad has run storm versions that vary
from trunk before and converged on trunk again.

In point of fact, the datetime change that is in trunk revision 386
will be more work for us to update to than a better API for doing
with.

-Rob

Revision history for this message
Robert Collins (lifeless) wrote :

This seems stalled. I'm told its not good enough, but I haven't been told what will be good enough, nor suggestions on how to get there.

I think this is an improvement over nothing, and it would be good to merge this with any *correctness* fixes needed, mark it as experimental in the API docs so folk don't depend on it, and then folk interested can iterate on making it really better.

Revision history for this message
Thomas Herve (therve) wrote :

Robert: can you give us the current code examples you have in Launchpad? It would help (me at least) to define what a good API would be.

I'm not particularly against the current branch, but it seems rather a small improvement over using store.execute directly. I think we can do better.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

You have been given concrete comments and suggestions, Robert. It's stalled because you
haven't pursued the review comments you got further, and simply stated "its outside my
understanding of the plumbing".

Revision history for this message
James Henstridge (jamesh) wrote :

Here's my thoughts on what Gustavo is suggesting:

1. The user should be able to create an object that acts similar to a table class to use the feature. Something like:

   WithTable = ???
   result = store.find(Foo, Foo.bar == WithTable.bar, WithTable.baz = 42)
   print list(result)

The WithTable object would compile similar to a normal table, but note that it had been used in the state object. The compile function for Select() could then generate the WITH clause if any such tables have been referenced by the query.

Generating the bulk of the with clause could be delegated to the WithTable's compile routine by calling it with a different context.

2. How do we create a WithTable? It would need the following information:
 * an alias for this temporary table.
 * a list of the column names. Do we also want the column types?
 * a query object that generates the temporary table.

What would be the most convenient way to collect this together? Perhaps something similar to ResultSet.get_select() would be convenient: it takes a column list we might be able to get column names from and produces a query object, so all it needs is a name.

3. Recursive queries could easily be handled internally to the WithTable in this model.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

> 1. The user should be able to create an object that acts
> similar to a table class to use the feature. Something like:
(...)
> The WithTable object would compile similar to a normal table,
> but note that it had been used in the state object. The compile
> function for Select() could then generate the WITH clause if any
> such tables have been referenced by the query.

Thanks James. That's exactly right.

In your example, WithTable could be defined as something like this:

    table = With("...definition...")
    WithTable = ClassAlias(cls, table)

also, normal classes could be defined with:

    table = With("...definition...")
    class Person(object):
        __storm_table__ = table

and could be used in finds in an equivalent way:

    table = With("...definition...")
    store.using(table).find(...)

> 2. How do we create a WithTable? It would need the following information:
(...)
> * a list of the column names. Do we also want the column types?

I don't think we need the column names, in the same way we don't have the
column names for normal tables. As long as it behaves like a table, we can
use the standard machinery to handle it.

Revision history for this message
James Henstridge (jamesh) wrote :

I guess I was misreading the syntax of the with_query, so column names are not required. For reference, here is the syntax:

  with_query_name [ ( column_name [, ...] ) ] AS ( select | insert | update | delete )

While the column list isn't required, it seems more robust to list the column names for cases where you would have duplicate names otherwise. I also don't think we'd make life any easier by automatically aliasing the columns to something like _1, _2, etc.

As far as the actual syntax goes, I'd think it common for users to use different variations on the query on each use (Robert can probably verify whether this is correct), so your second suggestion of tying the query to a class defined using standard class syntax seems a bit verbose. Something where you could link the query to an existing template class would seem more natural.

One other question Robert might have some input on: is it actually beneficial to use a full table class to represent the "with" table? They may not have a well defined primary key, and the table will be gone once the query completes, so it isn't clear you'd want to clutter the object cache with rows from the table. And are there many cases where you'd want to return objects from a "with" table from a Store.find() call, or are they just for use in the WHERE clause?

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

> While the column list isn't required, it seems more robust to list the column names
> for cases where you would have duplicate names otherwise. I also don't think we'd
> make life any easier by automatically aliasing the columns to something like _1, _2, etc.

Sounds good.

Please note that I wasn't suggesting using aliases. We might simply not get into the
business of defining the query in detail, in a way similar to how we don't detail
how the CREATE TABLE is run with its column names.

But having a With type which is closer to the other types (Select, Update, etc) sounds
good too.

Revision history for this message
Robert Collins (lifeless) wrote :

@therve - uses of with_ so far:

http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/view/head:/lib/lp/bugs/model/bugtask.py#L1925

http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/view/head:/lib/lp/code/model/branchmergeproposal.py#L650

http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/view/head:/lib/lp/blueprints/model/specification.py#L636

http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/view/head:/lib/lp/translations/model/potmsgset.py#L449

re: I'm not particularly against the current branch, but it seems rather a small improvement over using store.execute directly. I think we can do better

The big advantage is that for the returned data we can use class expressions to get an entire class back: the filter constrains are easy to write as literal SQL; getting Storm to cast the result to objects requires using the higher level API, doesn't it?

As you can see, its a little manual at the moment, and I do like the idea of being able to define queries for more complex cases.

Revision history for this message
Robert Collins (lifeless) wrote :
Download full text (3.5 KiB)

@jamesh
>2. How do we create a WithTable? It would need the following information:
> * an alias for this temporary table.
> * a list of the column names. Do we also want the column types?
> * a query object that generates the temporary table.

there are two ways we've used WITH so far:
 * to create a constraint - e.g. we calculate all the source package names published in a distroseries in ~300ms, and then we can use that as a constraint - with joining or doing a subquery on it. (this is the bugtask use of with). The performance win here is that we know a-priori that determining package names ad-hoc is slow - *most* packagenames have been published in ubuntu always, so cheaper to do a bulk lookup.

 * to get an actual table - see the (complex) use in branchmergeproposal: we have two aliases to BranchRevision, and are doing a DAG difference between the two aliases to find new revisions in the source; the 'source' with clause *is* the origin for the BranchRevision objects we want to return. the key points here, for me is that we want to populate BranchRevision objects in the cache, not some temporary class.

I don't know if this impacts what is needed to create a WithTable.

I haven't had cause to define column names for the temporary tables yet - one will want to minimise the data in the temp table anyway - a fat temp table isn't going to be fast.

> As far as the actual syntax goes, I'd think it common for users to use different variations on > the query on each use (Robert can probably verify whether this is correct), so your second
> suggestion of tying the query to a class defined using standard class syntax seems a bit
> verbose. Something where you could link the query to an existing template class would seem
> more natural.

If I understand this correctly, then yes - see the branchmergeproposal case above.

> One other question Robert might have some input on: is it actually beneficial to use a full
> table class to represent the "with" table? They may not have a well defined primary key, and
> the table will be gone once the query completes, so it isn't clear you'd want to clutter the
> object cache with rows from the table. And are there many cases where you'd want to return
> objects from a "with" table from a Store.find() call, or are they just for use in the WHERE
> clause?

We've only 4 data points so far, but I think we'll want the objects from the with table maybe 10-25% of the time: a significant but not overwhelming amount. Most of the time it will be just a constraint or graph traversal that is best done in a very narrow fashion.

@gustavo
> Please note that I wasn't suggesting using aliases. We might simply not get into the
> business of defining the query in detail, in a way similar to how we don't detail
> how the CREATE TABLE is run with its column names.

It would be nice to use query objects to define a query in a with - I really like the sound of that.

I don't think we'd want the example you have of a full Person table defined on top of a WITH clause: unless it loaded into the storm cache in the same place our regular Person table does (which IIRC would raise errors?).

@all
I'm still at a loss on how to...

Read more...

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

> I don't think we'd want the example you have of a full Person table defined on
> top of a WITH clause: unless it loaded into the storm cache in the same place
> our regular Person table does (which IIRC would raise errors?).

My point is that whether you find this useful right now or not, it will just work,
because it's just another way to define a table.

> @all
> I'm still at a loss on how to code this up - it sounds nice but I need
> pointers: I got as far as I did with this on blind luck following the path of
> least resistance. I'm not asking for the code to be written for me, but I need
> to know where existing similar tests are that I can copy; how the compiler
> works so I can tell how to change it - or at least some hints pointing me in
> the appropriate directions...

It's hard to help you with plain "I don't get it. What do I do?" questions
after we provide you some guidance. We need more specific questions from
you taking into account what we already said, otherwise we'll end up just
saying the same thing again.

We can also have a voice call at some point to have a more concrete
implementation conversation, if you're finding hard to understand the
guidance we're providing and would find better to chat over it.

Revision history for this message
James Henstridge (jamesh) wrote :

@niemeyer
> Please note that I wasn't suggesting using aliases. We might simply not get
> into the business of defining the query in detail, in a way similar to how
> we don't detail how the CREATE TABLE is run with its column names.

While we don't really need to care about ordering or uniqueness of names for table definitions, I'm not sure that carries over to this feature. At the most basic level the WITH query will generate tuples of values, and we need names for those values in the outer query.

While we can rely on default naming in some cases, in others it is less clear. Take the following, for instance:

WITH company_employee AS (
  SELECT company.name, employee.name FROM company, employee WHERE company.id = employee.company_id)
SELECT company_employee.name FROM company_employee

What will company_employee.name refer to? What is the name of the other column? That's why I think it would be more robust to always give a column name list.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

@james

Agreed. If we are getting into the business of defining a With statement such as:

    With((a, b, c), ...)

Rather than

    With("<plain SQL>")

It'd be better to have explicit column names.

Revision history for this message
Robert Collins (lifeless) wrote :

On Wed, Mar 23, 2011 at 11:24 AM, Gustavo Niemeyer <email address hidden> wrote:
>
>> I don't think we'd want the example you have of a full Person table defined on
>> top of a WITH clause: unless it loaded into the storm cache in the same place
>> our regular Person table does (which IIRC would raise errors?).
>
> My point is that whether you find this useful right now or not, it will just work,
> because it's just another way to define a table.

Ok

>> @all
>> I'm still at a loss on how to code this up - it sounds nice but I need
>> pointers: I got as far as I did with this on blind luck following the path of
>> least resistance. I'm not asking for the code to be written for me, but I need
>> to know where existing similar tests are that I can copy; how the compiler
>> works so I can tell how to change it - or at least some hints pointing me in
>> the appropriate directions...
>
> It's hard to help you with plain "I don't get it. What do I do?" questions
> after we provide you some guidance.  We need more specific questions from
> you taking into account what we already said, otherwise we'll end up just
> saying the same thing again.

I don't mean to whinge, but AFAICT I haven't had any guidance : no
code pointers, design or structure pointers, other than 'something
like ClassAlias'. As a specific example; you don't like 'with_', but I
can't see any way to say to the compiler that a custom type should
compile to the front of the query - before the
SELECT/UPDATE/INSERT/DELETE.

> We can also have a voice call at some point to have a more concrete
> implementation conversation, if you're finding hard to understand the
> guidance we're providing and would find better to chat over it.

That could be very helpful - thanks. I'm timeshifted towards UK at the
moment while we work through a backlog of RT tickets impacting the LP
deployment environment, but I should have some overlap with you in
your afternoons, or in a week or so I'll be back to more NZ normal
time and have better overlap.

I've pushed up a sketch that may be what you meant, and toggled this
back to needs review.

Cheers,
Rob

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

> I don't mean to whinge, but AFAICT I haven't had any guidance : no
> code pointers, design or structure pointers, other than 'something

There is a lot of information above which you've consistently
ignored. If you need details, ask for them please, and we'll
be very happy to assist.

> like ClassAlias'. As a specific example; you don't like 'with_', but I
> can't see any way to say to the compiler that a custom type should
> compile to the front of the query - before the
> SELECT/UPDATE/INSERT/DELETE.

James already answered that above:

"""
The WithTable object would compile similar to a normal table, but
note that it had been used in the state object. The compile
function for Select() could then generate the WITH clause if any
such tables have been referenced by the query.
"""

Note that tables are *already* compiled out of order in the query, so
it would be something along those lines:

1) Add with_tables to the State type, as James said
2) Make With() statements *return* the table name only (what is
   seen before a reference in a query, like "foo" so that
   "foo.column" works) during compilation
3) Make With() statements inject the WITH fragment within with_tables
   of the state during compilation
4) Make Select push and pop with_tables (see how auto_tables is handled
   there), and before popping with_tables, generate a WITH prefix in
   case it's non-empty, as James said.

> That could be very helpful - thanks. I'm timeshifted towards UK at the
> moment while we work through a backlog of RT tickets impacting the LP

I'm in Brazil, so -3 ATM.. feel free to book a time which is suitable for
you and within my awaken time (fine to be off hours).

> I've pushed up a sketch that may be what you meant, and toggled this
> back to needs review.

Ok, this is still not going in the right direction, but the above hints
might help you. Let me know if you have more questions.

review: Needs Fixing

Unmerged revisions

390. By Robert Collins

Add a compiler class to let select expressions be reused by with_.

389. By Robert Collins

And __contains__ needs special handling.

388. By Robert Collins

Oops, missed count() support.

387. By Robert Collins

Add limited WITH support to storm.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'storm/expr.py'
2--- storm/expr.py 2011-03-15 07:54:00 +0000
3+++ storm/expr.py 2011-03-28 00:10:52 +0000
4@@ -636,12 +636,13 @@
5
6 class Select(Expr):
7 __slots__ = ("columns", "where", "tables", "default_tables", "order_by",
8- "group_by", "limit", "offset", "distinct", "having")
9+ "group_by", "limit", "offset", "distinct", "having", "with_")
10
11 def __init__(self, columns, where=Undef,
12 tables=Undef, default_tables=Undef,
13 order_by=Undef, group_by=Undef,
14- limit=Undef, offset=Undef, distinct=False, having=Undef):
15+ limit=Undef, offset=Undef, distinct=False, having=Undef,
16+ with_=Undef):
17 self.columns = columns
18 self.where = where
19 self.tables = tables
20@@ -652,10 +653,15 @@
21 self.offset = offset
22 self.distinct = distinct
23 self.having = having
24+ self.with_ = with_
25
26 @compile.when(Select)
27 def compile_select(compile, select, state):
28- tokens = ["SELECT "]
29+ tokens = []
30+ if select.with_ is not Undef:
31+ tokens.append("WITH ")
32+ tokens.append(compile(select.with_, state))
33+ tokens.append("SELECT ")
34 if select.distinct:
35 tokens.append("DISTINCT ")
36 state.push("auto_tables", [])
37@@ -1481,6 +1487,27 @@
38
39
40 # --------------------------------------------------------------------
41+# WITH clause
42+
43+from storm.expr import compile, Expr, SQL
44+
45+class With(Expr):
46+ """Compiles to a simple (name AS expr) used in constructing WITH clauses."""
47+
48+ def __init__(self, name, select):
49+ self.name = name
50+ self.select = select
51+
52+@compile.when(With)
53+def compile_with(compile, with_expr, state):
54+ tokens = []
55+ tokens.append(compile(with_expr.name, state, token=True))
56+ tokens.append(" AS ")
57+ tokens.append(compile(with_expr.select, state))
58+ return "".join(tokens)
59+
60+
61+# --------------------------------------------------------------------
62 # Set operator precedences.
63
64 compile.set_precedence(10, Select, Insert, Update, Delete)
65
66=== modified file 'storm/store.py'
67--- storm/store.py 2011-01-05 10:19:49 +0000
68+++ storm/store.py 2011-03-28 00:10:52 +0000
69@@ -232,6 +232,25 @@
70 """
71 return self._table_set(self, tables)
72
73+ def with_(self, clause):
74+ """Specify a with clause for use with the query.
75+
76+ For instance::
77+
78+ store.with_(SQL("tmpname as (SELECT ...)")).find(Person,
79+ SQL("Person.id in (SELECT id from tmpname)"))
80+
81+ will compile to
82+
83+ WITH tmpname as (SELECT ...)
84+ SELECT Person.id FROM Person WHERE
85+ PERSON.id in (SELECT id from tmpname);
86+
87+ @return: A L{WithSet}, which has C{find} and C{using} methods
88+ similar to L{Store.find} and L{Store.using}.
89+ """
90+ return self._with_set(self, clause)
91+
92 def add(self, obj):
93 """Add the given object to the store.
94
95@@ -367,7 +386,6 @@
96 # to test it without whitebox.
97 self._order.clear()
98
99-
100 def _mark_autoreload(self, obj=None, invalidate=False):
101 if obj is None:
102 obj_infos = self._iter_alive()
103@@ -908,12 +926,13 @@
104 """
105
106 def __init__(self, store, find_spec,
107- where=Undef, tables=Undef, select=Undef):
108+ where=Undef, tables=Undef, select=Undef, with_=Undef):
109 self._store = store
110 self._find_spec = find_spec
111 self._where = where
112 self._tables = tables
113 self._select = select
114+ self._with = with_
115 self._order_by = find_spec.default_order
116 self._offset = Undef
117 self._limit = Undef
118@@ -966,7 +985,7 @@
119 return Select(columns, self._where, self._tables, default_tables,
120 self._order_by, offset=self._offset, limit=self._limit,
121 distinct=self._distinct, group_by=self._group_by,
122- having=self._having)
123+ having=self._having, with_=self._with)
124
125 def _load_objects(self, result, values):
126 return self._find_spec.load_objects(self._store, result, values)
127@@ -1036,7 +1055,7 @@
128 if self._where is not Undef:
129 where.append(self._where)
130 select = Select(1, And(*where), self._tables,
131- default_tables)
132+ default_tables, with_=self._with)
133 else:
134 # Rewrite the predefined query and use it as a subquery.
135 aliased_columns = [Alias(column, "_key%d" % index)
136@@ -1219,7 +1238,7 @@
137 if (self._select is Undef and not self._distinct and
138 self._offset is Undef and self._limit is Undef):
139 select = Select(aggregate_func(expr), self._where,
140- self._tables, default_tables)
141+ self._tables, default_tables, with_=self._with)
142 else:
143 if expr is Undef:
144 aggregate = aggregate_func(expr)
145@@ -1618,9 +1637,10 @@
146 This will typically be constructed by a call to L{Store.using}.
147 """
148
149- def __init__(self, store, tables):
150+ def __init__(self, store, tables, with_=Undef):
151 self._store = store
152 self._tables = tables
153+ self._with = with_
154
155 def find(self, cls_spec, *args, **kwargs):
156 """Perform a query on the previously specified tables.
157@@ -1634,12 +1654,46 @@
158 self._store.flush()
159 find_spec = FindSpec(cls_spec)
160 where = get_where_for_args(args, kwargs, find_spec.default_cls)
161- return self._store._result_set_factory(self._store, find_spec,
162- where, self._tables)
163+ return self._store._result_set_factory(self._store, find_spec, where,
164+ self._tables, with_=self._with)
165+
166+
167+class WithSet(object):
168+ """The representation of a with clause for a query.
169+
170+ This is constructed by calling L{Store.with_}.
171+ """
172+
173+ def __init__(self, store, clause):
174+ self._store = store
175+ self._clause = clause
176+
177+ def find(self, cls_spec, *args, **kwargs):
178+ """Perform a query using the with clause.
179+
180+ This is identical to L{Store.find} except that a with clause is
181+ inserted.
182+
183+ @return: A L{ResultSet}.
184+ """
185+ if self._store._implicit_flush_block_count == 0:
186+ self._store.flush()
187+ find_spec = FindSpec(cls_spec)
188+ where = get_where_for_args(args, kwargs, find_spec.default_cls)
189+ return self._store._result_set_factory(self._store, find_spec, where,
190+ with_=self._clause)
191+
192+ def using(self, *tables):
193+ """Limit the query to some tables.
194+
195+ See L{Store.using}.
196+ """
197+ return self._store._table_set(self._store, tables, self._clause)
198
199
200 Store._result_set_factory = ResultSet
201 Store._table_set = TableSet
202+Store._with_set = WithSet
203
204
205 class FindSpec(object):
206
207=== modified file 'test'
208--- test 2009-11-09 03:26:09 +0000
209+++ test 2011-03-28 00:10:52 +0000
210@@ -94,7 +94,7 @@
211 return testpaths
212
213 def test_with_runner(runner):
214- usage = "test.py [options] [<test filename>, ...]"
215+ usage = "test [options] [<test filename>, ...]"
216
217 parser = optparse.OptionParser(usage=usage)
218
219
220=== modified file 'tests/store/postgres.py'
221--- tests/store/postgres.py 2009-03-04 16:56:58 +0000
222+++ tests/store/postgres.py 2011-03-28 00:10:52 +0000
223@@ -22,6 +22,7 @@
224 import gc
225
226 from storm.database import create_database
227+from storm.expr import Alias, With, SQL
228 from storm.properties import Enum, Int, List
229 from storm.info import get_obj_info
230
231@@ -207,6 +208,39 @@
232 self.store.flush()
233 self.assertEquals(events, [])
234
235+ def test_find_with_smoke(self):
236+ result = self.store.with_(SQL("tmp as (select 20 as id)")).using(Foo
237+ ).find(Foo.title, SQL("foo.id in (select id from tmp)"));
238+ self.assertEqual(list(result), [u'Title 20'])
239+
240+ def test_find_with_using_smoke(self):
241+ result = self.store.with_(SQL("tmp as (select 20 as id)")).find(
242+ Foo.title, SQL("foo.id in (select id from tmp)"));
243+ self.assertEqual(list(result), [u'Title 20'])
244+
245+ def test_count_with_smoke(self):
246+ result = self.store.with_(SQL("tmp as (select 20 as id)")).using(Foo
247+ ).find(Foo.title, SQL("foo.id in (select id from tmp)"));
248+ self.assertEqual(1, result.count())
249+
250+ def test_count_with_distinct_smoke(self):
251+ result = self.store.with_(SQL("tmp as (select 20 as id)")).using(Foo
252+ ).find(Foo.title, SQL("foo.id in (select id from tmp)"));
253+ result = result.config(distinct=True)
254+ self.assertEqual(1, result.count())
255+
256+ def test_contains_with_smoke(self):
257+ result = self.store.with_(SQL("tmp as (select 20 as id)")).using(Foo
258+ ).find(Foo.id, SQL("foo.id in (select id from tmp)"));
259+ self.assertTrue(20 in result)
260+
261+ def test_with_with_expr(self):
262+ expr = self.store.find(Foo.id, Foo.title.like(u'Title 20'))._get_select()
263+ result= self.store.with_(With('tmp', expr)).find(Foo.id,
264+ SQL("foo.id in (select id from tmp)"))
265+ self.assertTrue(20 in result)
266+ self.assertEqual(1, result.count())
267+
268
269 class PostgresEmptyResultSetTest(TestHelper, EmptyResultSetTest):
270

Subscribers

People subscribed via source and target branches

to status/vote changes: