Merge lp:~seif/zeitgeist/fix-redundant-statement into lp:zeitgeist/0.1

[Seif Lotfy]

Basically I found this statement. For me we could get rid of one of them. Right now using only one statement of both works for all our test cases.

if return_mode == 0:
    sql = "SELECT DISTINCT id FROM event_view"
elif return_mode == 1:
    sql = "SELECT id FROM event_view"

Do we really need both? In any case here is a little merge proposal

[Markus Korn (thekorn)]

If you are sure that we don't need `DISTINCT` in this query (I have not checked it in detail), you should also remove the `return_mode` argument.

review: Needs Fixing

[Markus Korn (thekorn)]

This testcase [0] works fine in lp:zeitgeist, but fails in your branch. So removing DISTINCT seems to be wrong, Disapproved ;)

[0] http://paste.ubuntu.com/535972/

review: Disapprove

[Siegfried Gevatter (rainct)]

Yeah, don't remove DISTINCT.

[Mikkel Kamstrup Erlandsen (kamstrup)]

I can see what you're getting at. I recall that I told Siegfried to
add DISTINCT at the hackfest because we where getting some wrong
results... I can't remember the exact details...

[Siegfried Gevatter (rainct)]

Uhm? Without the distinct FindEventIds will be giving duplicated stuff.

[Mikkel Kamstrup Erlandsen (kamstrup)]

review needsfixing

Ah right. Now I get it.

If we have multiple subjects for one event we'll have multiple rows in
events_view with the same id. I can't see how our multi-subject tests
can pass if you remove the DISTINCT. It may be worthwhile adding a
test that proves my point...

If you're very keen on removing the DISTINCT (fx. if you have
profiling data to back it up) then it looks like you can do it if you
manually coalesce the id list before returning it or passing it to
get_events().

[Mikkel Kamstrup Erlandsen (kamstrup)]

I just added this test in trunk rev 1638. And indeed it fails if I
remove DISTINCT.

Unmerged revisions

1637. By Seif Lotfy on 2010-11-24

removed redundant statement

=== modified file '_zeitgeist/engine/main.py'
--- _zeitgeist/engine/main.py	2010-11-12 18:09:45 +0000
+++ _zeitgeist/engine/main.py	2010-11-24 15:51:17 +0000
@@ -335,9 +335,7 @@
 		if not where.may_have_results():
 			return []
 		
-		if return_mode == 0:
-			sql = "SELECT DISTINCT id FROM event_view"
-		elif return_mode == 1:
+		if return_mode in (0,1):
 			sql = "SELECT id FROM event_view"
 		else:
 			raise NotImplementedError, "Unsupported return_mode."