Un-hardcode the use of LeftJoin. Make it dependent on the presence of a...

Un-hardcode the use of LeftJoin. Make it dependent on the presence of a left_join_list parameter to the catalog that lists the names of virtual_columns whose related table definitions should be left-joined, insteand of inner-joined, to the catalog.

git-svn-id: https://svn.erp5.org/repos/public/erp5/sandbox/catalog_join@42277 20353a03-c40f-0410-a6d1-a30d3c3de9de

product/ZSQLCatalog/ColumnMap.py

@@ -36,7 +36,8 @@ from Products.ZSQLCatalog.interfaces.column_map import IColumnMap
 from Products.ZSQLCatalog.SQLCatalog import profiler_decorator
 from Products.ZSQLCatalog.TableDefinition import (PlaceHolderTableDefinition,
                                                   TableAlias,
-                                                  InnerJoin)
+                                                  InnerJoin,
+                                                  LeftJoin)
 
 DEFAULT_GROUP_ID = None
 
@@ -55,7 +56,10 @@ class ColumnMap(object):
   implements(IColumnMap)
 
   @profiler_decorator
-  def __init__(self, catalog_table_name=None, table_override_map=None):
+  def __init__(self,
+               catalog_table_name=None,
+               table_override_map=None,
+               left_join_list=None):
     self.catalog_table_name = catalog_table_name
     # Key: group
     # Value: set of column names
@@ -89,18 +93,19 @@ class ColumnMap(object):
     # Entries: column name
     self.column_ignore_set = set()
     self.join_table_set = set()
-    self.straight_join_table_list = []
-    self.left_join_table_list = []
     self.join_query_list = []
     self.table_override_map = table_override_map or {}
     self.table_definition = PlaceHolderTableDefinition()
     # We need to keep track of the original definition to do inner joins on it
     self._inner_table_definition = self.table_definition
+    self.left_join_list = left_join_list
 
   @profiler_decorator
   def registerColumn(self, raw_column, group=DEFAULT_GROUP_ID, simple_query=None):
     assert ' as ' not in raw_column.lower()
     # Sanitize input: extract column from raw column (might contain COUNT, ...).
+    # XXX This is not enough to parse something like: 
+    # GROUP_CONCAT(DISTINCT foo ORDER BY bar)
     if '(' in raw_column:
       function, column = raw_column.split('(')
       column = column.strip()
@@ -529,14 +534,6 @@ class ColumnMap(object):
     return [self.getTableAlias(table_name, group=group)
             for (group, table_name) in self.join_table_set]
 
-  def getStraightJoinTableList(self):
-    # XXX this function is unused and should be removed
-    return self.straight_join_table_list[:]
-
-  def getLeftJoinTableList(self):
-    # XXX this function is unused and should be removed
-    return self.left_join_table_list[:]
-
   def _getTableOverride(self, table_name):
     # self.table_override_map is a dictionary mapping table names to
     # strings containing aliases of arbitrary table definitions
@@ -545,9 +542,12 @@ class ColumnMap(object):
     table_override_w_alias = self.table_override_map.get(table_name)
     if table_override_w_alias is None:
       return table_name
-    # XXX move the table aliasing cleanup to EntireQuery class, so we
-    # don't need SQL syntax knowledge in ColumnMap.  Normalise the AS
-    # sql keyword to remove the last aliasing in the string if present. E.g.:
+    # XXX move the cleanup of table alias overrides to EntireQuery
+    # class or ZSQLCatalog, so we don't need SQL syntax knowledge in
+    # ColumnMap. 
+    #
+    # Normalise the AS sql keyword to remove the last
+    # aliasing in the string if present. E.g.:
     # 
     # '(SELECT sub_catalog.* 
     #   FROM catalog AS sub_catalog
@@ -596,17 +596,23 @@ class ColumnMap(object):
       return self.table_definition
     return None
 
-  def addJoin(self, join_definition, condition):
-    """ Replaces the current table_definition with a new one, assuming
-    it is a join definition, and replacing it's left side with the
-    previous table definition.
-
-    Effectively, this method wraps the current table definition within
-    the received join_definition.
+  def addRelatedKeyJoin(self, related_key_id, right_side, condition):
+    """ Wraps the current table_definition in the left-side of a new join.
+    Use an InnerJoin or a LeftJoin depending on whether the related_key_id is
+    in the left_join_list or not.
     """
+    # XXX: to fix TestERP5Catalog.test_56_CreateUidDuringClearCatalog,
+    # Create here a list of joins and try to merge each new entry into
+    # one of the pre-existing entries by comparing their right-sides.
+    # XXX 2: This is the place were we could do ordering of inner and left
+    # joins so as to get better performance. For instance, a quick win is to
+    # add all inner-joins first, and all left-joins later. We could also decide
+    # on the order of left-joins based on the order of self.left_join_list or
+    # even a catalog property/configuration.
     assert self._setMinimalTableDefinition()
-    assert join_definition.left_tabledef is None, join_definition.left_tabledef
-    join_definition.left_tabledef = self.table_definition
+    Join = (related_key_id in self.left_join_list) and LeftJoin or InnerJoin
+    join_definition = Join(self.table_definition, right_side,
+                           condition=condition)
     self.table_definition = join_definition
 
   # def getFinalTableDefinition(self):

product/ZSQLCatalog/Query/EntireQuery.py

@@ -54,15 +54,22 @@ class EntireQuery(object):
   column_map = None
 
   @profiler_decorator
-  def __init__(self, query, order_by_list=(), group_by_list=(),
-               select_dict=None, limit=None, catalog_table_name=None,
-               extra_column_list=(), from_expression=None,
+  def __init__(self, query,
+               order_by_list=(),
+               group_by_list=(),
+               select_dict=None,
+               left_join_list=(),
+               limit=None,
+               catalog_table_name=None,
+               extra_column_list=(),
+               from_expression=None,
                order_by_override_list=None):
     self.query = query
     self.order_by_list = list(order_by_list)
     self.order_by_override_set = frozenset(order_by_override_list)
     self.group_by_list = list(group_by_list)
     self.select_dict = defaultDict(select_dict)
+    self.left_join_list = left_join_list
     self.limit = limit
     self.catalog_table_name = catalog_table_name
     self.extra_column_list = list(extra_column_list)
@@ -79,7 +86,9 @@ class EntireQuery(object):
       # method or do it here ?
       # Column Map was not built yet, do it.
       column_map = ColumnMap(catalog_table_name=self.catalog_table_name,
-                             table_override_map=self.from_expression)
+                             table_override_map=self.from_expression,
+                             left_join_list=self.left_join_list,
+                            )
       self.column_map = column_map
       for extra_column in self.extra_column_list:
         table, column = extra_column.replace('`', '').split('.')

product/ZSQLCatalog/SQLCatalog.py

@@ -2296,6 +2296,8 @@ class Catalog(Folder,
         select_dict = None
     elif isinstance(select_dict, (list, tuple)):
       select_dict = dict([(x, None) for x in select_dict])
+    # Handle left_join_list
+    left_join_list = kw.pop('left_join_list', ())
     # Handle order_by_list
     order_by_list = kw.pop('order_by_list', None)
     sort_on = kw.pop('sort_on', None)
@@ -2333,6 +2335,7 @@ class Catalog(Folder,
       order_by_override_list=order_by_override_list,
       group_by_list=group_by_list,
       select_dict=select_dict,
+      left_join_list=left_join_list,
       limit=limit,
       catalog_table_name=query_table,
       extra_column_list=extra_column_list,

product/ZSQLCatalog/SearchKey/RelatedKey.py

@@ -215,14 +215,15 @@ class RelatedKey(SearchKey):
     # add a left join on this related key, based on the inner-join of the
     # related key tables.
     query_table_join_condition = join_condition_list.pop()
-    right = self.stitchJoinDefinition(table_alias_list,
-                                      join_condition_list,
-                                      column_map)
-    table_def = LeftJoin(None,
-                         right,
-                         condition=query_table_join_condition)
-    column_map.addJoin(table_def, condition=query_table_join_condition)
-    return None # XXX decide what to do with the comment below
+    right_side = self.stitchJoinDefinition(table_alias_list,
+                                           join_condition_list,
+                                           column_map)
+    column_map.addRelatedKeyJoin(self.column,
+                                 right_side=right_side,
+                                 condition=query_table_join_condition)
+    return None
+    # XXX decide what to do with the comment below and the rest of the code.
+    # possibly we need to move all the code above into .registerColumnMap()
     # Important:
     # Former catalog separated join condition from related query.
     # Example:

product/ZSQLCatalog/interfaces/column_map.py

@@ -284,18 +284,3 @@ class IColumnMap(Interface):
       Return a copy of the table alias list for tables requiring a join with
       catalog table.
     """
-
-  def getStraightJoinTableList():
-    """
-      Returns the list of tables used this search and which
-      need to be joined with the main table using explicit
-      indices.
-    """
-
-  def getLeftJoinTableList():
-    """
-      Returns the list of tables used this search and which
-      need to be LEFT joined with the main table using explicit
-      indices.
-    """
-