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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gustavo Niemeyer | Needs Fixing | ||
Review via email: mp+52630@code.launchpad.net |
Commit message
Description of the change
Implement WITH support - enough for Launchpads needs.
Gustavo Niemeyer (niemeyer) wrote : | # |
[1]
199 + result = self.store.
200 + ).find(Foo.title, SQL("foo.id in (select id from tmp)"));
201 + self.assertEqua
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.
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.
> 200 + ).find(Foo.title, SQL("foo.id in (select id from tmp)"));
> 201 + self.assertEqua
>
> 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
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.
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
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.
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.
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".
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.
3. Recursive queries could easily be handled internally to the WithTable in this model.
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(".
WithTable = ClassAlias(cls, table)
also, normal classes could be defined with:
table = With(".
class Person(object):
and could be used in finds in an equivalent way:
table = With(".
store.
> 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.
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?
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.
Robert Collins (lifeless) wrote : | # |
@therve - uses of with_ so far:
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.
Robert Collins (lifeless) wrote : | # |
@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 branchmergeprop
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...
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.
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.
SELECT company_
What will company_
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.
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/
> 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
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/
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.
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
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 | 636 | 636 | ||
6 | 637 | class Select(Expr): | 637 | class Select(Expr): |
7 | 638 | __slots__ = ("columns", "where", "tables", "default_tables", "order_by", | 638 | __slots__ = ("columns", "where", "tables", "default_tables", "order_by", |
9 | 639 | "group_by", "limit", "offset", "distinct", "having") | 639 | "group_by", "limit", "offset", "distinct", "having", "with_") |
10 | 640 | 640 | ||
11 | 641 | def __init__(self, columns, where=Undef, | 641 | def __init__(self, columns, where=Undef, |
12 | 642 | tables=Undef, default_tables=Undef, | 642 | tables=Undef, default_tables=Undef, |
13 | 643 | order_by=Undef, group_by=Undef, | 643 | order_by=Undef, group_by=Undef, |
15 | 644 | limit=Undef, offset=Undef, distinct=False, having=Undef): | 644 | limit=Undef, offset=Undef, distinct=False, having=Undef, |
16 | 645 | with_=Undef): | ||
17 | 645 | self.columns = columns | 646 | self.columns = columns |
18 | 646 | self.where = where | 647 | self.where = where |
19 | 647 | self.tables = tables | 648 | self.tables = tables |
20 | @@ -652,10 +653,15 @@ | |||
21 | 652 | self.offset = offset | 653 | self.offset = offset |
22 | 653 | self.distinct = distinct | 654 | self.distinct = distinct |
23 | 654 | self.having = having | 655 | self.having = having |
24 | 656 | self.with_ = with_ | ||
25 | 655 | 657 | ||
26 | 656 | @compile.when(Select) | 658 | @compile.when(Select) |
27 | 657 | def compile_select(compile, select, state): | 659 | def compile_select(compile, select, state): |
29 | 658 | tokens = ["SELECT "] | 660 | tokens = [] |
30 | 661 | if select.with_ is not Undef: | ||
31 | 662 | tokens.append("WITH ") | ||
32 | 663 | tokens.append(compile(select.with_, state)) | ||
33 | 664 | tokens.append("SELECT ") | ||
34 | 659 | if select.distinct: | 665 | if select.distinct: |
35 | 660 | tokens.append("DISTINCT ") | 666 | tokens.append("DISTINCT ") |
36 | 661 | state.push("auto_tables", []) | 667 | state.push("auto_tables", []) |
37 | @@ -1481,6 +1487,27 @@ | |||
38 | 1481 | 1487 | ||
39 | 1482 | 1488 | ||
40 | 1483 | # -------------------------------------------------------------------- | 1489 | # -------------------------------------------------------------------- |
41 | 1490 | # WITH clause | ||
42 | 1491 | |||
43 | 1492 | from storm.expr import compile, Expr, SQL | ||
44 | 1493 | |||
45 | 1494 | class With(Expr): | ||
46 | 1495 | """Compiles to a simple (name AS expr) used in constructing WITH clauses.""" | ||
47 | 1496 | |||
48 | 1497 | def __init__(self, name, select): | ||
49 | 1498 | self.name = name | ||
50 | 1499 | self.select = select | ||
51 | 1500 | |||
52 | 1501 | @compile.when(With) | ||
53 | 1502 | def compile_with(compile, with_expr, state): | ||
54 | 1503 | tokens = [] | ||
55 | 1504 | tokens.append(compile(with_expr.name, state, token=True)) | ||
56 | 1505 | tokens.append(" AS ") | ||
57 | 1506 | tokens.append(compile(with_expr.select, state)) | ||
58 | 1507 | return "".join(tokens) | ||
59 | 1508 | |||
60 | 1509 | |||
61 | 1510 | # -------------------------------------------------------------------- | ||
62 | 1484 | # Set operator precedences. | 1511 | # Set operator precedences. |
63 | 1485 | 1512 | ||
64 | 1486 | compile.set_precedence(10, Select, Insert, Update, Delete) | 1513 | compile.set_precedence(10, Select, Insert, Update, Delete) |
65 | 1487 | 1514 | ||
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 | 232 | """ | 232 | """ |
71 | 233 | return self._table_set(self, tables) | 233 | return self._table_set(self, tables) |
72 | 234 | 234 | ||
73 | 235 | def with_(self, clause): | ||
74 | 236 | """Specify a with clause for use with the query. | ||
75 | 237 | |||
76 | 238 | For instance:: | ||
77 | 239 | |||
78 | 240 | store.with_(SQL("tmpname as (SELECT ...)")).find(Person, | ||
79 | 241 | SQL("Person.id in (SELECT id from tmpname)")) | ||
80 | 242 | |||
81 | 243 | will compile to | ||
82 | 244 | |||
83 | 245 | WITH tmpname as (SELECT ...) | ||
84 | 246 | SELECT Person.id FROM Person WHERE | ||
85 | 247 | PERSON.id in (SELECT id from tmpname); | ||
86 | 248 | |||
87 | 249 | @return: A L{WithSet}, which has C{find} and C{using} methods | ||
88 | 250 | similar to L{Store.find} and L{Store.using}. | ||
89 | 251 | """ | ||
90 | 252 | return self._with_set(self, clause) | ||
91 | 253 | |||
92 | 235 | def add(self, obj): | 254 | def add(self, obj): |
93 | 236 | """Add the given object to the store. | 255 | """Add the given object to the store. |
94 | 237 | 256 | ||
95 | @@ -367,7 +386,6 @@ | |||
96 | 367 | # to test it without whitebox. | 386 | # to test it without whitebox. |
97 | 368 | self._order.clear() | 387 | self._order.clear() |
98 | 369 | 388 | ||
99 | 370 | |||
100 | 371 | def _mark_autoreload(self, obj=None, invalidate=False): | 389 | def _mark_autoreload(self, obj=None, invalidate=False): |
101 | 372 | if obj is None: | 390 | if obj is None: |
102 | 373 | obj_infos = self._iter_alive() | 391 | obj_infos = self._iter_alive() |
103 | @@ -908,12 +926,13 @@ | |||
104 | 908 | """ | 926 | """ |
105 | 909 | 927 | ||
106 | 910 | def __init__(self, store, find_spec, | 928 | def __init__(self, store, find_spec, |
108 | 911 | where=Undef, tables=Undef, select=Undef): | 929 | where=Undef, tables=Undef, select=Undef, with_=Undef): |
109 | 912 | self._store = store | 930 | self._store = store |
110 | 913 | self._find_spec = find_spec | 931 | self._find_spec = find_spec |
111 | 914 | self._where = where | 932 | self._where = where |
112 | 915 | self._tables = tables | 933 | self._tables = tables |
113 | 916 | self._select = select | 934 | self._select = select |
114 | 935 | self._with = with_ | ||
115 | 917 | self._order_by = find_spec.default_order | 936 | self._order_by = find_spec.default_order |
116 | 918 | self._offset = Undef | 937 | self._offset = Undef |
117 | 919 | self._limit = Undef | 938 | self._limit = Undef |
118 | @@ -966,7 +985,7 @@ | |||
119 | 966 | return Select(columns, self._where, self._tables, default_tables, | 985 | return Select(columns, self._where, self._tables, default_tables, |
120 | 967 | self._order_by, offset=self._offset, limit=self._limit, | 986 | self._order_by, offset=self._offset, limit=self._limit, |
121 | 968 | distinct=self._distinct, group_by=self._group_by, | 987 | distinct=self._distinct, group_by=self._group_by, |
123 | 969 | having=self._having) | 988 | having=self._having, with_=self._with) |
124 | 970 | 989 | ||
125 | 971 | def _load_objects(self, result, values): | 990 | def _load_objects(self, result, values): |
126 | 972 | return self._find_spec.load_objects(self._store, result, values) | 991 | return self._find_spec.load_objects(self._store, result, values) |
127 | @@ -1036,7 +1055,7 @@ | |||
128 | 1036 | if self._where is not Undef: | 1055 | if self._where is not Undef: |
129 | 1037 | where.append(self._where) | 1056 | where.append(self._where) |
130 | 1038 | select = Select(1, And(*where), self._tables, | 1057 | select = Select(1, And(*where), self._tables, |
132 | 1039 | default_tables) | 1058 | default_tables, with_=self._with) |
133 | 1040 | else: | 1059 | else: |
134 | 1041 | # Rewrite the predefined query and use it as a subquery. | 1060 | # Rewrite the predefined query and use it as a subquery. |
135 | 1042 | aliased_columns = [Alias(column, "_key%d" % index) | 1061 | aliased_columns = [Alias(column, "_key%d" % index) |
136 | @@ -1219,7 +1238,7 @@ | |||
137 | 1219 | if (self._select is Undef and not self._distinct and | 1238 | if (self._select is Undef and not self._distinct and |
138 | 1220 | self._offset is Undef and self._limit is Undef): | 1239 | self._offset is Undef and self._limit is Undef): |
139 | 1221 | select = Select(aggregate_func(expr), self._where, | 1240 | select = Select(aggregate_func(expr), self._where, |
141 | 1222 | self._tables, default_tables) | 1241 | self._tables, default_tables, with_=self._with) |
142 | 1223 | else: | 1242 | else: |
143 | 1224 | if expr is Undef: | 1243 | if expr is Undef: |
144 | 1225 | aggregate = aggregate_func(expr) | 1244 | aggregate = aggregate_func(expr) |
145 | @@ -1618,9 +1637,10 @@ | |||
146 | 1618 | This will typically be constructed by a call to L{Store.using}. | 1637 | This will typically be constructed by a call to L{Store.using}. |
147 | 1619 | """ | 1638 | """ |
148 | 1620 | 1639 | ||
150 | 1621 | def __init__(self, store, tables): | 1640 | def __init__(self, store, tables, with_=Undef): |
151 | 1622 | self._store = store | 1641 | self._store = store |
152 | 1623 | self._tables = tables | 1642 | self._tables = tables |
153 | 1643 | self._with = with_ | ||
154 | 1624 | 1644 | ||
155 | 1625 | def find(self, cls_spec, *args, **kwargs): | 1645 | def find(self, cls_spec, *args, **kwargs): |
156 | 1626 | """Perform a query on the previously specified tables. | 1646 | """Perform a query on the previously specified tables. |
157 | @@ -1634,12 +1654,46 @@ | |||
158 | 1634 | self._store.flush() | 1654 | self._store.flush() |
159 | 1635 | find_spec = FindSpec(cls_spec) | 1655 | find_spec = FindSpec(cls_spec) |
160 | 1636 | where = get_where_for_args(args, kwargs, find_spec.default_cls) | 1656 | where = get_where_for_args(args, kwargs, find_spec.default_cls) |
163 | 1637 | return self._store._result_set_factory(self._store, find_spec, | 1657 | return self._store._result_set_factory(self._store, find_spec, where, |
164 | 1638 | where, self._tables) | 1658 | self._tables, with_=self._with) |
165 | 1659 | |||
166 | 1660 | |||
167 | 1661 | class WithSet(object): | ||
168 | 1662 | """The representation of a with clause for a query. | ||
169 | 1663 | |||
170 | 1664 | This is constructed by calling L{Store.with_}. | ||
171 | 1665 | """ | ||
172 | 1666 | |||
173 | 1667 | def __init__(self, store, clause): | ||
174 | 1668 | self._store = store | ||
175 | 1669 | self._clause = clause | ||
176 | 1670 | |||
177 | 1671 | def find(self, cls_spec, *args, **kwargs): | ||
178 | 1672 | """Perform a query using the with clause. | ||
179 | 1673 | |||
180 | 1674 | This is identical to L{Store.find} except that a with clause is | ||
181 | 1675 | inserted. | ||
182 | 1676 | |||
183 | 1677 | @return: A L{ResultSet}. | ||
184 | 1678 | """ | ||
185 | 1679 | if self._store._implicit_flush_block_count == 0: | ||
186 | 1680 | self._store.flush() | ||
187 | 1681 | find_spec = FindSpec(cls_spec) | ||
188 | 1682 | where = get_where_for_args(args, kwargs, find_spec.default_cls) | ||
189 | 1683 | return self._store._result_set_factory(self._store, find_spec, where, | ||
190 | 1684 | with_=self._clause) | ||
191 | 1685 | |||
192 | 1686 | def using(self, *tables): | ||
193 | 1687 | """Limit the query to some tables. | ||
194 | 1688 | |||
195 | 1689 | See L{Store.using}. | ||
196 | 1690 | """ | ||
197 | 1691 | return self._store._table_set(self._store, tables, self._clause) | ||
198 | 1639 | 1692 | ||
199 | 1640 | 1693 | ||
200 | 1641 | Store._result_set_factory = ResultSet | 1694 | Store._result_set_factory = ResultSet |
201 | 1642 | Store._table_set = TableSet | 1695 | Store._table_set = TableSet |
202 | 1696 | Store._with_set = WithSet | ||
203 | 1643 | 1697 | ||
204 | 1644 | 1698 | ||
205 | 1645 | class FindSpec(object): | 1699 | class FindSpec(object): |
206 | 1646 | 1700 | ||
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 | 94 | return testpaths | 94 | return testpaths |
212 | 95 | 95 | ||
213 | 96 | def test_with_runner(runner): | 96 | def test_with_runner(runner): |
215 | 97 | usage = "test.py [options] [<test filename>, ...]" | 97 | usage = "test [options] [<test filename>, ...]" |
216 | 98 | 98 | ||
217 | 99 | parser = optparse.OptionParser(usage=usage) | 99 | parser = optparse.OptionParser(usage=usage) |
218 | 100 | 100 | ||
219 | 101 | 101 | ||
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 | 22 | import gc | 22 | import gc |
225 | 23 | 23 | ||
226 | 24 | from storm.database import create_database | 24 | from storm.database import create_database |
227 | 25 | from storm.expr import Alias, With, SQL | ||
228 | 25 | from storm.properties import Enum, Int, List | 26 | from storm.properties import Enum, Int, List |
229 | 26 | from storm.info import get_obj_info | 27 | from storm.info import get_obj_info |
230 | 27 | 28 | ||
231 | @@ -207,6 +208,39 @@ | |||
232 | 207 | self.store.flush() | 208 | self.store.flush() |
233 | 208 | self.assertEquals(events, []) | 209 | self.assertEquals(events, []) |
234 | 209 | 210 | ||
235 | 211 | def test_find_with_smoke(self): | ||
236 | 212 | result = self.store.with_(SQL("tmp as (select 20 as id)")).using(Foo | ||
237 | 213 | ).find(Foo.title, SQL("foo.id in (select id from tmp)")); | ||
238 | 214 | self.assertEqual(list(result), [u'Title 20']) | ||
239 | 215 | |||
240 | 216 | def test_find_with_using_smoke(self): | ||
241 | 217 | result = self.store.with_(SQL("tmp as (select 20 as id)")).find( | ||
242 | 218 | Foo.title, SQL("foo.id in (select id from tmp)")); | ||
243 | 219 | self.assertEqual(list(result), [u'Title 20']) | ||
244 | 220 | |||
245 | 221 | def test_count_with_smoke(self): | ||
246 | 222 | result = self.store.with_(SQL("tmp as (select 20 as id)")).using(Foo | ||
247 | 223 | ).find(Foo.title, SQL("foo.id in (select id from tmp)")); | ||
248 | 224 | self.assertEqual(1, result.count()) | ||
249 | 225 | |||
250 | 226 | def test_count_with_distinct_smoke(self): | ||
251 | 227 | result = self.store.with_(SQL("tmp as (select 20 as id)")).using(Foo | ||
252 | 228 | ).find(Foo.title, SQL("foo.id in (select id from tmp)")); | ||
253 | 229 | result = result.config(distinct=True) | ||
254 | 230 | self.assertEqual(1, result.count()) | ||
255 | 231 | |||
256 | 232 | def test_contains_with_smoke(self): | ||
257 | 233 | result = self.store.with_(SQL("tmp as (select 20 as id)")).using(Foo | ||
258 | 234 | ).find(Foo.id, SQL("foo.id in (select id from tmp)")); | ||
259 | 235 | self.assertTrue(20 in result) | ||
260 | 236 | |||
261 | 237 | def test_with_with_expr(self): | ||
262 | 238 | expr = self.store.find(Foo.id, Foo.title.like(u'Title 20'))._get_select() | ||
263 | 239 | result= self.store.with_(With('tmp', expr)).find(Foo.id, | ||
264 | 240 | SQL("foo.id in (select id from tmp)")) | ||
265 | 241 | self.assertTrue(20 in result) | ||
266 | 242 | self.assertEqual(1, result.count()) | ||
267 | 243 | |||
268 | 210 | 244 | ||
269 | 211 | class PostgresEmptyResultSetTest(TestHelper, EmptyResultSetTest): | 245 | class PostgresEmptyResultSetTest(TestHelper, EmptyResultSetTest): |
270 | 212 | 246 |
http:// www.postgresql. org/docs/ 9.0/static/ queries- with.html for what this permits