# HG changeset patch # User Sylvain Thénault # Date 1311163803 -7200 # Node ID f31f9882c90f269f46c19686fbc6e15b100b9217 # Parent d911a73ac8c50bf862107eea0360ebba0e0c3224 [rql2sql] closes #1832859: fake HAVING terms w/ EXISTS terms there are cases where we want to use the HAVING trick on terms that should belong to an EXISTS subquery. Currently , this causes bad sql. We should fix that until we've a proper rql grammar avoiding the need for this trick. Notice at some point, we may want **actuall** HAVING clauses and GROUPBY used in EXISTS subquery, but that's another story. diff -r d911a73ac8c5 -r f31f9882c90f server/sources/rql2sql.py --- a/server/sources/rql2sql.py Wed Jul 20 14:10:02 2011 +0200 +++ b/server/sources/rql2sql.py Wed Jul 20 14:10:03 2011 +0200 @@ -56,6 +56,7 @@ from logilab.database import FunctionDescr, SQL_FUNCTIONS_REGISTRY from rql import BadRQLQuery, CoercionError +from rql.utils import common_parent from rql.stmts import Union, Select from rql.nodes import (SortTerm, VariableRef, Constant, Function, Variable, Or, Not, Comparison, ColumnAlias, Relation, SubQuery, Exists) @@ -669,7 +670,7 @@ 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 + # put them in fakehaving if they don't share an Or node as ancestor # with another comparison containing an aggregat function for compnode in tocheck: parents = set() @@ -784,7 +785,20 @@ sorts = select.orderby groups = select.groupby having = select.having - morerestr = extract_fake_having_terms(having) + for restr in extract_fake_having_terms(having): + scope = None + for vref in restr.get_nodes(VariableRef): + vscope = vref.variable.scope + if vscope is select: + continue # ignore select scope, so restriction is added to + # the inner most scope possible + if scope is None: + scope = vscope + elif vscope is not scope: + scope = common_parent(scope, vscope).scope + if scope is None: + scope = select + scope.add_restriction(restr) # 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 @@ -829,7 +843,7 @@ self._in_wrapping_query = False self._state = state try: - sql = self._solutions_sql(select, morerestr, sols, distinct, + sql = self._solutions_sql(select, sols, distinct, needalias or needwrap) # generate groups / having before wrapping query selection to get # correct column aliases @@ -900,15 +914,13 @@ except KeyError: continue - def _solutions_sql(self, select, morerestr, solutions, distinct, needalias): + def _solutions_sql(self, select, 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)) diff -r d911a73ac8c5 -r f31f9882c90f server/test/unittest_rql2sql.py --- a/server/test/unittest_rql2sql.py Wed Jul 20 14:10:02 2011 +0200 +++ b/server/test/unittest_rql2sql.py Wed Jul 20 14:10:03 2011 +0200 @@ -1643,12 +1643,26 @@ '''SELECT (A || _X.cw_ref) FROM cw_Affaire AS _X''') - def test_or_having_fake_terms(self): + def test_or_having_fake_terms_base(self): self._check('Any X WHERE X is CWUser, X creation_date D HAVING YEAR(D) = "2010" OR D = NULL', '''SELECT _X.cw_eid FROM cw_CWUser AS _X WHERE ((CAST(EXTRACT(YEAR from _X.cw_creation_date) AS INTEGER)=2010) OR (_X.cw_creation_date IS NULL))''') + def test_or_having_fake_terms_exists(self): + # crash with rql <= 0.29.0 + self._check('Any X WHERE X is CWUser, EXISTS(B bookmarked_by X, B creation_date D) HAVING D=2010 OR D=NULL, D=1 OR D=NULL', + '''SELECT _X.cw_eid +FROM cw_CWUser AS _X +WHERE EXISTS(SELECT 1 FROM bookmarked_by_relation AS rel_bookmarked_by0, cw_Bookmark AS _B WHERE rel_bookmarked_by0.eid_from=_B.cw_eid AND rel_bookmarked_by0.eid_to=_X.cw_eid AND ((_B.cw_creation_date=1) OR (_B.cw_creation_date IS NULL)) AND ((_B.cw_creation_date=2010) OR (_B.cw_creation_date IS NULL)))''') + + def test_or_having_fake_terms_nocrash(self): + # crash with rql <= 0.29.0 + self._check('Any X WHERE X is CWUser, X creation_date D HAVING D=2010 OR D=NULL, D=1 OR D=NULL', + '''SELECT _X.cw_eid +FROM cw_CWUser AS _X +WHERE ((_X.cw_creation_date=1) OR (_X.cw_creation_date IS NULL)) AND ((_X.cw_creation_date=2010) OR (_X.cw_creation_date IS NULL))''') + def test_not_no_where(self): # XXX will check if some in_group relation exists, that's it. # We can't actually know if we want to check if there are some @@ -1699,7 +1713,7 @@ def test_regexp(self): self.skipTest('regexp-based pattern matching not implemented in sqlserver') - def test_or_having_fake_terms(self): + def test_or_having_fake_terms_base(self): self._check('Any X WHERE X is CWUser, X creation_date D HAVING YEAR(D) = "2010" OR D = NULL', '''SELECT _X.cw_eid FROM cw_CWUser AS _X @@ -1984,7 +1998,7 @@ yield t - def test_or_having_fake_terms(self): + def test_or_having_fake_terms_base(self): self._check('Any X WHERE X is CWUser, X creation_date D HAVING YEAR(D) = "2010" OR D = NULL', '''SELECT _X.cw_eid FROM cw_CWUser AS _X @@ -2095,7 +2109,7 @@ FROM cw_Personne AS _P''') - def test_or_having_fake_terms(self): + def test_or_having_fake_terms_base(self): self._check('Any X WHERE X is CWUser, X creation_date D HAVING YEAR(D) = "2010" OR D = NULL', '''SELECT _X.cw_eid FROM cw_CWUser AS _X