launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17901
[Merge] lp:~wgrant/launchpad/sqlbase-E-strings into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/sqlbase-E-strings into lp:launchpad.
Commit message:
Port sqlvalues and co to postgres E'strings', rather than relying on standard_conforming_strings=off.
Requested reviews:
Stuart Bishop (stub)
Related bugs:
Bug #90809 in Launchpad itself: "Launchpad should run with standard_conforming_strings=on in postgresql.conf"
https://bugs.launchpad.net/launchpad/+bug/90809
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/sqlbase-E-strings/+merge/250263
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.
--
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/bugs/model/tests/test_bugtasksearch.py'
--- lib/lp/bugs/model/tests/test_bugtasksearch.py 2015-01-29 16:28:30 +0000
+++ lib/lp/bugs/model/tests/test_bugtasksearch.py 2015-02-19 07:16:10 +0000
@@ -1853,7 +1853,7 @@
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag IN ('fred'))""")
+ AND BugTag.tag IN (E'fred'))""")
self.assertEqualIgnoringWhitespace(
expected_query,
self.searchClause(any(u'fred')))
@@ -1865,7 +1865,7 @@
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'fred')""")
+ AND BugTag.tag = E'fred')""")
self.assertEqualIgnoringWhitespace(
expected_query,
self.searchClause(all(u'fred')))
@@ -1877,7 +1877,7 @@
"""NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'fred')""")
+ AND BugTag.tag = E'fred')""")
self.assertEqualIgnoringWhitespace(
expected_query,
self.searchClause(any(u'-fred')))
@@ -1889,7 +1889,7 @@
"""NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag IN ('fred'))""")
+ AND BugTag.tag IN (E'fred'))""")
self.assertEqualIgnoringWhitespace(
expected_query,
self.searchClause(all(u'-fred')))
@@ -1929,7 +1929,7 @@
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag IN ('bob', 'fred'))""",
+ AND BugTag.tag IN (E'bob', E'fred'))""",
self.searchClause(any(u'fred', u'bob')))
# In an `any` query, a positive wildcard is dominant over
# other positive tags because "bugs with one or more tags" is
@@ -1948,11 +1948,11 @@
(EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'bob')
+ AND BugTag.tag = E'bob')
AND EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'fred'))""",
+ AND BugTag.tag = E'fred'))""",
self.searchClause(any(u'-fred', u'-bob')))
# In an `any` query, a negative wildcard is superfluous in the
# presence of other negative tags because "bugs without a
@@ -1961,7 +1961,7 @@
"""NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'fred')""",
+ AND BugTag.tag = E'fred')""",
self.searchClause(any(u'-fred', u'-*')))
def test_multiple_tag_presence_all(self):
@@ -1971,11 +1971,11 @@
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'bob')
+ AND BugTag.tag = E'bob')
AND EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'fred')""",
+ AND BugTag.tag = E'fred')""",
self.searchClause(all(u'fred', u'bob')))
# In an `all` query, a positive wildcard is superfluous in the
# presence of other positive tags because "bugs with a
@@ -1985,7 +1985,7 @@
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'fred')""",
+ AND BugTag.tag = E'fred')""",
self.searchClause(all(u'fred', u'*')))
def test_multiple_tag_absence_all(self):
@@ -1995,7 +1995,7 @@
"""NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag IN ('bob', 'fred'))""",
+ AND BugTag.tag IN (E'bob', E'fred'))""",
self.searchClause(all(u'-fred', u'-bob')))
# In an `all` query, a negative wildcard is dominant over
# other negative tags because "bugs without any tags" is a
@@ -2015,26 +2015,26 @@
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag IN ('fred'))
+ AND BugTag.tag IN (E'fred'))
OR NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'bob')""",
+ AND BugTag.tag = E'bob')""",
self.searchClause(any(u'fred', u'-bob')))
self.assertEqualIgnoringWhitespace(
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag IN ('eric', 'fred'))
+ AND BugTag.tag IN (E'eric', E'fred'))
OR NOT
(EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'bob')
+ AND BugTag.tag = E'bob')
AND EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'harry'))""",
+ AND BugTag.tag = E'harry'))""",
self.searchClause(any(u'fred', u'-bob', u'eric', u'-harry')))
# The positive wildcard is dominant over other positive tags.
self.assertEqualIgnoringWhitespace(
@@ -2045,11 +2045,11 @@
(EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'bob')
+ AND BugTag.tag = E'bob')
AND EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'harry'))""",
+ AND BugTag.tag = E'harry'))""",
self.searchClause(any(u'fred', u'-bob', u'*', u'-harry')))
# The negative wildcard is superfluous in the presence of
# other negative tags.
@@ -2057,11 +2057,11 @@
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag IN ('eric', 'fred'))
+ AND BugTag.tag IN (E'eric', E'fred'))
OR NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'bob')""",
+ AND BugTag.tag = E'bob')""",
self.searchClause(any(u'fred', u'-bob', u'eric', u'-*')))
# The negative wildcard is not superfluous in the absence of
# other negative tags.
@@ -2069,7 +2069,7 @@
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag IN ('eric', 'fred'))
+ AND BugTag.tag IN (E'eric', E'fred'))
OR NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug)""",
@@ -2084,7 +2084,7 @@
OR NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'harry')""",
+ AND BugTag.tag = E'harry')""",
self.searchClause(any(u'fred', u'-*', u'*', u'-harry')))
def test_mixed_tags_all(self):
@@ -2095,25 +2095,25 @@
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'fred')
+ AND BugTag.tag = E'fred')
AND NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag IN ('bob'))""",
+ AND BugTag.tag IN (E'bob'))""",
self.searchClause(all(u'fred', u'-bob')))
self.assertEqualIgnoringWhitespace(
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'eric')
+ AND BugTag.tag = E'eric')
AND EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'fred')
+ AND BugTag.tag = E'fred')
AND NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag IN ('bob', 'harry'))""",
+ AND BugTag.tag IN (E'bob', E'harry'))""",
self.searchClause(all(u'fred', u'-bob', u'eric', u'-harry')))
# The positive wildcard is superfluous in the presence of
# other positive tags.
@@ -2121,11 +2121,11 @@
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'fred')
+ AND BugTag.tag = E'fred')
AND NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag IN ('bob', 'harry'))""",
+ AND BugTag.tag IN (E'bob', E'harry'))""",
self.searchClause(all(u'fred', u'-bob', u'*', u'-harry')))
# The positive wildcard is not superfluous in the absence of
# other positive tags.
@@ -2136,18 +2136,18 @@
AND NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag IN ('bob', 'harry'))""",
+ AND BugTag.tag IN (E'bob', E'harry'))""",
self.searchClause(all(u'-bob', u'*', u'-harry')))
# The negative wildcard is dominant over other negative tags.
self.assertEqualIgnoringWhitespace(
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'eric')
+ AND BugTag.tag = E'eric')
AND EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'fred')
+ AND BugTag.tag = E'fred')
AND NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug)""",
@@ -2159,7 +2159,7 @@
"""EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug
- AND BugTag.tag = 'fred')
+ AND BugTag.tag = E'fred')
AND NOT EXISTS
(SELECT 1 FROM BugTag
WHERE BugTag.bug = BugTaskFlat.bug)""",
=== modified file 'lib/lp/services/database/sqlbase.py'
--- lib/lp/services/database/sqlbase.py 2013-06-21 02:39:33 +0000
+++ lib/lp/services/database/sqlbase.py 2015-02-19 07:16:10 +0000
@@ -283,18 +283,18 @@
>>> quote(1.0)
'1.0'
>>> quote("hello")
- "'hello'"
+ "E'hello'"
>>> quote("'hello'")
- "'''hello'''"
+ "E'\\'hello\\''"
>>> quote(r"\'hello")
- "'\\\\''hello'"
+ "E'\\\\\\'hello'"
Note that we need to receive a Unicode string back, because our
query will be a Unicode string (the entire query will be encoded
before sending across the wire to the database).
>>> quote(u"\N{TRADE MARK SIGN}")
- u"'\u2122'"
+ u"E'\u2122'"
Timezone handling is not implemented, since all timestamps should
be UTC anyway.
@@ -348,18 +348,18 @@
>>> "SELECT * FROM mytable WHERE mycol LIKE '%%' || %s || '%%'" \
... % quote_like('%')
- "SELECT * FROM mytable WHERE mycol LIKE '%' || '\\\\%' || '%'"
+ "SELECT * FROM mytable WHERE mycol LIKE '%' || E'\\\\%' || '%'"
Note that we need 2 backslashes to quote, as per the docs on
the LIKE operator. This is because, unless overridden, the LIKE
operator uses the same escape character as the SQL parser.
>>> quote_like('100%')
- "'100\\\\%'"
+ "E'100\\\\%'"
>>> quote_like('foobar_alpha1')
- "'foobar\\\\_alpha1'"
+ "E'foobar\\\\_alpha1'"
>>> quote_like('hello')
- "'hello'"
+ "E'hello'"
Only strings are supported by this method.
@@ -371,7 +371,7 @@
"""
if not isinstance(x, basestring):
raise TypeError('Not a string (%s)' % type(x))
- return quote(x).replace('%', r'\\%').replace('_', r'\\_')
+ return quote(x.replace('%', r'\%').replace('_', r'\_'))
def sqlvalues(*values, **kwvalues):
@@ -393,7 +393,7 @@
>>> sqlvalues(1)
('1',)
>>> sqlvalues(1, "bad ' string")
- ('1', "'bad '' string'")
+ ('1', "E'bad \\\\' string'")
You can also use it when using dict-style substitution.
@@ -464,7 +464,7 @@
BugTask.importance = 999
>>> print convert_storm_clause_to_string(Bug.title == "foo'bar'")
- Bug.title = 'foo''bar'''
+ Bug.title = E'foo\\'bar\\''
>>> print convert_storm_clause_to_string(
... Or(BugTask.importance == BugTaskImportance.UNKNOWN,
@@ -475,7 +475,7 @@
... And(Bug.title == 'foo', BugTask.bug == Bug.id,
... Or(BugTask.importance == BugTaskImportance.UNKNOWN,
... BugTask.importance == BugTaskImportance.HIGH)))
- Bug.title = 'foo' AND BugTask.bug = Bug.id AND
+ Bug.title = E'foo' AND BugTask.bug = Bug.id AND
(BugTask.importance = 999 OR BugTask.importance = 40)
"""
state = State()
=== modified file 'lib/lp/services/webapp/tests/test_batching.py'
--- lib/lp/services/webapp/tests/test_batching.py 2012-12-26 01:32:19 +0000
+++ lib/lp/services/webapp/tests/test_batching.py 2015-02-19 07:16:10 +0000
@@ -420,7 +420,7 @@
limit_expression = range_factory.lessThanOrGreaterThanExpression(
expressions, limits)
self.assertEqual(
- "(Person.id, Person.name) > (1, 'foo')",
+ "(Person.id, Person.name) > (1, E'foo')",
compile(limit_expression))
def test_lessThanOrGreaterThanExpression__desc(self):
@@ -433,7 +433,7 @@
limit_expression = range_factory.lessThanOrGreaterThanExpression(
expressions, limits)
self.assertEqual(
- "(Person.id, Person.name) < (1, 'foo')",
+ "(Person.id, Person.name) < (1, E'foo')",
compile(limit_expression))
def test_equalsExpressionsFromLimits(self):
@@ -484,7 +484,7 @@
[where_clause] = range_factory.whereExpressions(
[Person.id, Person.name], [1, 'foo'])
self.assertEquals(
- "(Person.id, Person.name) > (1, 'foo')", compile(where_clause))
+ "(Person.id, Person.name) > (1, E'foo')", compile(where_clause))
def test_whereExpressions__two_sort_columns_desc_desc(self):
"""If the descending sort columns c1, c2 and the memo values
@@ -499,7 +499,7 @@
[where_clause] = range_factory.whereExpressions(
[Desc(Person.id), Desc(Person.name)], [1, 'foo'])
self.assertEquals(
- "(Person.id, Person.name) < (1, 'foo')", compile(where_clause))
+ "(Person.id, Person.name) < (1, E'foo')", compile(where_clause))
def test_whereExpressions__two_sort_columns_asc_desc(self):
"""If the ascending sort column c1, the descending sort column
@@ -517,7 +517,7 @@
[where_clause_1, where_clause_2] = range_factory.whereExpressions(
[Person.id, Desc(Person.name)], [1, 'foo'])
self.assertEquals(
- "Person.id = ? AND ((Person.name) < ('foo'))",
+ "Person.id = ? AND ((Person.name) < (E'foo'))",
compile(where_clause_1))
self.assertEquals('(Person.id) > (1)', compile(where_clause_2))
=== modified file 'lib/sqlobject/__init__.py'
--- lib/sqlobject/__init__.py 2012-02-06 08:10:12 +0000
+++ lib/sqlobject/__init__.py 2015-02-19 07:16:10 +0000
@@ -20,7 +20,7 @@
_sqlStringReplace = [
('\\', '\\\\'),
- ("'", "''"),
+ ("'", "\\'"),
('\000', '\\0'),
('\b', '\\b'),
('\n', '\\n'),
@@ -42,7 +42,7 @@
elif isinstance(value, (str, unicode)):
for orig, repl in _sqlStringReplace:
value = value.replace(orig, repl)
- return "'%s'" % value
+ return "E'%s'" % value
elif isinstance(value, bool):
if value:
return "'t'"
Follow ups
