Launchpad itself

Merge lp:~wgrant/launchpad/sqlbase-E-strings into lp:launchpad

  1. sqlbase-E-strings
  2. Merge into devel
Proposed by William Grant
Status: Merged
Merged at revision: 17381
Proposed branch: lp:~wgrant/launchpad/sqlbase-E-strings
Merge into: lp:launchpad
Diff against target: 414 lines (+53/-53)
4 files modified
lib/lp/bugs/model/tests/test_bugtasksearch.py (+34/-34)
lib/lp/services/database/sqlbase.py (+12/-12)
lib/lp/services/webapp/tests/test_batching.py (+5/-5)
lib/sqlobject/__init__.py (+2/-2)
To merge this branch: bzr merge lp:~wgrant/launchpad/sqlbase-E-strings
Reviewer Review Type Date Requested Status
Stuart Bishop (community) Approve
Review via email: mp+250263@code.launchpad.net

Commit message

Port sqlvalues and co to postgres E'strings', rather than relying on standard_conforming_strings=off.

Description of the change

Port sqlvalues and co to postgres E'strings', rather than relying on standard_conforming_strings=off. They also now escape ' as \' rather than '', mostly so people can lazily paste them into Python and not get insane results.

To post a comment you must log in.
Revision history for this message
Stuart Bishop (stub) wrote :

Yup.

psycopg2 can do some of this if you know where to look, but this implementation is simpler and easier to maintain than the alternative.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/model/tests/test_bugtasksearch.py'
2--- lib/lp/bugs/model/tests/test_bugtasksearch.py 2015-01-29 16:28:30 +0000
3+++ lib/lp/bugs/model/tests/test_bugtasksearch.py 2015-02-19 07:16:10 +0000
4@@ -1853,7 +1853,7 @@
5 """EXISTS
6 (SELECT 1 FROM BugTag
7 WHERE BugTag.bug = BugTaskFlat.bug
8- AND BugTag.tag IN ('fred'))""")
9+ AND BugTag.tag IN (E'fred'))""")
10 self.assertEqualIgnoringWhitespace(
11 expected_query,
12 self.searchClause(any(u'fred')))
13@@ -1865,7 +1865,7 @@
14 """EXISTS
15 (SELECT 1 FROM BugTag
16 WHERE BugTag.bug = BugTaskFlat.bug
17- AND BugTag.tag = 'fred')""")
18+ AND BugTag.tag = E'fred')""")
19 self.assertEqualIgnoringWhitespace(
20 expected_query,
21 self.searchClause(all(u'fred')))
22@@ -1877,7 +1877,7 @@
23 """NOT EXISTS
24 (SELECT 1 FROM BugTag
25 WHERE BugTag.bug = BugTaskFlat.bug
26- AND BugTag.tag = 'fred')""")
27+ AND BugTag.tag = E'fred')""")
28 self.assertEqualIgnoringWhitespace(
29 expected_query,
30 self.searchClause(any(u'-fred')))
31@@ -1889,7 +1889,7 @@
32 """NOT EXISTS
33 (SELECT 1 FROM BugTag
34 WHERE BugTag.bug = BugTaskFlat.bug
35- AND BugTag.tag IN ('fred'))""")
36+ AND BugTag.tag IN (E'fred'))""")
37 self.assertEqualIgnoringWhitespace(
38 expected_query,
39 self.searchClause(all(u'-fred')))
40@@ -1929,7 +1929,7 @@
41 """EXISTS
42 (SELECT 1 FROM BugTag
43 WHERE BugTag.bug = BugTaskFlat.bug
44- AND BugTag.tag IN ('bob', 'fred'))""",
45+ AND BugTag.tag IN (E'bob', E'fred'))""",
46 self.searchClause(any(u'fred', u'bob')))
47 # In an `any` query, a positive wildcard is dominant over
48 # other positive tags because "bugs with one or more tags" is
49@@ -1948,11 +1948,11 @@
50 (EXISTS
51 (SELECT 1 FROM BugTag
52 WHERE BugTag.bug = BugTaskFlat.bug
53- AND BugTag.tag = 'bob')
54+ AND BugTag.tag = E'bob')
55 AND EXISTS
56 (SELECT 1 FROM BugTag
57 WHERE BugTag.bug = BugTaskFlat.bug
58- AND BugTag.tag = 'fred'))""",
59+ AND BugTag.tag = E'fred'))""",
60 self.searchClause(any(u'-fred', u'-bob')))
61 # In an `any` query, a negative wildcard is superfluous in the
62 # presence of other negative tags because "bugs without a
63@@ -1961,7 +1961,7 @@
64 """NOT EXISTS
65 (SELECT 1 FROM BugTag
66 WHERE BugTag.bug = BugTaskFlat.bug
67- AND BugTag.tag = 'fred')""",
68+ AND BugTag.tag = E'fred')""",
69 self.searchClause(any(u'-fred', u'-*')))
70
71 def test_multiple_tag_presence_all(self):
72@@ -1971,11 +1971,11 @@
73 """EXISTS
74 (SELECT 1 FROM BugTag
75 WHERE BugTag.bug = BugTaskFlat.bug
76- AND BugTag.tag = 'bob')
77+ AND BugTag.tag = E'bob')
78 AND EXISTS
79 (SELECT 1 FROM BugTag
80 WHERE BugTag.bug = BugTaskFlat.bug
81- AND BugTag.tag = 'fred')""",
82+ AND BugTag.tag = E'fred')""",
83 self.searchClause(all(u'fred', u'bob')))
84 # In an `all` query, a positive wildcard is superfluous in the
85 # presence of other positive tags because "bugs with a
86@@ -1985,7 +1985,7 @@
87 """EXISTS
88 (SELECT 1 FROM BugTag
89 WHERE BugTag.bug = BugTaskFlat.bug
90- AND BugTag.tag = 'fred')""",
91+ AND BugTag.tag = E'fred')""",
92 self.searchClause(all(u'fred', u'*')))
93
94 def test_multiple_tag_absence_all(self):
95@@ -1995,7 +1995,7 @@
96 """NOT EXISTS
97 (SELECT 1 FROM BugTag
98 WHERE BugTag.bug = BugTaskFlat.bug
99- AND BugTag.tag IN ('bob', 'fred'))""",
100+ AND BugTag.tag IN (E'bob', E'fred'))""",
101 self.searchClause(all(u'-fred', u'-bob')))
102 # In an `all` query, a negative wildcard is dominant over
103 # other negative tags because "bugs without any tags" is a
104@@ -2015,26 +2015,26 @@
105 """EXISTS
106 (SELECT 1 FROM BugTag
107 WHERE BugTag.bug = BugTaskFlat.bug
108- AND BugTag.tag IN ('fred'))
109+ AND BugTag.tag IN (E'fred'))
110 OR NOT EXISTS
111 (SELECT 1 FROM BugTag
112 WHERE BugTag.bug = BugTaskFlat.bug
113- AND BugTag.tag = 'bob')""",
114+ AND BugTag.tag = E'bob')""",
115 self.searchClause(any(u'fred', u'-bob')))
116 self.assertEqualIgnoringWhitespace(
117 """EXISTS
118 (SELECT 1 FROM BugTag
119 WHERE BugTag.bug = BugTaskFlat.bug
120- AND BugTag.tag IN ('eric', 'fred'))
121+ AND BugTag.tag IN (E'eric', E'fred'))
122 OR NOT
123 (EXISTS
124 (SELECT 1 FROM BugTag
125 WHERE BugTag.bug = BugTaskFlat.bug
126- AND BugTag.tag = 'bob')
127+ AND BugTag.tag = E'bob')
128 AND EXISTS
129 (SELECT 1 FROM BugTag
130 WHERE BugTag.bug = BugTaskFlat.bug
131- AND BugTag.tag = 'harry'))""",
132+ AND BugTag.tag = E'harry'))""",
133 self.searchClause(any(u'fred', u'-bob', u'eric', u'-harry')))
134 # The positive wildcard is dominant over other positive tags.
135 self.assertEqualIgnoringWhitespace(
136@@ -2045,11 +2045,11 @@
137 (EXISTS
138 (SELECT 1 FROM BugTag
139 WHERE BugTag.bug = BugTaskFlat.bug
140- AND BugTag.tag = 'bob')
141+ AND BugTag.tag = E'bob')
142 AND EXISTS
143 (SELECT 1 FROM BugTag
144 WHERE BugTag.bug = BugTaskFlat.bug
145- AND BugTag.tag = 'harry'))""",
146+ AND BugTag.tag = E'harry'))""",
147 self.searchClause(any(u'fred', u'-bob', u'*', u'-harry')))
148 # The negative wildcard is superfluous in the presence of
149 # other negative tags.
150@@ -2057,11 +2057,11 @@
151 """EXISTS
152 (SELECT 1 FROM BugTag
153 WHERE BugTag.bug = BugTaskFlat.bug
154- AND BugTag.tag IN ('eric', 'fred'))
155+ AND BugTag.tag IN (E'eric', E'fred'))
156 OR NOT EXISTS
157 (SELECT 1 FROM BugTag
158 WHERE BugTag.bug = BugTaskFlat.bug
159- AND BugTag.tag = 'bob')""",
160+ AND BugTag.tag = E'bob')""",
161 self.searchClause(any(u'fred', u'-bob', u'eric', u'-*')))
162 # The negative wildcard is not superfluous in the absence of
163 # other negative tags.
164@@ -2069,7 +2069,7 @@
165 """EXISTS
166 (SELECT 1 FROM BugTag
167 WHERE BugTag.bug = BugTaskFlat.bug
168- AND BugTag.tag IN ('eric', 'fred'))
169+ AND BugTag.tag IN (E'eric', E'fred'))
170 OR NOT EXISTS
171 (SELECT 1 FROM BugTag
172 WHERE BugTag.bug = BugTaskFlat.bug)""",
173@@ -2084,7 +2084,7 @@
174 OR NOT EXISTS
175 (SELECT 1 FROM BugTag
176 WHERE BugTag.bug = BugTaskFlat.bug
177- AND BugTag.tag = 'harry')""",
178+ AND BugTag.tag = E'harry')""",
179 self.searchClause(any(u'fred', u'-*', u'*', u'-harry')))
180
181 def test_mixed_tags_all(self):
182@@ -2095,25 +2095,25 @@
183 """EXISTS
184 (SELECT 1 FROM BugTag
185 WHERE BugTag.bug = BugTaskFlat.bug
186- AND BugTag.tag = 'fred')
187+ AND BugTag.tag = E'fred')
188 AND NOT EXISTS
189 (SELECT 1 FROM BugTag
190 WHERE BugTag.bug = BugTaskFlat.bug
191- AND BugTag.tag IN ('bob'))""",
192+ AND BugTag.tag IN (E'bob'))""",
193 self.searchClause(all(u'fred', u'-bob')))
194 self.assertEqualIgnoringWhitespace(
195 """EXISTS
196 (SELECT 1 FROM BugTag
197 WHERE BugTag.bug = BugTaskFlat.bug
198- AND BugTag.tag = 'eric')
199+ AND BugTag.tag = E'eric')
200 AND EXISTS
201 (SELECT 1 FROM BugTag
202 WHERE BugTag.bug = BugTaskFlat.bug
203- AND BugTag.tag = 'fred')
204+ AND BugTag.tag = E'fred')
205 AND NOT EXISTS
206 (SELECT 1 FROM BugTag
207 WHERE BugTag.bug = BugTaskFlat.bug
208- AND BugTag.tag IN ('bob', 'harry'))""",
209+ AND BugTag.tag IN (E'bob', E'harry'))""",
210 self.searchClause(all(u'fred', u'-bob', u'eric', u'-harry')))
211 # The positive wildcard is superfluous in the presence of
212 # other positive tags.
213@@ -2121,11 +2121,11 @@
214 """EXISTS
215 (SELECT 1 FROM BugTag
216 WHERE BugTag.bug = BugTaskFlat.bug
217- AND BugTag.tag = 'fred')
218+ AND BugTag.tag = E'fred')
219 AND NOT EXISTS
220 (SELECT 1 FROM BugTag
221 WHERE BugTag.bug = BugTaskFlat.bug
222- AND BugTag.tag IN ('bob', 'harry'))""",
223+ AND BugTag.tag IN (E'bob', E'harry'))""",
224 self.searchClause(all(u'fred', u'-bob', u'*', u'-harry')))
225 # The positive wildcard is not superfluous in the absence of
226 # other positive tags.
227@@ -2136,18 +2136,18 @@
228 AND NOT EXISTS
229 (SELECT 1 FROM BugTag
230 WHERE BugTag.bug = BugTaskFlat.bug
231- AND BugTag.tag IN ('bob', 'harry'))""",
232+ AND BugTag.tag IN (E'bob', E'harry'))""",
233 self.searchClause(all(u'-bob', u'*', u'-harry')))
234 # The negative wildcard is dominant over other negative tags.
235 self.assertEqualIgnoringWhitespace(
236 """EXISTS
237 (SELECT 1 FROM BugTag
238 WHERE BugTag.bug = BugTaskFlat.bug
239- AND BugTag.tag = 'eric')
240+ AND BugTag.tag = E'eric')
241 AND EXISTS
242 (SELECT 1 FROM BugTag
243 WHERE BugTag.bug = BugTaskFlat.bug
244- AND BugTag.tag = 'fred')
245+ AND BugTag.tag = E'fred')
246 AND NOT EXISTS
247 (SELECT 1 FROM BugTag
248 WHERE BugTag.bug = BugTaskFlat.bug)""",
249@@ -2159,7 +2159,7 @@
250 """EXISTS
251 (SELECT 1 FROM BugTag
252 WHERE BugTag.bug = BugTaskFlat.bug
253- AND BugTag.tag = 'fred')
254+ AND BugTag.tag = E'fred')
255 AND NOT EXISTS
256 (SELECT 1 FROM BugTag
257 WHERE BugTag.bug = BugTaskFlat.bug)""",
258
259=== modified file 'lib/lp/services/database/sqlbase.py'
260--- lib/lp/services/database/sqlbase.py 2013-06-21 02:39:33 +0000
261+++ lib/lp/services/database/sqlbase.py 2015-02-19 07:16:10 +0000
262@@ -283,18 +283,18 @@
263 >>> quote(1.0)
264 '1.0'
265 >>> quote("hello")
266- "'hello'"
267+ "E'hello'"
268 >>> quote("'hello'")
269- "'''hello'''"
270+ "E'\\'hello\\''"
271 >>> quote(r"\'hello")
272- "'\\\\''hello'"
273+ "E'\\\\\\'hello'"
274
275 Note that we need to receive a Unicode string back, because our
276 query will be a Unicode string (the entire query will be encoded
277 before sending across the wire to the database).
278
279 >>> quote(u"\N{TRADE MARK SIGN}")
280- u"'\u2122'"
281+ u"E'\u2122'"
282
283 Timezone handling is not implemented, since all timestamps should
284 be UTC anyway.
285@@ -348,18 +348,18 @@
286
287 >>> "SELECT * FROM mytable WHERE mycol LIKE '%%' || %s || '%%'" \
288 ... % quote_like('%')
289- "SELECT * FROM mytable WHERE mycol LIKE '%' || '\\\\%' || '%'"
290+ "SELECT * FROM mytable WHERE mycol LIKE '%' || E'\\\\%' || '%'"
291
292 Note that we need 2 backslashes to quote, as per the docs on
293 the LIKE operator. This is because, unless overridden, the LIKE
294 operator uses the same escape character as the SQL parser.
295
296 >>> quote_like('100%')
297- "'100\\\\%'"
298+ "E'100\\\\%'"
299 >>> quote_like('foobar_alpha1')
300- "'foobar\\\\_alpha1'"
301+ "E'foobar\\\\_alpha1'"
302 >>> quote_like('hello')
303- "'hello'"
304+ "E'hello'"
305
306 Only strings are supported by this method.
307
308@@ -371,7 +371,7 @@
309 """
310 if not isinstance(x, basestring):
311 raise TypeError('Not a string (%s)' % type(x))
312- return quote(x).replace('%', r'\\%').replace('_', r'\\_')
313+ return quote(x.replace('%', r'\%').replace('_', r'\_'))
314
315
316 def sqlvalues(*values, **kwvalues):
317@@ -393,7 +393,7 @@
318 >>> sqlvalues(1)
319 ('1',)
320 >>> sqlvalues(1, "bad ' string")
321- ('1', "'bad '' string'")
322+ ('1', "E'bad \\\\' string'")
323
324 You can also use it when using dict-style substitution.
325
326@@ -464,7 +464,7 @@
327 BugTask.importance = 999
328
329 >>> print convert_storm_clause_to_string(Bug.title == "foo'bar'")
330- Bug.title = 'foo''bar'''
331+ Bug.title = E'foo\\'bar\\''
332
333 >>> print convert_storm_clause_to_string(
334 ... Or(BugTask.importance == BugTaskImportance.UNKNOWN,
335@@ -475,7 +475,7 @@
336 ... And(Bug.title == 'foo', BugTask.bug == Bug.id,
337 ... Or(BugTask.importance == BugTaskImportance.UNKNOWN,
338 ... BugTask.importance == BugTaskImportance.HIGH)))
339- Bug.title = 'foo' AND BugTask.bug = Bug.id AND
340+ Bug.title = E'foo' AND BugTask.bug = Bug.id AND
341 (BugTask.importance = 999 OR BugTask.importance = 40)
342 """
343 state = State()
344
345=== modified file 'lib/lp/services/webapp/tests/test_batching.py'
346--- lib/lp/services/webapp/tests/test_batching.py 2012-12-26 01:32:19 +0000
347+++ lib/lp/services/webapp/tests/test_batching.py 2015-02-19 07:16:10 +0000
348@@ -420,7 +420,7 @@
349 limit_expression = range_factory.lessThanOrGreaterThanExpression(
350 expressions, limits)
351 self.assertEqual(
352- "(Person.id, Person.name) > (1, 'foo')",
353+ "(Person.id, Person.name) > (1, E'foo')",
354 compile(limit_expression))
355
356 def test_lessThanOrGreaterThanExpression__desc(self):
357@@ -433,7 +433,7 @@
358 limit_expression = range_factory.lessThanOrGreaterThanExpression(
359 expressions, limits)
360 self.assertEqual(
361- "(Person.id, Person.name) < (1, 'foo')",
362+ "(Person.id, Person.name) < (1, E'foo')",
363 compile(limit_expression))
364
365 def test_equalsExpressionsFromLimits(self):
366@@ -484,7 +484,7 @@
367 [where_clause] = range_factory.whereExpressions(
368 [Person.id, Person.name], [1, 'foo'])
369 self.assertEquals(
370- "(Person.id, Person.name) > (1, 'foo')", compile(where_clause))
371+ "(Person.id, Person.name) > (1, E'foo')", compile(where_clause))
372
373 def test_whereExpressions__two_sort_columns_desc_desc(self):
374 """If the descending sort columns c1, c2 and the memo values
375@@ -499,7 +499,7 @@
376 [where_clause] = range_factory.whereExpressions(
377 [Desc(Person.id), Desc(Person.name)], [1, 'foo'])
378 self.assertEquals(
379- "(Person.id, Person.name) < (1, 'foo')", compile(where_clause))
380+ "(Person.id, Person.name) < (1, E'foo')", compile(where_clause))
381
382 def test_whereExpressions__two_sort_columns_asc_desc(self):
383 """If the ascending sort column c1, the descending sort column
384@@ -517,7 +517,7 @@
385 [where_clause_1, where_clause_2] = range_factory.whereExpressions(
386 [Person.id, Desc(Person.name)], [1, 'foo'])
387 self.assertEquals(
388- "Person.id = ? AND ((Person.name) < ('foo'))",
389+ "Person.id = ? AND ((Person.name) < (E'foo'))",
390 compile(where_clause_1))
391 self.assertEquals('(Person.id) > (1)', compile(where_clause_2))
392
393
394=== modified file 'lib/sqlobject/__init__.py'
395--- lib/sqlobject/__init__.py 2012-02-06 08:10:12 +0000
396+++ lib/sqlobject/__init__.py 2015-02-19 07:16:10 +0000
397@@ -20,7 +20,7 @@
398
399 _sqlStringReplace = [
400 ('\\', '\\\\'),
401- ("'", "''"),
402+ ("'", "\\'"),
403 ('\000', '\\0'),
404 ('\b', '\\b'),
405 ('\n', '\\n'),
406@@ -42,7 +42,7 @@
407 elif isinstance(value, (str, unicode)):
408 for orig, repl in _sqlStringReplace:
409 value = value.replace(orig, repl)
410- return "'%s'" % value
411+ return "E'%s'" % value
412 elif isinstance(value, bool):
413 if value:
414 return "'t'"