[rql2sql] when using HAVING to by-pass rql limitation (not to filter on result of an aggregat function), we should emit SQL that doesn't use HAVING to avoid potential backend error because variables are not grouped. Closes #1061603. stable
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Thu, 17 Jun 2010 18:36:16 +0200
branchstable
changeset 5782 8ff48d1a319f
parent 5781 a3e60e0fb0f3
child 5783 c5ff8cd74758
[rql2sql] when using HAVING to by-pass rql limitation (not to filter on result of an aggregat function), we should emit SQL that doesn't use HAVING to avoid potential backend error because variables are not grouped. Closes #1061603.
hooks/metadata.py
schemas/bootstrap.py
server/sources/rql2sql.py
server/test/unittest_querier.py
--- a/hooks/metadata.py	Thu Jun 17 17:49:58 2010 +0200
+++ b/hooks/metadata.py	Thu Jun 17 18:36:16 2010 +0200
@@ -15,12 +15,10 @@
 #
 # You should have received a copy of the GNU Lesser General Public License along
 # with CubicWeb.  If not, see <http://www.gnu.org/licenses/>.
-"""Core hooks: set generic metadata
+"""Core hooks: set generic metadata"""
 
-"""
 __docformat__ = "restructuredtext en"
 
-
 from datetime import datetime
 
 from cubicweb.selectors import implements
--- a/schemas/bootstrap.py	Thu Jun 17 17:49:58 2010 +0200
+++ b/schemas/bootstrap.py	Thu Jun 17 18:36:16 2010 +0200
@@ -16,8 +16,8 @@
 # You should have received a copy of the GNU Lesser General Public License along
 # with CubicWeb.  If not, see <http://www.gnu.org/licenses/>.
 """core CubicWeb schema necessary for bootstrapping the actual instance's schema
+"""
 
-"""
 __docformat__ = "restructuredtext en"
 _ = unicode
 
--- a/server/sources/rql2sql.py	Thu Jun 17 17:49:58 2010 +0200
+++ b/server/sources/rql2sql.py	Thu Jun 17 18:36:16 2010 +0200
@@ -45,9 +45,8 @@
 and Informix.
 
 .. _Comparison of different SQL implementations: http://www.troels.arvin.dk/db/rdbms
-
+"""
 
-"""
 __docformat__ = "restructuredtext en"
 
 import threading
@@ -56,8 +55,8 @@
 
 from rql import BadRQLQuery, CoercionError
 from rql.stmts import Union, Select
-from rql.nodes import (SortTerm, VariableRef, Constant, Function, Not,
-                       Variable, ColumnAlias, Relation, SubQuery, Exists)
+from rql.nodes import (SortTerm, VariableRef, Constant, Function, Variable, Or,
+                       Not, Comparison, ColumnAlias, Relation, SubQuery, Exists)
 
 from cubicweb import QueryError
 from cubicweb.server.sqlutils import SQL_PREFIX
@@ -397,6 +396,49 @@
         self.restrictions = self._restr_stack.pop()
         return restrictions, self.actual_tables.pop()
 
+def extract_fake_having_terms(having):
+    """RQL's HAVING may be used to contains stuff that should go in the WHERE
+    clause of the SQL query, due to RQL grammar limitation. Split them...
+
+    Return a list nodes that can be ANDed with query's WHERE clause. Having
+    subtrees updated in place.
+    """
+    fakehaving = []
+    for subtree in having:
+        ors, tocheck = set(), []
+        for compnode in subtree.get_nodes(Comparison):
+            for fnode in compnode.get_nodes(Function):
+                if fnode.descr().aggregat:
+                    p = compnode.parent
+                    oor = None
+                    while not isinstance(p, Select):
+                        if isinstance(p, Or):
+                            oor = p
+                        p = p.parent
+                    if oor is not None:
+                        ors.add(oor)
+                    break
+            else:
+                tocheck.append(compnode)
+        # tocheck hold a set of comparison not implying an aggregat function
+        # put them in fakehaving if the don't share an Or node as ancestor
+        # with another comparison containing an aggregat function
+        for compnode in tocheck:
+            parents = set()
+            p = compnode.parent
+            oor = None
+            while not isinstance(p, Select):
+                if p in ors:
+                    break
+                if isinstance(p, Or):
+                    oor = p
+                p = p.parent
+            else:
+                node = oor or compnode
+                if not node in fakehaving:
+                    fakehaving.append(node)
+                    compnode.parent.remove(node)
+    return fakehaving
 
 class SQLGenerator(object):
     """
@@ -494,6 +536,7 @@
         sorts = select.orderby
         groups = select.groupby
         having = select.having
+        morerestr = extract_fake_having_terms(having)
         # remember selection, it may be changed and have to be restored
         origselection = select.selection[:]
         # check if the query will have union subquery, if it need sort term
@@ -545,7 +588,8 @@
         self._in_wrapping_query = False
         self._state = state
         try:
-            sql = self._solutions_sql(select, sols, distinct, needalias or needwrap)
+            sql = self._solutions_sql(select, morerestr, sols, distinct,
+                                      needalias or needwrap)
             # generate groups / having before wrapping query selection to
             # get correct column aliases
             self._in_wrapping_query = needwrap
@@ -608,13 +652,15 @@
                 except KeyError:
                     continue
 
-    def _solutions_sql(self, select, solutions, distinct, needalias):
+    def _solutions_sql(self, select, morerestr, solutions, distinct, needalias):
         sqls = []
         for solution in solutions:
             self._state.reset(solution)
             # visit restriction subtree
             if select.where is not None:
                 self._state.add_restriction(select.where.accept(self))
+            for restriction in morerestr:
+                self._state.add_restriction(restriction.accept(self))
             sql = [self._selection_sql(select.selection, distinct, needalias)]
             if self._state.restrictions:
                 sql.append('WHERE %s' % ' AND '.join(self._state.restrictions))
--- a/server/test/unittest_querier.py	Thu Jun 17 17:49:58 2010 +0200
+++ b/server/test/unittest_querier.py	Thu Jun 17 18:36:16 2010 +0200
@@ -511,6 +511,21 @@
         self.assertEquals(len(rset.rows), 1)
         self.assertEquals(rset.rows[0][0], self.ueid)
 
+    def test_select_having_non_aggregat_1(self):
+        rset = self.execute('Any L WHERE X login L, X creation_date CD '
+                            'HAVING YEAR(CD) = %s' % date.today().year)
+        self.assertListEquals(rset.rows,
+                              [[u'admin'],
+                               [u'anon']])
+
+    def test_select_having_non_aggregat_2(self):
+        rset = self.execute('Any L GROUPBY L WHERE X login L, X in_group G, '
+                            'X creation_date CD HAVING YEAR(CD) = %s OR COUNT(G) > 1'
+                            % date.today().year)
+        self.assertListEquals(rset.rows,
+                              [[u'admin'],
+                               [u'anon']])
+
     def test_select_complex_sort(self):
         """need sqlite including http://www.sqlite.org/cvstrac/tktview?tn=3773 fix"""
         rset = self.execute('Any X ORDERBY X,D LIMIT 5 WHERE X creation_date D')