[nodes] Drop VariableRef.__cmp__ implementation (closes #1190458)

The existing implementation relies on hash returning different values for objects that compared equal. This is horribly wrong. Instead, stop implementing comparison, and use the is_equivalent method explicitly.

authorRémi Cardona <remi.cardona@free.fr>
changesetd475a40f349d
branchdefault
phasepublic
hiddenno
parent revision#346fc9258bbe Added tag 0.33.2, debian/0.33.2-1, centos/0.33.2-1 for changeset c629ea9f78f6
child revision#e96aa511f9db gecode_solver: fix build for python3
files modified by this revision
rql/__init__.py
rql/nodes.py
rql/stcheck.py
rql/stmts.py
# HG changeset patch
# User Rémi Cardona <remi.cardona@free.fr>
# Date 1441714596 -7200
# Tue Sep 08 14:16:36 2015 +0200
# Node ID d475a40f349dede7818b8c6a5630ba68771d3f58
# Parent 346fc9258bbec706cec6fa9478aaf91c9e8d488f
[nodes] Drop VariableRef.__cmp__ implementation (closes #1190458)

The existing implementation relies on hash returning different values
for objects that compared equal. This is horribly wrong. Instead, stop
implementing comparison, and use the is_equivalent method explicitly.

diff --git a/rql/__init__.py b/rql/__init__.py
@@ -147,18 +147,19 @@
1                      rel = vref.relation()
2                      if rel is None:
3                          term = vref
4                          while not term.parent is select:
5                              term = term.parent
6 -                        if term in select.selection:
7 +                        if any(term.is_equivalent(t) for t in select.selection):
8                              rhs = copy_uid_node(select, rhs, vconsts)
9                              if vref is term:
10 -                                select.selection[select.selection.index(vref)] = rhs
11 +                                index = next(i for i, var in enumerate(select.selection) if vref.is_equivalent(var))
12 +                                select.selection[index] = rhs
13                                  rhs.parent = select
14                              else:
15                                  vref.parent.replace(vref, rhs)
16 -                        elif term in select.orderby:
17 +                        elif any(term.is_equivalent(o) for o in select.orderby):
18                              # remove from orderby
19                              select.remove(term)
20                          elif not select.having:
21                              # remove from groupby if no HAVING clause
22                              select.remove(term)
diff --git a/rql/nodes.py b/rql/nodes.py
@@ -370,10 +370,12 @@
23          assert self.query is None
24          self.query = node
25          node.parent = self
26 
27      def is_equivalent(self, other):
28 +        if self is other:
29 +            return True
30          raise NotImplementedError
31 
32      def as_string(self, encoding=None, kwargs=None):
33          content = self.query and self.query.as_string(encoding, kwargs)
34          return 'EXISTS(%s)' % content
@@ -811,13 +813,10 @@
35          return self.name
36 
37      def __repr__(self):
38          return 'VarRef(%r)' % self.variable
39 
40 -    def __cmp__(self, other):
41 -        return not self.is_equivalent(other)
42 -
43      def register_reference(self):
44          self.variable.register_reference(self)
45 
46      def unregister_reference(self):
47          self.variable.unregister_reference(self)
diff --git a/rql/stcheck.py b/rql/stcheck.py
@@ -153,11 +153,11 @@
48                     and not (vinfo & VAR_SELECTED):
49                  raise BadRQLQuery('unbound variable %s (%s)' % (var.name, selected))
50          if node.groupby:
51              # check that selected variables are used in groups
52              for var in node.selection:
53 -                if isinstance(var, VariableRef) and not var in node.groupby:
54 +                if isinstance(var, VariableRef) and not any(var.is_equivalent(g) for g in node.groupby):
55                      state.error('variable %s should be grouped' % var)
56              for group in node.groupby:
57                  self._check_selected(group, 'group', state)
58          if node.distinct and node.orderby:
59              # check that variables referenced in the given term are reachable from
@@ -267,11 +267,11 @@
60                      state.error('order column out of bound %s' % term.value)
61          else:
62              stmt = term.stmt
63              for tvref in variable_refs(term):
64                  for vref in tvref.variable.references():
65 -                    if vref.relation() or vref in stmt.selection:
66 +                    if vref.relation() or any(vref.is_equivalent(s) for s in stmt.selection):
67                          break
68                  else:
69                      msg = 'sort variable %s is not referenced any where else'
70                      state.error(msg % tvref.name)
71 
diff --git a/rql/stmts.py b/rql/stmts.py
@@ -700,34 +700,38 @@
72      # * undo support
73      # * references handling
74      def replace(self, oldnode, newnode):
75          if oldnode is self.where:
76              self.where = newnode
77 -        elif oldnode in self.selection:
78 -            self.selection[self.selection.index(oldnode)] = newnode
79 -        elif oldnode in self.orderby:
80 -            self.orderby[self.orderby.index(oldnode)] = newnode
81 -        elif oldnode in self.groupby:
82 -            self.groupby[self.groupby.index(oldnode)] = newnode
83 -        elif oldnode in self.having:
84 -            self.having[self.having.index(oldnode)] = newnode
85 +        elif any(oldnode.is_equivalent(s) for s in self.selection):
86 +            index = next(i for i, s in enumerate(self.selection) if oldnode.is_equivalent(s))
87 +            self.selection[index] = newnode
88 +        elif any(oldnode.is_equivalent(o) for o in self.orderby):
89 +            index = next(i for i, o in enumerate(self.orderby) if oldnode.is_equivalent(o))
90 +            self.orderby[index] = newnode
91 +        elif any(oldnode.is_equivalent(g) for g in self.groupby):
92 +            index = next(i for i, g in enumerate(self.groupby) if oldnode.is_equivalent(g))
93 +            self.groupby[index] = newnode
94 +        elif any(oldnode.is_equivalent(h) for h in self.having):
95 +            index = next(i for i, h in enumerate(self.having) if oldnode.is_equivalent(h))
96 +            self.having[index] = newnode
97          else:
98              raise Exception('duh XXX %s' % oldnode)
99 -        # XXX no undo/reference support 'by design' (eg breaks things if you add
100 +        # XXX no undo/reference support 'by design' (i.e. breaks things if you add
101          # it...)
102          oldnode.parent = None
103          newnode.parent = self
104          return oldnode, self, None
105 
106      def remove(self, node):
107          if node is self.where:
108              self.where = None
109 -        elif node in self.orderby:
110 +        elif any(node.is_equivalent(o) for o in self.orderby):
111              self.remove_sort_term(node)
112 -        elif node in self.groupby:
113 +        elif any(node.is_equivalent(g) for g in self.groupby):
114              self.remove_group_term(node)
115 -        elif node in self.having:
116 +        elif any(node.is_equivalent(h) for h in self.having):
117              self.having.remove(node)
118          # XXX selection
119          else:
120              raise Exception('duh XXX')
121          node.parent = None
@@ -817,11 +821,12 @@
122          if self.should_register_op:
123              from rql.undo import RemoveGroupOperation
124              self.undo_manager.add_operation(RemoveGroupOperation(term))
125          for vref in term.iget_nodes(nodes.VariableRef):
126              vref.unregister_reference()
127 -        self.groupby.remove(term)
128 +        index = next(i for i, g in enumerate(self.groupby) if term.is_equivalent(g))
129 +        del self.groupby[index]
130      remove_group_var = deprecated('[rql 0.29] use remove_group_term instead')(remove_group_term)
131 
132      def remove_groups(self):
133          for vref in self.groupby[:]:
134              self.remove_group_term(vref)
@@ -868,11 +873,11 @@
135 
136      def select_only_variables(self):
137          selection = []
138          for term in self.selection:
139              for vref in term.iget_nodes(nodes.VariableRef):
140 -                if not vref in selection:
141 +                if not any(vref.is_equivalent(s) for s in selection):
142                      vref.parent = self
143                      selection.append(vref)
144          self.selection = selection
145 
146