Merge lp:~wgrant/storm/distinct-on into lp:storm

[William Grant]

This (surprisingly small) branch adds DISTINCT ON support. Select()'s distinct argument has traditionally been a boolean, so it seemed safe and sensible enough to allow it to optionally take a list of columns to imply DISTINCT ON. I opted to use raw=True, as is already used for the other Select arguments.

[Данило Шеган (danilo)]

Does it make sense to have a test to confirm it works properly for multi-table selects and distinct on a column which is not included in the select columns?

Otherwise, looks pretty good.

review: Approve

[Thomas Herve (therve)]

It's relatively unfortunate to change the global compiler, as it's only supported on Postgres. OTOH it should fail pretty obviously on other databases, so I guess it's fine.

[1] It would be nice to update the docstring of Store.config to mention the new format for distinct (if you're using Postgres).

[2] The state.push calls should be moved before the compilation of the columns.

[3] Please add a note in the NEWS file mentioning the feature.

Thanks, merge once fixed!

review: Approve

[lp:~wgrant/storm/distinct-on]

392. By William Grant on 2011-05-16

Update config(distinct)'s docstring.

393. By William Grant on 2011-05-16

Push state before compiling columns.

394. By William Grant on 2011-05-16

Add NEWS entry.

395. By William Grant on 2011-05-16

Fix existing NEWS indentation.

396. By William Grant on 2011-05-16

Note that DISTINCT ON is only supported by postgres.

397. By William Grant on 2011-05-16

Merge trunk.

=== modified file 'NEWS'
--- NEWS	2011-05-04 11:50:23 +0000
+++ NEWS	2011-05-16 10:52:31 +0000
@@ -18,12 +18,14 @@
                   "schema-uri: "postgres://schema_user@host/db"}]
     manager = ZStormResourceManager(databases)
 
-    where the "schema-uri" key is optional and would default to "uri" if
-    not given. The old format of the 'databases' parameter is still supported
-    but deprecated.
-  - A new storm.twisted.transact module has been added with facilities to
-    integrate Storm with twisted, by running transactions in a separate
-    thread from the main one in order to not block the reactor.
+   where the "schema-uri" key is optional and would default to "uri" if
+   not given. The old format of the 'databases' parameter is still supported
+   but deprecated.
+ - A new storm.twisted.transact module has been added with facilities to
+   integrate Storm with twisted, by running transactions in a separate
+   thread from the main one in order to not block the reactor.
+ - ResultSet.config's "distinct" argument now also accepts a tuple of
+   columns, which will be turned into a DISTINCT ON clause.
 
 Bug fixes
 ---------

=== modified file 'storm/expr.py'
--- storm/expr.py	2011-03-15 07:54:00 +0000
+++ storm/expr.py	2011-05-16 10:52:31 +0000
@@ -656,10 +656,13 @@
 @compile.when(Select)
 def compile_select(compile, select, state):
     tokens = ["SELECT "]
+    state.push("auto_tables", [])
+    state.push("context", COLUMN)
     if select.distinct:
         tokens.append("DISTINCT ")
-    state.push("auto_tables", [])
-    state.push("context", COLUMN)
+        if isinstance(select.distinct, (tuple, list)):
+            tokens.append(
+                "ON (%s) " % compile(select.distinct, state, raw=True))
     tokens.append(compile(select.columns, state))
     tables_pos = len(tokens)
     parameters_pos = len(state.parameters)

=== modified file 'storm/store.py'
--- storm/store.py	2011-01-05 10:19:49 +0000
+++ storm/store.py	2011-05-16 10:52:31 +0000
@@ -935,8 +935,9 @@
     def config(self, distinct=None, offset=None, limit=None):
         """Configure this result object in-place. All parameters are optional.
 
-        @param distinct: Boolean enabling/disabling usage of the DISTINCT
-            keyword in the query made.
+        @param distinct: If True, enables usage of the DISTINCT keyword in
+            the query. If a tuple or list of columns, inserts a DISTINCT ON
+            (only supported by PostgreSQL).
         @param offset: Offset where results will start to be retrieved
             from the result set.
         @param limit: Limit the number of objects retrieved from the

=== modified file 'tests/expr.py'
--- tests/expr.py	2010-12-07 15:00:30 +0000
+++ tests/expr.py	2011-05-16 10:52:31 +0000
@@ -657,6 +657,16 @@
                           'SELECT DISTINCT column1, column2 FROM "table 1"')
         self.assertEquals(state.parameters, [])
 
+    def test_select_distinct_on(self):
+        expr = Select([column1, column2], Undef, [table1],
+                      distinct=[column2, column1])
+        state = State()
+        statement = compile(expr, state)
+        self.assertEquals(statement,
+                          'SELECT DISTINCT ON (column2, column1) '
+                          'column1, column2 FROM "table 1"')
+        self.assertEquals(state.parameters, [])
+
     def test_select_where(self):
         expr = Select([column1, Func1()],
                       Func1(),