[rql2sql] fix bug with NOT of inlined relation: NULL values are not properly handled and hence some rows won't be returned while the should stable
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Wed, 21 Jul 2010 12:42:18 +0200
branchstable
changeset 6003 5fbc1c4c13ff
parent 6002 0ce7052ce30b
child 6004 d17d3b34bc12
[rql2sql] fix bug with NOT of inlined relation: NULL values are not properly handled and hence some rows won't be returned while the should
server/sources/rql2sql.py
server/test/unittest_rql2sql.py
--- a/server/sources/rql2sql.py	Wed Jul 21 12:41:14 2010 +0200
+++ b/server/sources/rql2sql.py	Wed Jul 21 12:42:18 2010 +0200
@@ -818,26 +818,35 @@
 
     def _visit_inlined_relation(self, relation):
         lhsvar, _, rhsvar, rhsconst = relation_info(relation)
-        # we are sure here to have a lhsvar
-        assert lhsvar is not None
-        if isinstance(relation.parent, Not) \
-               and len(lhsvar.stinfo['relations']) > 1 \
-               and (rhsvar is not None and rhsvar._q_invariant):
+        # we are sure lhsvar is not None
+        lhssql = self._inlined_var_sql(lhsvar, relation.r_type)
+        if rhsvar is None:
+            moresql = None
+        else:
+            moresql = self._extra_join_sql(relation, lhssql, rhsvar)
+        if isinstance(relation.parent, Not):
             self._state.done.add(relation.parent)
-            return '%s IS NULL' % self._inlined_var_sql(lhsvar, relation.r_type)
-        lhssql = self._inlined_var_sql(lhsvar, relation.r_type)
-        if rhsconst is not None:
-            return '%s=%s' % (lhssql, rhsconst.accept(self))
-        if isinstance(rhsvar, Variable) and not rhsvar.name in self._varmap:
+            if rhsvar is not None and rhsvar._q_invariant:
+                sql = '%s IS NULL' % lhssql
+            else:
+                # column != 1234 may not get back rows where column is NULL...
+                sql = '(%s IS NULL OR %s!=%s)' % (
+                    lhssql, lhssql, (rhsvar or rhsconst).accept(self))
+        elif rhsconst is not None:
+            sql = '%s=%s' % (lhssql, rhsconst.accept(self))
+        elif isinstance(rhsvar, Variable) and rhsvar._q_invariant and \
+                 not rhsvar.name in self._varmap:
             # if the rhs variable is only linked to this relation, this mean we
             # only want the relation to exists, eg NOT NULL in case of inlined
             # relation
-            if rhsvar._q_invariant:
-                sql = self._extra_join_sql(relation, lhssql, rhsvar)
-                if sql:
-                    return sql
-                return '%s IS NOT NULL' % lhssql
-        return '%s=%s' % (lhssql, rhsvar.accept(self))
+            if moresql is not None:
+                return moresql
+            return '%s IS NOT NULL' % lhssql
+        else:
+            sql = '%s=%s' % (lhssql, rhsvar.accept(self))
+        if moresql is None:
+            return sql
+        return '%s AND %s' % (sql, moresql)
 
     def _process_relation_term(self, relation, rid, termvar, termconst, relfield):
         if termconst or not termvar._q_invariant:
@@ -849,7 +858,7 @@
                 termsql = termvar.accept(self)
                 yield '%s.%s=%s' % (rid, relfield, termsql)
             extrajoin = self._extra_join_sql(relation, '%s.%s' % (rid, relfield), termvar)
-            if extrajoin:
+            if extrajoin is not None:
                 yield extrajoin
 
     def _visit_relation(self, relation, rschema):
@@ -1234,7 +1243,7 @@
             # no principal defined, relation is necessarily the principal and
             # so nothing to return here
             pass
-        return ''
+        return None
 
     def _temp_table_scope(self, select, table):
         scope = 9999
--- a/server/test/unittest_rql2sql.py	Wed Jul 21 12:41:14 2010 +0200
+++ b/server/test/unittest_rql2sql.py	Wed Jul 21 12:42:18 2010 +0200
@@ -271,7 +271,7 @@
     ('Any O WHERE NOT S ecrit_par O, S eid 1, S inline1 P, O inline2 P',
      '''SELECT _O.cw_eid
 FROM cw_Note AS _S, cw_Personne AS _O
-WHERE NOT (_S.cw_ecrit_par=_O.cw_eid) AND _S.cw_eid=1 AND _S.cw_inline1 IS NOT NULL AND _O.cw_inline2=_S.cw_inline1'''),
+WHERE (_S.cw_ecrit_par IS NULL OR _S.cw_ecrit_par!=_O.cw_eid) AND _S.cw_eid=1 AND _S.cw_inline1 IS NOT NULL AND _O.cw_inline2=_S.cw_inline1'''),
 
     ('DISTINCT Any S ORDERBY stockproc(SI) WHERE NOT S ecrit_par O, S para SI',
      '''SELECT T1.C0 FROM (SELECT DISTINCT _S.cw_eid AS C0, STOCKPROC(_S.cw_para) AS C1
@@ -832,7 +832,7 @@
     ('Any O,AD  WHERE NOT S inline1 O, S eid 123, O todo_by AD?',
      '''SELECT _O.cw_eid, rel_todo_by0.eid_to
 FROM cw_Affaire AS _O LEFT OUTER JOIN todo_by_relation AS rel_todo_by0 ON (rel_todo_by0.eid_from=_O.cw_eid), cw_Note AS _S
-WHERE NOT (_S.cw_inline1=_O.cw_eid) AND _S.cw_eid=123''')
+WHERE (_S.cw_inline1 IS NULL OR _S.cw_inline1!=_O.cw_eid) AND _S.cw_eid=123''')
     ]
 
 VIRTUAL_VARS = [
@@ -982,7 +982,7 @@
     ('Any N WHERE NOT N ecrit_par P, P nom "toto"',
      '''SELECT _N.cw_eid
 FROM cw_Note AS _N, cw_Personne AS _P
-WHERE NOT (_N.cw_ecrit_par=_P.cw_eid) AND _P.cw_nom=toto'''),
+WHERE (_N.cw_ecrit_par IS NULL OR _N.cw_ecrit_par!=_P.cw_eid) AND _P.cw_nom=toto'''),
 
     ('Any P WHERE NOT N ecrit_par P, P nom "toto"',
      '''SELECT _P.cw_eid
@@ -1002,7 +1002,7 @@
     ('Any P WHERE NOT N ecrit_par P, P is Personne, N eid 512',
      '''SELECT _P.cw_eid
 FROM cw_Note AS _N, cw_Personne AS _P
-WHERE NOT (_N.cw_ecrit_par=_P.cw_eid) AND _N.cw_eid=512'''),
+WHERE (_N.cw_ecrit_par IS NULL OR _N.cw_ecrit_par!=_P.cw_eid) AND _N.cw_eid=512'''),
 
     ('Any S,ES,T WHERE S state_of ET, ET name "CWUser", ES allowed_transition T, T destination_state S',
      # XXX "_T.cw_destination_state IS NOT NULL" could be avoided here but it's not worth it
@@ -1030,11 +1030,12 @@
     ('DISTINCT Any X WHERE X from_entity OET, NOT X from_entity NET, OET name "Image", NET eid 1',
      '''SELECT DISTINCT _X.cw_eid
 FROM cw_CWAttribute AS _X, cw_CWEType AS _OET
-WHERE _X.cw_from_entity=_OET.cw_eid AND NOT (_X.cw_from_entity=1) AND _OET.cw_name=Image
+WHERE _X.cw_from_entity=_OET.cw_eid AND (_X.cw_from_entity IS NULL OR _X.cw_from_entity!=1) AND _OET.cw_name=Image
 UNION
 SELECT DISTINCT _X.cw_eid
 FROM cw_CWEType AS _OET, cw_CWRelation AS _X
-WHERE _X.cw_from_entity=_OET.cw_eid AND NOT (_X.cw_from_entity=1) AND _OET.cw_name=Image'''),
+WHERE _X.cw_from_entity=_OET.cw_eid AND (_X.cw_from_entity IS NULL OR _X.cw_from_entity!=1) AND _OET.cw_name=Image'''),
+
     ]
 
 INTERSECT = [