[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.
--- 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))
--- 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