# HG changeset patch # User Sylvain Thénault # Date 1253723025 -7200 # Node ID 34e451da9b5d28e37c1accfa81173f29dd6791c0 # Parent 1df034d5b6ec2f41f5e011cf39ed44c58bb61961 [security] test and fix/refactor optimization of optional varialbe when rewriting rql diff -r 1df034d5b6ec -r 34e451da9b5d rqlrewrite.py --- a/rqlrewrite.py Wed Sep 23 18:21:37 2009 +0200 +++ b/rqlrewrite.py Wed Sep 23 18:23:45 2009 +0200 @@ -384,28 +384,38 @@ # no more references, undefine the variable del self.select.defined_vars[vref.name] - def _may_be_shared(self, relation, target, searchedvarname): - """return True if the snippet relation can be skipped to use a relation - from the original query + def _may_be_shared_with(self, sniprel, target, searchedvarname): + """if the snippet relation can be skipped to use a relation from the + original query, return that relation node """ - # if cardinality is in '?1', we can ignore the relation and use variable - # from the original query - rschema = self.schema.rschema(relation.r_type) - if target == 'object': - cardindex = 0 - ttypes_func = rschema.objects - rprop = rschema.rproperty - else: # target == 'subject': - cardindex = 1 - ttypes_func = rschema.subjects - rprop = lambda x, y, z: rschema.rproperty(y, x, z) + rschema = self.schema.rschema(sniprel.r_type) + try: + if target == 'object': + orel = self.varinfo['lhs_rels'][sniprel.r_type] + cardindex = 0 + ttypes_func = rschema.objects + rprop = rschema.rproperty + else: # target == 'subject': + orel = self.varinfo['rhs_rels'][sniprel.r_type] + cardindex = 1 + ttypes_func = rschema.subjects + rprop = lambda x, y, z: rschema.rproperty(y, x, z) + except KeyError, ex: + # may be raised by self.varinfo['xhs_rels'][sniprel.r_type] + return None + # can't share neged relation or relations with different outer join + if (orel.neged(strict=True) or sniprel.neged(strict=True) + or (orel.optional and orel.optional != sniprel.optional)): + return None + # if cardinality is in '?1', we can ignore the snippet relation and use + # variable from the original query for etype in self.varinfo['stinfo']['possibletypes']: for ttype in ttypes_func(etype): if rprop(etype, ttype, 'cardinality')[cardindex] in '+*': - return False - return True + return None + return orel - def _use_outer_term(self, snippet_varname, term): + def _use_orig_term(self, snippet_varname, term): key = (self.current_expr, self.varmap, snippet_varname) if key in self.rewritten: insertedvar = self.select.defined_vars.pop(self.rewritten[key]) @@ -479,27 +489,17 @@ key = (self.current_expr, self.varmap, rhs.name) self.pending_keys.append( (key, action) ) return - if lhs.name in self.revvarmap: - # on lhs - # see if we can reuse this relation - rels = self.varinfo['lhs_rels'] - if (node.r_type in rels and isinstance(rhs, n.VariableRef) - and rhs.name != 'U' and not rels[node.r_type].neged(strict=True) - and self._may_be_shared(node, 'object', lhs.name)): - # ok, can share variable - term = rels[node.r_type].children[1].children[0] - self._use_outer_term(rhs.name, term) - return - elif isinstance(rhs, n.VariableRef) and rhs.name in self.revvarmap and lhs.name != 'U': - # on rhs - # see if we can reuse this relation - rels = self.varinfo['rhs_rels'] - if (node.r_type in rels and not rels[node.r_type].neged(strict=True) - and self._may_be_shared(node, 'subject', rhs.name)): - # ok, can share variable - term = rels[node.r_type].children[0] - self._use_outer_term(lhs.name, term) - return + if isinstance(rhs, n.VariableRef): + if lhs.name in self.revvarmap and rhs.name != 'U': + orel = self._may_be_shared_with(node, 'object', lhs.name) + if orel is not None: + self._use_orig_term(rhs.name, orel.children[1].children[0]) + return + elif rhs.name in self.revvarmap and lhs.name != 'U': + orel = self._may_be_shared_with(node, 'subject', rhs.name) + if orel is not None: + self._use_orig_term(lhs.name, orel.children[0]) + return rel = n.Relation(node.r_type, node.optional) for c in node.children: rel.append(c.accept(self)) diff -r 1df034d5b6ec -r 34e451da9b5d test/unittest_rqlrewrite.py --- a/test/unittest_rqlrewrite.py Wed Sep 23 18:21:37 2009 +0200 +++ b/test/unittest_rqlrewrite.py Wed Sep 23 18:23:45 2009 +0200 @@ -136,19 +136,72 @@ "(Any C,T WHERE C in_state B, D in_group F, G require_state B, G name 'read', " "G require_group F, C title T, D eid %(A)s, C is Card)") - def test_relation_optimization(self): + def test_relation_optimization_1_lhs(self): # since Card in_state State as monovalued cardinality, the in_state # relation used in the rql expression can be ignored and S replaced by # the variable from the incoming query - card_constraint = ('X in_state S, U in_group G, P require_state S,' - 'P name "read", P require_group G') + snippet = ('X in_state S, S name "hop"') rqlst = parse('Card C WHERE C in_state STATE') - rewrite(rqlst, {('C', 'X'): (card_constraint,)}, {}) + rewrite(rqlst, {('C', 'X'): (snippet,)}, {}) + self.failUnlessEqual(rqlst.as_string(), + "Any C WHERE C in_state STATE, C is Card, " + "EXISTS(STATE name 'hop'), STATE is State") + def test_relation_optimization_1_rhs(self): + snippet = ('TW subworkflow_exit X, TW name "hop"') + rqlst = parse('WorkflowTransition C WHERE C subworkflow_exit EXIT') + rewrite(rqlst, {('EXIT', 'X'): (snippet,)}, {}) + self.failUnlessEqual(rqlst.as_string(), + "Any C WHERE C subworkflow_exit EXIT, C is WorkflowTransition, " + "EXISTS(C name 'hop'), EXIT is SubWorkflowExitPoint") + + def test_relation_optimization_2_lhs(self): + # optional relation can be shared if also optional in the snippet + snippet = ('X in_state S?, S name "hop"') + rqlst = parse('Card C WHERE C in_state STATE?') + rewrite(rqlst, {('C', 'X'): (snippet,)}, {}) + self.failUnlessEqual(rqlst.as_string(), + "Any C WHERE C in_state STATE?, C is Card, " + "EXISTS(STATE name 'hop'), STATE is State") + def test_relation_optimization_2_rhs(self): + snippet = ('TW? subworkflow_exit X, TW name "hop"') + rqlst = parse('SubWorkflowExitPoint EXIT WHERE C? subworkflow_exit EXIT') + rewrite(rqlst, {('EXIT', 'X'): (snippet,)}, {}) self.failUnlessEqual(rqlst.as_string(), - u"Any C WHERE C in_state STATE, C is Card, A eid %(B)s, " - "EXISTS(A in_group D, E require_state STATE, " - "E name 'read', E require_group D, D is CWGroup, E is CWPermission), " - "STATE is State") + "Any EXIT WHERE C? subworkflow_exit EXIT, EXIT is SubWorkflowExitPoint, " + "EXISTS(C name 'hop'), C is WorkflowTransition") + + def test_relation_optimization_3_lhs(self): + # optional relation in the snippet but not in the orig tree can be shared + snippet = ('X in_state S?, S name "hop"') + rqlst = parse('Card C WHERE C in_state STATE') + rewrite(rqlst, {('C', 'X'): (snippet,)}, {}) + self.failUnlessEqual(rqlst.as_string(), + "Any C WHERE C in_state STATE, C is Card, " + "EXISTS(STATE name 'hop'), STATE is State") + def test_relation_optimization_3_rhs(self): + snippet = ('TW? subworkflow_exit X, TW name "hop"') + rqlst = parse('WorkflowTransition C WHERE C subworkflow_exit EXIT') + rewrite(rqlst, {('EXIT', 'X'): (snippet,)}, {}) + self.failUnlessEqual(rqlst.as_string(), + "Any C WHERE C subworkflow_exit EXIT, C is WorkflowTransition, " + "EXISTS(C name 'hop'), EXIT is SubWorkflowExitPoint") + + def test_relation_non_optimization_1_lhs(self): + # but optional relation in the orig tree but not in the snippet can't be shared + snippet = ('X in_state S, S name "hop"') + rqlst = parse('Card C WHERE C in_state STATE?') + rewrite(rqlst, {('C', 'X'): (snippet,)}, {}) + self.failUnlessEqual(rqlst.as_string(), + "Any C WHERE C in_state STATE?, C is Card, " + "EXISTS(C in_state A, A name 'hop', A is State), STATE is State") + def test_relation_non_optimization_1_rhs(self): + snippet = ('TW subworkflow_exit X, TW name "hop"') + rqlst = parse('SubWorkflowExitPoint EXIT WHERE C? subworkflow_exit EXIT') + rewrite(rqlst, {('EXIT', 'X'): (snippet,)}, {}) + self.failUnlessEqual(rqlst.as_string(), + "Any EXIT WHERE C? subworkflow_exit EXIT, EXIT is SubWorkflowExitPoint, " + "EXISTS(A subworkflow_exit EXIT, A name 'hop', A is WorkflowTransition), " + "C is WorkflowTransition") def test_unsupported_constraint_1(self): # CWUser doesn't have require_permission