Commit 6bbe51d8 authored by Vincent Pelletier's avatar Vincent Pelletier

ZSQLCatalog: Stop hiding possible duplicates in z_getitem_by_{path,uid}

LIMIT hides duplicates. We want to know if we ever violate the
soft-constraint of path unicity in catalog, so stop setting a LIMIT.
Also, for uid lookup, LIMIT is meaningless as this is ha hard unicity
constraint (must be enforced by relational database for ERP5 to work).

Also, simplify both the DTML and the SQL by having fewer ways to be
invoked (backward-compatible).
parent 667c91df
SELECT <dtml-if uid_only>uid<dtml-else>uid,path</dtml-if> from catalog SELECT uid, path FROM catalog WHERE <dtml-sqltest column="path" expr="path_list" op=eq type="string" multiple>
WHERE
<dtml-if path_list>
path IN (<dtml-in path_list><dtml-sqlvar sequence-item type="string">
<dtml-if sequence-end><dtml-else>,</dtml-if></dtml-in>)
LIMIT <dtml-sqlvar expr="len(path_list)" type="int">
<dtml-else>
<dtml-sqltest path op=eq type="string">
LIMIT 1
</dtml-if>
...@@ -14,9 +14,7 @@ ...@@ -14,9 +14,7 @@
</item> </item>
<item> <item>
<key> <string>arguments_src</string> </key> <key> <string>arguments_src</string> </key>
<value> <string>path\r\n <value> <string>path_list</string> </value>
path_list\r\n
uid_only</string> </value>
</item> </item>
<item> <item>
<key> <string>cache_time_</string> </key> <key> <string>cache_time_</string> </key>
......
SELECT <dtml-if path_only>path<dtml-else>uid,path</dtml-if> from catalog SELECT uid, path FROM catalog WHERE <dtml-sqltest column="uid" expr="uid_list" op=eq type="int" multiple>
WHERE
<dtml-if uid_list>
uid IN (<dtml-in uid_list><dtml-sqlvar sequence-item type="int">
<dtml-if sequence-end><dtml-else>,</dtml-if></dtml-in>)
LIMIT <dtml-sqlvar expr="len(uid_list)" type="int">
<dtml-else>
<dtml-sqltest uid op=eq type="int">
LIMIT 1
</dtml-if>
...@@ -14,9 +14,7 @@ ...@@ -14,9 +14,7 @@
</item> </item>
<item> <item>
<key> <string>arguments_src</string> </key> <key> <string>arguments_src</string> </key>
<value> <string>uid\r\n <value> <string>uid_list</string> </value>
uid_list\r\n
path_only</string> </value>
</item> </item>
<item> <item>
<key> <string>cache_time_</string> </key> <key> <string>cache_time_</string> </key>
......
...@@ -886,11 +886,11 @@ class Catalog(Folder, ...@@ -886,11 +886,11 @@ class Catalog(Folder,
# It could also have a performance impact for traversals to objects in # It could also have a performance impact for traversals to objects in
# the acquisition context on Zope 2.12 even when it didn't raise a weird # the acquisition context on Zope 2.12 even when it didn't raise a weird
# error. # error.
method = self._getOb(self.getSqlGetitemByUid()) search_result = self._getOb(self.getSqlGetitemByUid())(uid_list=[uid])
search_result = method(uid = uid) if search_result:
if len(search_result) > 0: result, = search_result
return search_result[0] return result
raise KeyError, uid raise KeyError(uid)
security.declarePrivate('editSchema') security.declarePrivate('editSchema')
def editSchema(self, names_list): def editSchema(self, names_list):
...@@ -1369,7 +1369,7 @@ class Catalog(Folder, ...@@ -1369,7 +1369,7 @@ class Catalog(Folder,
# exceptional case. # exceptional case.
error_message = 'uid %r is shared between %r (catalog) and %r (being indexed) ! This can break relations' % ( error_message = 'uid %r is shared between %r (catalog) and %r (being indexed) ! This can break relations' % (
uid, uid,
uid_path_dict[uid], catalog_path,
path, path,
) )
if self.sql_catalog_raise_error_on_uid_check: if self.sql_catalog_raise_error_on_uid_check:
...@@ -1605,9 +1605,9 @@ class Catalog(Folder, ...@@ -1605,9 +1605,9 @@ class Catalog(Folder,
for x in self._getOb( for x in self._getOb(
self.getSqlGetitemByPath() self.getSqlGetitemByPath()
)( )(
path=None, path=None, # BBB
path_list=path_list, path_list=path_list,
uid_only=False, uid_only=False, # BBB
) )
} }
...@@ -1619,9 +1619,9 @@ class Catalog(Folder, ...@@ -1619,9 +1619,9 @@ class Catalog(Folder,
for x in self._getOb( for x in self._getOb(
self.getSqlGetitemByUid() self.getSqlGetitemByUid()
)( )(
uid=None, uid=None, # BBB
uid_list=uid_list, uid_list=uid_list,
path_only=False, path_only=False, # BBB
) )
} }
......
...@@ -58,7 +58,7 @@ class TestSQLCatalog(unittest.TestCase): ...@@ -58,7 +58,7 @@ class TestSQLCatalog(unittest.TestCase):
# test that our method actually gets called while looking records up by # test that our method actually gets called while looking records up by
# uid by raising our own exception # uid by raising our own exception
self._catalog.sql_getitem_by_uid = 'z_dummy_lookup_method' self._catalog.sql_getitem_by_uid = 'z_dummy_lookup_method'
def z_dummy_lookup_method(uid): def z_dummy_lookup_method(uid_list):
raise MyError('foo') raise MyError('foo')
self._catalog.z_dummy_lookup_method = z_dummy_lookup_method self._catalog.z_dummy_lookup_method = z_dummy_lookup_method
self.assertRaises(MyError, self._catalog.getRecordForUid, 1) self.assertRaises(MyError, self._catalog.getRecordForUid, 1)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment