Merge lp:~jamesh/storm/bug-585704 into lp:storm

Proposed by James Henstridge
Status: Merged
Merged at revision: 373
Proposed branch: lp:~jamesh/storm/bug-585704
Merge into: lp:storm
Diff against target: 83 lines (+9/-8)
4 files modified
NEWS (+1/-0)
storm/variables.py (+1/-1)
tests/databases/base.py (+1/-1)
tests/databases/postgres.py (+6/-6)
To merge this branch: bzr merge lp:~jamesh/storm/bug-585704
Reviewer Review Type Date Requested Status
John O'Brien (community) Approve
Thomas Herve (community) Approve
Review via email: mp+26472@code.launchpad.net

Description of the change

Fix some incompatibilities with the latest versions of pyscopg2.

The problems all seemed to indicate bugs in Storm, where we were passing text data to psycopg2 using binary types. With newer psycopg2, the Binary() type includes an explicit cast when interpolated, which caused the problems.

To post a comment you must log in.
Revision history for this message
Thomas Herve (therve) wrote :

Seems reasonable, +1!

review: Approve
Revision history for this message
John O'Brien (jdobrien) wrote :

After applying your fix, we are getting this:

ERROR: operator does not exist: lifecycle_status = bytea
LINE 1: ...th = E'~/Ubuntu One' AND UserDefinedFolder.status = E'Live':...

When we use a Type:

CREATE TYPE lifecycle_status as enum('Live', 'Dead')

review: Needs Information
Revision history for this message
John O'Brien (jdobrien) wrote :

> After applying your fix, we are getting this:
>
> ERROR: operator does not exist: lifecycle_status = bytea
> LINE 1: ...th = E'~/Ubuntu One' AND UserDefinedFolder.status = E'Live':...
>
> When we use a Type:
>
> CREATE TYPE lifecycle_status as enum('Live', 'Dead')

BTW, In order to fix this, we made sure we only assigned Unicode strings to the Properties.

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

John: I assume you'd get that error without my fix if you move to the new version of psycopg2.

This branch doesn't change the way Storm passes byte strings down to psycopg2 (it has always wrapped them in psycopg2.Binary() objects) -- it just fixes a few cases inside Storm where we got byte string vs. unicode string wrong. Switching those custom variable types to use unicode strings was the correct fix for the U1 code.

Revision history for this message
John O'Brien (jdobrien) wrote :

thanks for the reply, this fix worked great for us.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-04-16 09:46:32 +0000
3+++ NEWS 2010-06-01 08:21:26 +0000
4@@ -10,6 +10,7 @@
5
6 Bug fixes
7 ---------
8+ - Make storm compatible with psycopg2 2.2 (bug #585704).
9
10
11 0.16 (2009-11-28)
12
13=== modified file 'storm/variables.py'
14--- storm/variables.py 2009-10-03 16:08:04 +0000
15+++ storm/variables.py 2010-06-01 08:21:26 +0000
16@@ -369,7 +369,7 @@
17 @staticmethod
18 def parse_get(value, to_db):
19 if to_db:
20- return str(value)
21+ return unicode(value)
22 return value
23
24
25
26=== modified file 'tests/databases/base.py'
27--- tests/databases/base.py 2010-04-16 07:12:13 +0000
28+++ tests/databases/base.py 2010-06-01 08:21:26 +0000
29@@ -206,7 +206,7 @@
30 "VALUES ('Title 30')")
31 primary_key = (Column("id", SQLToken("test")),
32 Column("title", SQLToken("test")))
33- primary_variables = (Variable(), Variable("Title 30"))
34+ primary_variables = (Variable(), Variable(u"Title 30"))
35 expr = result.get_insert_identity(primary_key, primary_variables)
36 select = Select(Column("title", SQLToken("test")), expr)
37 result = self.connection.execute(select)
38
39=== modified file 'tests/databases/postgres.py'
40--- tests/databases/postgres.py 2009-07-29 15:40:35 +0000
41+++ tests/databases/postgres.py 2010-06-01 08:21:26 +0000
42@@ -267,35 +267,35 @@
43
44 def test_case_default_like(self):
45
46- like = Like(SQLRaw("description"), "%hullah%")
47+ like = Like(SQLRaw("description"), u"%hullah%")
48 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])
49 result = self.connection.execute(expr)
50 self.assertEquals(result.get_all(), [(1,)])
51
52- like = Like(SQLRaw("description"), "%HULLAH%")
53+ like = Like(SQLRaw("description"), u"%HULLAH%")
54 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])
55 result = self.connection.execute(expr)
56 self.assertEquals(result.get_all(), [(2,)])
57
58 def test_case_sensitive_like(self):
59
60- like = Like(SQLRaw("description"), "%hullah%", case_sensitive=True)
61+ like = Like(SQLRaw("description"), u"%hullah%", case_sensitive=True)
62 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])
63 result = self.connection.execute(expr)
64 self.assertEquals(result.get_all(), [(1,)])
65
66- like = Like(SQLRaw("description"), "%HULLAH%", case_sensitive=True)
67+ like = Like(SQLRaw("description"), u"%HULLAH%", case_sensitive=True)
68 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])
69 result = self.connection.execute(expr)
70 self.assertEquals(result.get_all(), [(2,)])
71
72 def test_case_insensitive_like(self):
73
74- like = Like(SQLRaw("description"), "%hullah%", case_sensitive=False)
75+ like = Like(SQLRaw("description"), u"%hullah%", case_sensitive=False)
76 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])
77 result = self.connection.execute(expr)
78 self.assertEquals(result.get_all(), [(1,), (2,)])
79- like = Like(SQLRaw("description"), "%HULLAH%", case_sensitive=False)
80+ like = Like(SQLRaw("description"), u"%HULLAH%", case_sensitive=False)
81 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])
82 result = self.connection.execute(expr)
83 self.assertEquals(result.get_all(), [(1,), (2,)])

Subscribers

People subscribed via source and target branches

to status/vote changes: