Storm

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

  1. bug-585704
  2. Merge into trunk
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
=== modified file 'NEWS'
--- NEWS 2010-04-16 09:46:32 +0000
+++ NEWS 2010-06-01 08:21:26 +0000
@@ -10,6 +10,7 @@
1010
11Bug fixes11Bug fixes
12---------12---------
13 - Make storm compatible with psycopg2 2.2 (bug #585704).
1314
1415
150.16 (2009-11-28)160.16 (2009-11-28)
1617
=== modified file 'storm/variables.py'
--- storm/variables.py 2009-10-03 16:08:04 +0000
+++ storm/variables.py 2010-06-01 08:21:26 +0000
@@ -369,7 +369,7 @@
369 @staticmethod369 @staticmethod
370 def parse_get(value, to_db):370 def parse_get(value, to_db):
371 if to_db:371 if to_db:
372 return str(value)372 return unicode(value)
373 return value373 return value
374374
375375
376376
=== modified file 'tests/databases/base.py'
--- tests/databases/base.py 2010-04-16 07:12:13 +0000
+++ tests/databases/base.py 2010-06-01 08:21:26 +0000
@@ -206,7 +206,7 @@
206 "VALUES ('Title 30')")206 "VALUES ('Title 30')")
207 primary_key = (Column("id", SQLToken("test")),207 primary_key = (Column("id", SQLToken("test")),
208 Column("title", SQLToken("test")))208 Column("title", SQLToken("test")))
209 primary_variables = (Variable(), Variable("Title 30"))209 primary_variables = (Variable(), Variable(u"Title 30"))
210 expr = result.get_insert_identity(primary_key, primary_variables)210 expr = result.get_insert_identity(primary_key, primary_variables)
211 select = Select(Column("title", SQLToken("test")), expr)211 select = Select(Column("title", SQLToken("test")), expr)
212 result = self.connection.execute(select)212 result = self.connection.execute(select)
213213
=== modified file 'tests/databases/postgres.py'
--- tests/databases/postgres.py 2009-07-29 15:40:35 +0000
+++ tests/databases/postgres.py 2010-06-01 08:21:26 +0000
@@ -267,35 +267,35 @@
267267
268 def test_case_default_like(self):268 def test_case_default_like(self):
269269
270 like = Like(SQLRaw("description"), "%hullah%")270 like = Like(SQLRaw("description"), u"%hullah%")
271 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])271 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])
272 result = self.connection.execute(expr)272 result = self.connection.execute(expr)
273 self.assertEquals(result.get_all(), [(1,)])273 self.assertEquals(result.get_all(), [(1,)])
274274
275 like = Like(SQLRaw("description"), "%HULLAH%")275 like = Like(SQLRaw("description"), u"%HULLAH%")
276 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])276 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])
277 result = self.connection.execute(expr)277 result = self.connection.execute(expr)
278 self.assertEquals(result.get_all(), [(2,)])278 self.assertEquals(result.get_all(), [(2,)])
279279
280 def test_case_sensitive_like(self):280 def test_case_sensitive_like(self):
281281
282 like = Like(SQLRaw("description"), "%hullah%", case_sensitive=True)282 like = Like(SQLRaw("description"), u"%hullah%", case_sensitive=True)
283 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])283 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])
284 result = self.connection.execute(expr)284 result = self.connection.execute(expr)
285 self.assertEquals(result.get_all(), [(1,)])285 self.assertEquals(result.get_all(), [(1,)])
286286
287 like = Like(SQLRaw("description"), "%HULLAH%", case_sensitive=True)287 like = Like(SQLRaw("description"), u"%HULLAH%", case_sensitive=True)
288 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])288 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])
289 result = self.connection.execute(expr)289 result = self.connection.execute(expr)
290 self.assertEquals(result.get_all(), [(2,)])290 self.assertEquals(result.get_all(), [(2,)])
291291
292 def test_case_insensitive_like(self):292 def test_case_insensitive_like(self):
293293
294 like = Like(SQLRaw("description"), "%hullah%", case_sensitive=False)294 like = Like(SQLRaw("description"), u"%hullah%", case_sensitive=False)
295 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])295 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])
296 result = self.connection.execute(expr)296 result = self.connection.execute(expr)
297 self.assertEquals(result.get_all(), [(1,), (2,)])297 self.assertEquals(result.get_all(), [(1,), (2,)])
298 like = Like(SQLRaw("description"), "%HULLAH%", case_sensitive=False)298 like = Like(SQLRaw("description"), u"%HULLAH%", case_sensitive=False)
299 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])299 expr = Select(SQLRaw("id"), like, tables=["like_case_insensitive_test"])
300 result = self.connection.execute(expr)300 result = self.connection.execute(expr)
301 self.assertEquals(result.get_all(), [(1,), (2,)])301 self.assertEquals(result.get_all(), [(1,), (2,)])

Subscribers

People subscribed via source and target branches

to status/vote changes: