Add backward compatibility with old related-key methods

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

product/ZSQLCatalog/ColumnMap.py

@@ -28,6 +28,7 @@
 ##############################################################################
 
 import re
+import itertools
 from zLOG import LOG, WARNING, INFO
 from interfaces.column_map import IColumnMap
 from zope.interface.verify import verifyClass
@@ -93,6 +94,8 @@ class ColumnMap(object):
     # Entries: column name
     self.column_ignore_set = set()
     self.join_table_set = set()
+    # BBB: Remove join_query_list and its uses when all RelatedKey methods have
+    # been converted to properly return each Join condition separately.
     self.join_query_list = []
     self.table_override_map = table_override_map or {}
     self.table_definition = PlaceHolderTableDefinition()
@@ -336,7 +339,6 @@ class ColumnMap(object):
         if related_key_definition is not None:
           join_query = sql_catalog.getSearchKey(column_name, 'RelatedKey').buildQuery(sql_catalog=sql_catalog, related_key_definition=related_key_definition)
           join_query.registerColumnMap(sql_catalog, self)
-          #self._addJoinQuery(join_query)
           join_query_to_build_list.append(join_query)
 
     # List all possible tables, with all used column for each
@@ -422,6 +424,8 @@ class ColumnMap(object):
     # (i.e. joins comming from 'sort_on', 'select_dict', etc.)
     for join_query in join_query_to_build_list:
       # XXX ugly use of inner attribute of join_query. Please Refactor:
+      # search_keys don't actually return SQLExpressions, but they add
+      # join definitions in the column_map
       join_query.search_key.buildSQLExpression(sql_catalog=sql_catalog,
                                                column_map=self,
                                                only_group_columns=False,
@@ -471,7 +475,11 @@ class ColumnMap(object):
     return self.table_alias_dict[(group, self.catalog_table_name)]
 
   def getTableAliasDict(self):
-    return self.table_map.copy()
+    if self.join_query_list:
+      # BBB: Using implicit joins
+      return self.table_map.copy()
+    else:
+      return None
 
   @profiler_decorator
   def resolveColumn(self, column, table_name, group=DEFAULT_GROUP_ID):
@@ -501,13 +509,26 @@ class ColumnMap(object):
   def getTableAlias(self, table_name, group=DEFAULT_GROUP_ID):
     return self.table_alias_dict[(group, table_name)]
 
-  def _addJoinQuery(self, query):
-    raise RuntimeError('Implicit Join Requested: %r. Please add an explicit '
-                       'join instead' % (query,))
+  def _addJoinQueryForColumn(self, column, query):
+    # BBB: This is a backward compatibility method that will be
+    # removed in the future, when all related key methods have been adapted
+    # to provide all Join conditions separately
+    if column in self.left_join_list:
+      raise RuntimeError('Left Join requested for column: %r, but rendered '
+                         'join query is not compatible and would result in an '
+                         'Implicit Inner Join:\n%s' % 
+                         (column, query,))
     self.join_query_list.append(query)
 
   def iterJoinQueryList(self):
-    return iter(self.join_query_list)
+    if self.join_query_list:
+      # BBB: one or more RelatedKey methods weren't converted, so we got
+      # queries for an implicit inner join. Return them, and all the other
+      # queries we were using in our table definition
+      return itertools.chain(self.join_query_list,
+                             self.table_definition.getJoinConditionQueryList())
+    return []
+    
 
   @profiler_decorator
   def _addJoinTable(self, table_name, group=DEFAULT_GROUP_ID):
@@ -592,25 +613,35 @@ class ColumnMap(object):
     return True
 
   def getTableDefinition(self):
-    if self._setMinimalTableDefinition():
-      return self.table_definition
-    return None
-
-  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.
+    if not self._setMinimalTableDefinition():
+      raise RuntimeError("ColumnMap.build() must be called first!")
+    if self.join_query_list:
+      # BBB: One of the RelatedKeys registered an implicit join, do
+      # not return a table definition, self.getTableAliasDict() should
+      # be used instead
+      return None
+    return self.table_definition
+
+  def addRelatedKeyJoin(self, column, 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
+    column 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: to fix TestERP5Catalog.test_52_QueryAndTableAlias, 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.
+    # even a catalog property/configuration/script.
+    #
+    # XXX 3: This is also the place where we could check if explicit
+    # table aliases should cause some of these table definitions to be
+    # collapsed into others.
     assert self._setMinimalTableDefinition()
-    Join = (related_key_id in self.left_join_list) and LeftJoin or InnerJoin
+    Join = (column in self.left_join_list) and LeftJoin or InnerJoin
     join_definition = Join(self.table_definition, right_side,
                            condition=condition)
     self.table_definition = join_definition

product/ZSQLCatalog/Query/EntireQuery.py

@@ -159,10 +159,15 @@ class EntireQuery(object):
       sql_expression_list = [self.query.asSQLExpression(sql_catalog,
                                                         column_map,
                                                         only_group_columns)]
+      append = sql_expression_list.append
+      for join_query in column_map.iterJoinQueryList():
+        append(join_query.asSQLExpression(sql_catalog,
+                                          column_map, 
+                                          only_group_columns))
       # generate join expression based on column_map.getJoinTableAliasList
-      #append = sql_expression_list.append
-      # for join_query in column_map.iterJoinQueryList():
-      #   append(join_query.asSQLExpression(sql_catalog, column_map, only_group_columns))
+      # XXX: This is now done by ColumnMap to its table_definition,
+      # during build()
+      #
       # join_table_list = column_map.getJoinTableAliasList()
       # if len(join_table_list):
       #   # XXX: Is there any special rule to observe when joining tables ?
@@ -178,10 +183,14 @@ class EntireQuery(object):
       #     where_pattern % (x, ) for x in join_table_list
       #   )))
       self.from_expression = column_map.getTableDefinition()
+      table_alias_dict = column_map.getTableAliasDict()
+      assert ((self.from_expression is None) !=
+              (table_alias_dict is None)), ("Got both a from_expression "
+                                            "and a table_alias_dict")
       self.sql_expression_list = sql_expression_list
     return SQLExpression(
       self,
-      table_alias_dict=None, # column_map.getTableAliasDict(),
+      table_alias_dict=table_alias_dict,
       from_expression=self.from_expression,
       order_by_list=self.order_by_list,
       group_by_list=self.group_by_list,

product/ZSQLCatalog/SQLExpression.py

@@ -131,9 +131,6 @@ class SQLExpression(object):
         raise ValueError, 'Unrecognized "limit" value: %r' % (limit, )
     else:
       self.limit = (limit, )
-    if from_expression is not None:
-      warnings.warn("Providing a 'from_expression' is deprecated.",
-                    DeprecationWarning)
     self.from_expression = from_expression
 
   @profiler_decorator

product/ZSQLCatalog/SearchKey/RelatedKey.py

@@ -39,10 +39,20 @@ from zope.interface import implements
 from Products.ZSQLCatalog.SQLCatalog import profiler_decorator
 from Products.ZSQLCatalog.TableDefinition import TableAlias, InnerJoin, LeftJoin
 
+from logging import getLogger
+log = getLogger(__name__)
+
 BACKWARD_COMPATIBILITY = True
 
 RELATED_QUERY_SEPARATOR = "\nAND -- related query separator\n"
 
+RELATED_KEY_MISMATCH_MESSAGE = "\
+A rendered related key must contain the same number of querying \
+conditions as the tables it relates, properly separated by \
+RELATED_QUERY_SEPARATOR. \n\
+Offending related key: %r, for column %r, table_alias_list: %r, \
+rendered_related_key: \n%s"
+
 class RelatedKey(SearchKey):
   """
     This SearchKey handles searches on virtual columns of RelatedKey type.
@@ -207,25 +217,9 @@ class RelatedKey(SearchKey):
       src__=1,
       **table_alias_dict)
     join_condition_list = rendered_related_key.split(RELATED_QUERY_SEPARATOR)
-    assert len(join_condition_list) == len(table_alias_list), """
-      A related key must return the same number of querying conditions as the 
-      tables it relates
-    """.strip()
-    
-    # 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_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.
+    # Previously the catalog separated join condition from the related query.
     # Example:
     #   ComplexQuery(Query(title="foo"),
     #                Query(subordination_title="bar")
@@ -241,19 +235,53 @@ class RelatedKey(SearchKey):
     # This was done on purpose, because doing otherwise gives very poor
     # performances (on a simple data set, similar query can take *minutes* to
     # execute - as of MySQL 5.x).
-    # Doing the same way as the former catalog is required for backward
-    # compatibility, until a decent alternative is found (like spliting the
-    # "OR" expression into ensemblist operations at query level).
-    # Note that doing this has a side effect on result list, as objects
-    # lacking a relation will never appear in the result.
-    if BACKWARD_COMPATIBILITY:
-      # XXX: Calling a private-ish method on column_map.
-      # This should never happen. It should be removed as soon as an
-      # alternative exists.
-      column_map._addJoinQuery(SQLQuery(rendered_related_key))
-      return None
+    #
+    # Because of this, we never return an SQLExpression here, as it
+    # would mix join definition with column condition in the body of
+    # the WHERE clause. Instead we explicitly define a Join to the
+    # catalog. The ColumnMap defines whether this is an Inner Join or
+    # a Left Outer Join. Notice that if an Inner Join is decided,
+    # objects lacking a relationship will never appear in the result.
+
+    if len(join_condition_list) == len(table_alias_list):
+      # Good! we got a compatible method that splits the join
+      # conditions according to the related tables.
+      #
+      # Add a join on this related key, based on the chain of
+      # inner-joins of the related key tables.
+      query_table_join_condition = join_condition_list.pop()
+      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)
     else:
-      return SQLExpression(self, where_expression=rendered_related_key)
+      # Method did not render the related key condition with the
+      # appropriate separators so we could split it
+
+      # XXX: Can we try to parse rendered_related_key to select which
+      # conditions go with each table? Maybe we could still use
+      # explicit joins this way...
+
+      msg = RELATED_KEY_MISMATCH_MESSAGE % (self.related_key_id,
+                                            self.column,
+                                            table_alias_list,
+                                            rendered_related_key)
+      if BACKWARD_COMPATIBILITY:
+        # BBB: remove this branch of the condition, and the above
+        # constant, when all zsql_methods have been adapted to return
+        # the join queries properly separated by the
+        # RELATED_QUERY_SEPARATOR.
+
+        # The rendered related key doesn't have the separators for each
+        # joined table, so we revert to doing implicit inner joins:
+        log.warning(msg + "\n\nAdding an Implicit Join Condition...")
+        column_map._addJoinQueryForColumn(self.column,
+                                          SQLQuery(rendered_related_key))
+      else:
+        raise RuntimeError(msg)
+    return None
 
 verifyClass(IRelatedKey, RelatedKey)
 

product/ZSQLCatalog/TableDefinition.py

@@ -32,7 +32,7 @@
 
 SQL_SELECT_ALIAS_FORMAT = '%s AS `%s`'
 
-TESTDEBUG = False
+from Products.ZSQLCatalog.Query.SQLQuery import SQLQuery
 
 def escapeTable(table):
   return "`%s`" % table.replace('`', r'\`')
@@ -63,6 +63,20 @@ class TableDefinition(object):
   def render(self):
     raise NotImplementedError('should be implemented by subclasses')
 
+  def getJoinConditionQueryList(self):
+    """Return a list of SQLQuery objects containing all conditions
+    used in this table definition.
+
+    This is a deprecated method that is here only to accomodate the
+    fact that not all RelatedKey methods have been migrated.
+    """
+    query_list = []
+    self._extendJoinConditionQueryList(query_list)
+    return query_list
+
+  def _extendJoinConditionQueryList(self, query_list):
+    raise NotImplementedError('should be implemented by subclasses')
+
 class PlaceHolderTableDefinition(TableDefinition):
   """Table Definition that simply holds an inner table definition and
   delegates to it the rendering.
@@ -89,6 +103,9 @@ class PlaceHolderTableDefinition(TableDefinition):
     assert self.table_definition is not None, "table definition wasn't set"
     return self.table_definition.render()
 
+  def _extendJoinConditionQueryList(self, query_list):
+    assert self.table_definition is not None, "table definition wasn't set"
+    return self.table_definition._extendJoinConditionQueryList(query_list)
 
 class TableAlias(TableDefinition):
   """Definition of a table alias as a FROM expression"""
@@ -118,6 +135,9 @@ class TableAlias(TableDefinition):
   def __repr__(self):
     return '<%s %r AS %r>' % (self.__class__.__name__, self.table, self.alias)
 
+  def _extendJoinConditionQueryList(self, query_list):
+    pass
+
 JOIN_FORMAT = """
   (
     %(left)s 
@@ -161,6 +181,11 @@ class InnerJoin(TableDefinition):
                                         self.right_tabledef,
                                         self.condition)
 
+  def _extendJoinConditionQueryList(self, query_list):
+    self.left_tabledef._extendJoinConditionQueryList(query_list)
+    self.right_tabledef._extendJoinConditionQueryList(query_list)
+    query_list.append(SQLQuery(self.condition))
+
 class LeftJoin(InnerJoin):
   """Definition of a left-join as a FROM expression"""