# HG changeset patch
# User Sylvain Thenault <sylvain.thenault@logilab.fr>
# Date 1223036581 -7200
#      Fri Oct 03 14:23:01 2008 +0200
# Node ID 44b5c41b0bef599d0fc8306999499e893c3ad8ec
# Parent  2611bcb4f5e49851e833011a14a0482c3d934c4e
rql checker raise BadRQLQuery for inconsistent orderby on distinct query

diff --git a/ChangeLog b/ChangeLog
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,10 @@
 ChangeLog for RQL
 =================
 
+--
+    * rql checker raise BadRQLQuery for inconsistent orderby on distinct query
+    * correctly restore .parent when undoing RemoveNodeOperation
+	
 2008-09-24  --  0.20.0
     * is_instance_of support
     * raise BadRQLQuery on queries like 'Any X WHERE X name Toto, P is Person'
diff --git a/nodes.py b/nodes.py
--- a/nodes.py
+++ b/nodes.py
@@ -152,7 +152,14 @@
 
     def add_eid_restriction(self, var, eid): 
         """builds a restriction node to express '<var> eid <eid>'"""
-        return self.add_restriction(make_relation(var, 'eid', (eid, 'Int'), Constant))
+        if isinstance(eid, (set, frozenset, tuple, list, dict)):
+            rel = make_relation(var, 'eid', ('IN',), Function, operator='=')
+            infunc = rel.children[1].children[0]
+            for eid_ in sorted(eid):
+                infunc.append(Constant(eid_, 'Int'))
+            return self.add_restriction(rel)
+        return self.add_restriction(make_relation(var, 'eid', (eid, 'Int'),
+                                                  Constant))
     
     def add_type_restriction(self, var, etype):
         """builds a restriction node to express : variable is etype"""
diff --git a/stcheck.py b/stcheck.py
--- a/stcheck.py
+++ b/stcheck.py
@@ -8,6 +8,7 @@
 
 from itertools import chain
 from logilab.common.compat import any
+from logilab.common.graph import has_path
 
 from rql._exceptions import BadRQLQuery
 from rql.utils import function_description
@@ -87,12 +88,9 @@
         pass
     
     def visit_select(self, node, errors):
+        node.vargraph = {} # graph representing links between variable
         self._visit_selectedterm(node, errors)
-#         #XXX from should be added to children, no ?
-#         for subquery in node.from_:
-#             self.visit_union(subquery, errors)
-#         if node.sortterms:
-#             self._visit(node.sortterms, errors)            
+        
     def leave_select(self, node, errors):
         selected = node.selection
         # check selected variable are used in restriction
@@ -106,21 +104,43 @@
                     errors.append('variable %s should be grouped' % var)
             for group in node.groupby:
                 self._check_selected(group, 'group', errors)
-#         # check that variables referenced in the given term are selected
-#         for term in node.orderby:
-#             for vref in term.iget_nodes(VariableRef):
-#                 # no stinfo yet, use references
-#                 try:
-#                     for ovref in node.defined_vars[vref.name].references():
-#                         rel = ovref.relation()
-#                         if rel is not None:
-#                             break
-#                     else:
-#                         msg = 'variable %s used in %s is not referenced by %s'
-#                         errors.append(msg % (vref.name, termtype, node.as_string()))
-#                 except KeyError:
-#                     msg = 'variable %s used in %s is not referenced by %s'
-#                     errors.append(msg % (vref.name, termtype, node.as_string()))
+        if node.distinct:
+            # check that variables referenced in the given term are reachable from
+            # a selected variable with only ?1 cardinalityselected
+            graph = node.vargraph
+            selectidx = frozenset(vref.name for term in selected for vref in term.iget_nodes(VariableRef))
+            schema = self.schema
+            for sortterm in node.orderby:
+                for vref in sortterm.term.iget_nodes(VariableRef):
+                    if vref.name in selectidx:
+                        continue
+                    for vname in selectidx:
+                        if self.has_unique_value_path(graph, vname, vref.name):
+                            break
+                    else:
+                        msg = ('can\'t sort on variable %s which is linked to a'
+                               ' variable in the selection but may have different'
+                               ' values for a resulting row')
+                        errors.append(msg % vref.name)
+
+    def has_unique_value_path(self, graph, fromvar, tovar):
+        path = has_path(graph, fromvar, tovar)
+        if path is None:
+            return False
+        for tovar in path:
+            try:
+                rtype = graph[(fromvar, tovar)]
+                cardidx = 0
+            except KeyError:
+                rtype = graph[(tovar, fromvar)]
+                cardidx = 1
+            rschema = self.schema.rschema(rtype)
+            for rdef in rschema.iter_rdefs():
+                if not rschema.rproperty(rdef[0], rdef[1], 'cardinality')[cardidx] in '?1':
+                    return False
+            fromvar = tovar
+        return True
+        
 
     def visit_insert(self, insert, errors):
         self._visit_selectedterm(insert, errors)
@@ -221,6 +241,16 @@
                 assert rhs.children[0].type == None
         elif not relation.r_type in self.schema:
             errors.append('unknown relation `%s`' % relation.r_type)
+        try:
+            vargraph = relation.stmt.vargraph
+            rhsvarname = relation.children[1].children[0].variable.name
+            lhsvarname = relation.children[0].name
+        except AttributeError:
+            pass
+        else:
+            vargraph.setdefault(lhsvarname, []).append(rhsvarname)
+            vargraph.setdefault(rhsvarname, []).append(lhsvarname)
+            vargraph[(lhsvarname, rhsvarname)] = relation.r_type
         
     def leave_relation(self, relation, errors):
         pass
diff --git a/test/unittest_analyze.py b/test/unittest_analyze.py
--- a/test/unittest_analyze.py
+++ b/test/unittest_analyze.py
@@ -18,16 +18,25 @@
     
 
 class RelationSchema(ERSchema):
-    def __init__(self, assoc_types, symetric=False):
+    def __init__(self, assoc_types, symetric=False, card=None):
         self.assoc_types = assoc_types
         self.subj_types = [e_type[0] for e_type in assoc_types]
+        self.final = False
         d = {}
         for e_type, dest_types in assoc_types:
             for e_type in dest_types:
                 d[e_type] = 1
+                if e_type in ('Int', 'Datetime', 'String'):
+                    self.final = True
         self.obj_types = d.keys()
         self.symetric = symetric
         self.inlined = False
+        if card is None:
+            if self.final:
+                card = '?*'
+            else:
+                card = '**'
+        self.card = card
         
     def associations(self):
         return self.assoc_types
@@ -41,6 +50,15 @@
     def is_final(self):
         return self.obj_types[0] in FINAL_ETYPES
 
+    def iter_rdefs(self):
+        for subjtype, dest_types in self.assoc_types:
+            for objtype in dest_types:
+                yield subjtype, objtype
+
+    def rproperty(self, subj, obj, rprop):
+        assert rprop == 'cardinality'
+        return self.card
+    
 class EntitySchema(ERSchema):
     def __init__(self, type, specialized_by=None):
         self.type = type
@@ -60,7 +78,7 @@
                      'Eetype', 'Person', 'Company', 'Address', 'Student']:
             self._types[etype] = EntitySchema(etype)
         self._types['Person']._specialized_by = [self._types['Student']]
-        self._relations = {
+        self._relations  = {
             'eid' : RelationSchema( ( ('Person', ('Int',) ),
                                       ('Student', ('Int',) ),
                                       ('Company', ('Int',) ),
@@ -86,8 +104,8 @@
                                     ),
             'work_for' : RelationSchema( ( ('Person', ('Company',) ),
                                            ('Student', ('Company',) ),
-                                           )
-                                        ),
+                                           ),
+                                         card='?*'),
             'is' : RelationSchema( ( ('Person', ('Eetype',) ),
                                      ('Student', ('Eetype',) ),
                                      ('Company', ('Eetype',) ),
@@ -130,7 +148,9 @@
                                       )
                                     ),
             }
-        
+        for rtype, rschema in self._relations.iteritems():
+            rschema.type = rtype
+            
     def entities(self):
         return self._types.values()
         
@@ -151,7 +171,6 @@
     def eschema(self, e_type):
         return self._types[e_type]
         
-        
 UNRESOLVABLE_QUERIES = (
     'Person X WHERE Y work_for X',
     'Person X WHERE X work_for Y, Y is Address',
diff --git a/test/unittest_stcheck.py b/test/unittest_stcheck.py
--- a/test/unittest_stcheck.py
+++ b/test/unittest_stcheck.py
@@ -32,6 +32,10 @@
     'Any X, X/Z WHERE X is Person; WITH Y BEING (Any SUM(X) WHERE X is Person)',
 
     'Any X WHERE X name "Toto", P is Person',
+
+    # BAD QUERY cant sort on y
+    'DISTINCT Any X ORDERBY Y WHERE B work_for X, B name Y',
+    
     )
 
 OK_QUERIES = (
@@ -39,7 +43,16 @@
     ' UNION '
     '(Any N,COUNT(X) GROUPBY N WHERE X firstname N)',
 
-    'DISTINCT Any X, MAX(Y) GROUPBY X WHERE X is Person, Y is Company'
+    'DISTINCT Any X, MAX(Y) GROUPBY X WHERE X is Person, Y is Company',
+
+    # sorting allowed since order variable reachable from a selected
+    # variable with only ?1 cardinality
+    'DISTINCT Any B ORDERBY Y WHERE B work_for X, B name Y',
+    'DISTINCT Any B ORDERBY Y WHERE B work_for X, X name Y',
+
+#    'DISTINCT Any X ORDERBY SN WHERE X in_state S, S name SN',
+    
+    
     )
 
 class CheckClassTest(TestCase):