From 73f734bf80166c7dfce4892941752d7569a17524 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 6 Feb 2012 12:20:15 -0500 Subject: initial annotations approach to join conditions. all tests pass, plus additional tests in #1401 pass. would now like to reorganize RelationshipProperty more around the annotations concept. --- lib/sqlalchemy/orm/properties.py | 40 +++++++++++++++++++++++++++++---- lib/sqlalchemy/sql/expression.py | 2 +- lib/sqlalchemy/sql/util.py | 48 ++++++++++++++++++++++++++-------------- lib/sqlalchemy/sql/visitors.py | 6 ++--- 4 files changed, 71 insertions(+), 25 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 59c4cb3dc..a590ad706 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -14,7 +14,7 @@ mapped attributes. from sqlalchemy import sql, util, log, exc as sa_exc from sqlalchemy.sql.util import ClauseAdapter, criterion_as_pairs, \ join_condition, _shallow_annotate -from sqlalchemy.sql import operators, expression +from sqlalchemy.sql import operators, expression, visitors from sqlalchemy.orm import attributes, dependency, mapper, \ object_mapper, strategies, configure_mappers from sqlalchemy.orm.util import CascadeOptions, _class_to_mapper, \ @@ -444,6 +444,7 @@ class RelationshipProperty(StrategizedProperty): else: j = _orm_annotate(pj, exclude=self.property.remote_side) + # MARKMARK if criterion is not None and target_adapter: # limit this adapter to annotated only? criterion = target_adapter.traverse(criterion) @@ -1376,6 +1377,34 @@ class RelationshipProperty(StrategizedProperty): "argument to indicate which column lazy join " "condition should bind." % (col, self.mapper)) + count = [0] + def clone(elem): + if set(['local', 'remote']).intersection(elem._annotations): + return None + elif elem in self.local_side and elem in self.remote_side: + # TODO: OK this still sucks. this is basically, + # refuse, refuse, refuse the temptation to guess! + # but crap we really have to guess don't we. we + # might want to traverse here with cloned_traverse + # so we can see the binary exprs and do it at that + # level.... + if count[0] % 2 == 0: + elem = elem._annotate({'local':True}) + else: + elem = elem._annotate({'remote':True}) + count[0] += 1 + elif elem in self.local_side: + elem = elem._annotate({'local':True}) + elif elem in self.remote_side: + elem = elem._annotate({'remote':True}) + else: + elem = None + return elem + + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, {}, clone + ) + def _generate_backref(self): if not self.is_primary(): return @@ -1539,17 +1568,20 @@ class RelationshipProperty(StrategizedProperty): secondary_aliasizer.traverse(secondaryjoin) else: primary_aliasizer = ClauseAdapter(dest_selectable, - exclude=self.local_side, + #exclude=self.local_side, + exclude_fn=lambda c: "local" in c._annotations, equivalents=self.mapper._equivalent_columns) if source_selectable is not None: primary_aliasizer.chain( ClauseAdapter(source_selectable, - exclude=self.remote_side, + #exclude=self.remote_side, + exclude_fn=lambda c: "remote" in c._annotations, equivalents=self.parent._equivalent_columns)) secondary_aliasizer = None + primaryjoin = primary_aliasizer.traverse(primaryjoin) target_adapter = secondary_aliasizer or primary_aliasizer - target_adapter.include = target_adapter.exclude = None + target_adapter.include = target_adapter.exclude = target_adapter.exclude_fn = None else: target_adapter = None if source_selectable is None: diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py index b11e5ad42..30e19bc68 100644 --- a/lib/sqlalchemy/sql/expression.py +++ b/lib/sqlalchemy/sql/expression.py @@ -2184,7 +2184,7 @@ class ColumnElement(ClauseElement, _CompareMixin): for oth in to_compare: if use_proxies and self.shares_lineage(oth): return True - elif oth is self: + elif hash(oth) == hash(self): return True else: return False diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py index 97975441e..f0509c16f 100644 --- a/lib/sqlalchemy/sql/util.py +++ b/lib/sqlalchemy/sql/util.py @@ -225,7 +225,8 @@ def adapt_criterion_to_null(crit, nulls): return visitors.cloned_traverse(crit, {}, {'binary':visit_binary}) -def join_condition(a, b, ignore_nonexistent_tables=False, a_subset=None): +def join_condition(a, b, ignore_nonexistent_tables=False, + a_subset=None): """create a join condition between two tables or selectables. e.g.:: @@ -535,6 +536,10 @@ def criterion_as_pairs(expression, consider_as_foreign_keys=None, "'consider_as_foreign_keys' or " "'consider_as_referenced_keys'") + def col_is(a, b): + #return a is b + return a.compare(b) + def visit_binary(binary): if not any_operator and binary.operator is not operators.eq: return @@ -544,20 +549,20 @@ def criterion_as_pairs(expression, consider_as_foreign_keys=None, if consider_as_foreign_keys: if binary.left in consider_as_foreign_keys and \ - (binary.right is binary.left or + (col_is(binary.right, binary.left) or binary.right not in consider_as_foreign_keys): pairs.append((binary.right, binary.left)) elif binary.right in consider_as_foreign_keys and \ - (binary.left is binary.right or + (col_is(binary.left, binary.right) or binary.left not in consider_as_foreign_keys): pairs.append((binary.left, binary.right)) elif consider_as_referenced_keys: if binary.left in consider_as_referenced_keys and \ - (binary.right is binary.left or + (col_is(binary.right, binary.left) or binary.right not in consider_as_referenced_keys): pairs.append((binary.left, binary.right)) elif binary.right in consider_as_referenced_keys and \ - (binary.left is binary.right or + (col_is(binary.left, binary.right) or binary.left not in consider_as_referenced_keys): pairs.append((binary.right, binary.left)) else: @@ -669,11 +674,22 @@ class ClauseAdapter(visitors.ReplacingCloningVisitor): s.c.col1 == table2.c.col1 """ - def __init__(self, selectable, equivalents=None, include=None, exclude=None, adapt_on_names=False): + def __init__(self, selectable, equivalents=None, + include=None, exclude=None, + include_fn=None, exclude_fn=None, + adapt_on_names=False): self.__traverse_options__ = {'stop_on':[selectable]} self.selectable = selectable - self.include = include - self.exclude = exclude + if include: + assert not include_fn + self.include_fn = lambda e: e in include + else: + self.include_fn = include_fn + if exclude: + assert not exclude_fn + self.exclude_fn = lambda e: e in exclude + else: + self.exclude_fn = exclude_fn self.equivalents = util.column_dict(equivalents or {}) self.adapt_on_names = adapt_on_names @@ -693,19 +709,17 @@ class ClauseAdapter(visitors.ReplacingCloningVisitor): return newcol def replace(self, col): - if isinstance(col, expression.FromClause): - if self.selectable.is_derived_from(col): + if isinstance(col, expression.FromClause) and \ + self.selectable.is_derived_from(col): return self.selectable - - if not isinstance(col, expression.ColumnElement): + elif not isinstance(col, expression.ColumnElement): return None - - if self.include and col not in self.include: + elif self.include_fn and not self.include_fn(col): return None - elif self.exclude and col in self.exclude: + elif self.exclude_fn and self.exclude_fn(col): return None - - return self._corresponding_column(col, True) + else: + return self._corresponding_column(col, True) class ColumnAdapter(ClauseAdapter): """Extends ClauseAdapter with extra utility functions. diff --git a/lib/sqlalchemy/sql/visitors.py b/lib/sqlalchemy/sql/visitors.py index cdcf40aa8..75e099f0d 100644 --- a/lib/sqlalchemy/sql/visitors.py +++ b/lib/sqlalchemy/sql/visitors.py @@ -240,16 +240,16 @@ def replacement_traverse(obj, opts, replace): replacement by a given replacement function.""" cloned = util.column_dict() - stop_on = util.column_set(opts.get('stop_on', [])) + stop_on = util.column_set([id(x) for x in opts.get('stop_on', [])]) def clone(elem, **kw): - if elem in stop_on or \ + if id(elem) in stop_on or \ 'no_replacement_traverse' in elem._annotations: return elem else: newelem = replace(elem) if newelem is not None: - stop_on.add(newelem) + stop_on.add(id(newelem)) return newelem else: if elem not in cloned: -- cgit v1.2.1 From c11bc3f278862e72051ca5fe6ef35eb8d4bf9d99 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 6 Feb 2012 14:48:03 -0500 Subject: proof of concept. relationships.JoinCondition is given everything known about the relationship in an ORM-agnostic way. The primaryjoin is determined as before, but is then immediately parsed and annotated with "foreign", "local" and "remote" annotations. These annotations will then drive the additional decisions made, namely "direction" and "sync pairs". The end user will be able to pass a pre-annotated join condition in which case those annotations take effect as given, which will in one pass fix both #1401 as well as #610 (for #610, the "foreign" / "remote" annotation can recursively descend down to ColumnElement objects). --- lib/sqlalchemy/orm/relationships.py | 742 ++++++++++++++++++++++++++++++++++++ 1 file changed, 742 insertions(+) create mode 100644 lib/sqlalchemy/orm/relationships.py (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py new file mode 100644 index 000000000..bb70f8a11 --- /dev/null +++ b/lib/sqlalchemy/orm/relationships.py @@ -0,0 +1,742 @@ +# orm/relationships.py +# Copyright (C) 2005-2012 the SQLAlchemy authors and contributors +# +# This module is part of SQLAlchemy and is released under +# the MIT License: http://www.opensource.org/licenses/mit-license.php + +"""Heuristics related to join conditions as used in +:func:`.relationship`. + +Provides the :class:`.JoinCondition` object, which encapsulates +SQL annotation and aliasing behavior focused on the `primaryjoin` +and `secondaryjoin` aspects of :func:`.relationship`. + +""" + +from sqlalchemy import sql, util, log, exc as sa_exc, schema +from sqlalchemy.sql.util import ClauseAdapter, criterion_as_pairs, \ + join_condition, _shallow_annotate +from sqlalchemy.sql import operators, expression, visitors +from sqlalchemy.orm.interfaces import MANYTOMANY, MANYTOONE, ONETOMANY + +class JoinCondition(object): + def __init__(self, + parent_selectable, + child_selectable, + parent_local_selectable, + child_local_selectable, + primaryjoin=None, + secondary=None, + secondaryjoin=None, + parent_equivalents=None, + child_equivalents=None, + consider_as_foreign_keys=None, + local_remote_pairs=None, + remote_side=None, + extra_child_criterion=None, + self_referential=False, + prop=None, + support_sync=True + ): + self.parent_selectable = parent_selectable + self.parent_local_selectable = parent_local_selectable + self.child_selectable = child_selectable + self.child_local_selecatble = child_local_selectable + self.parent_equivalents = parent_equivalents + self.child_equivalents = child_equivalents + self.primaryjoin = primaryjoin + self.secondaryjoin = secondaryjoin + self.secondary = secondary + self.extra_child_criterion = extra_child_criterion + self.consider_as_foreign_keys = consider_as_foreign_keys + self.local_remote_pairs = local_remote_pairs + self.remote_side = remote_side + self.prop = prop + self.self_referential = self_referential + self.support_sync = support_sync + self._determine_joins() + self._parse_joins() + + def _determine_joins(self): + """Determine the 'primaryjoin' and 'secondaryjoin' attributes, + if not passed to the constructor already. + + This is based on analysis of the foreign key relationships + between the parent and target mapped selectables. + + """ + if self.secondaryjoin is not None and self.secondary is None: + raise sa_exc.ArgumentError( + "Property %s specified with secondary " + "join condition but " + "no secondary argument" % self.prop) + + # find a join between the given mapper's mapped table and + # the given table. will try the mapper's local table first + # for more specificity, then if not found will try the more + # general mapped table, which in the case of inheritance is + # a join. + try: + if self.secondary is not None: + if self.secondaryjoin is None: + self.secondaryjoin = \ + join_condition( + self.child_selectable, + self.secondary, + a_subset=self.child_local_selectable) + if self.primaryjoin is None: + self.primaryjoin = \ + join_condition( + self.parent_selectable, + self.secondary, + a_subset=parent_local_selectable) + else: + if self.primaryjoin is None: + self.primaryjoin = \ + join_condition( + self.parent_selectable, + self.child_selectable, + a_subset=self.parent_local_selectable) + except sa_exc.ArgumentError, e: + raise sa_exc.ArgumentError("Could not determine join " + "condition between parent/child tables on " + "relationship %s. Specify a 'primaryjoin' " + "expression. If 'secondary' is present, " + "'secondaryjoin' is needed as well." + % self.prop) + + def _parse_joins(self): + """Apply 'remote', 'local' and 'foreign' annotations + to the primary and secondary join conditions. + + """ + parentcols = util.column_set(self.parent_selectable.c) + targetcols = util.column_set(self.child_selectable.c) + + def col_is(a, b): + return a.compare(b) + + def is_foreign(a, b): + if self.consider_as_foreign_keys: + if a in self.consider_as_foreign_keys and ( + col_is(a, b) or + b not in self.consider_as_foreign_keys + ): + return a + elif b in self.consider_as_foreign_keys and ( + col_is(a, b) or + a not in self.consider_as_foreign_keys + ): + return b + elif isinstance(a, schema.Column) and \ + isinstance(b, schema.Column): + if a.references(b): + return a + elif b.references(a): + return b + + any_operator = not self.support_sync + + def visit_binary(binary): + #if not any_operator and binary.operator is not operators.eq: + # return + if not isinstance(binary.left, sql.ColumnElement) or \ + not isinstance(binary.right, sql.ColumnElement): + return + + if "foreign" not in binary.left._annotations and \ + "foreign" not in binary.right._annotations: + col = is_foreign(binary.left, binary.right) + if col is not None: + if col is binary.left: + binary.left = binary.left._annotate( + {"foreign":True}) + elif col is binary.right: + binary.right = binary.right._annotate( + {"foreign":True}) + # TODO: when the two cols are the same. + + if "remote" not in binary.left._annotations and \ + "remote" not in binary.right._annotations: + if self.local_remote_pairs: + raise NotImplementedError() + elif self.remote_side: + raise NotImplementedError() + elif self.self_referential: + # assume one to many - FKs are "Remote" + if "foreign" in binary.left._annotations: + binary.left = binary.left._annotate( + {"remote":True}) + if binary.right in parentcols: + binary.right = binary.right._annotate( + {"local":True}) + elif "foreign" in binary.right._annotations: + binary.right = binary.right._annotate( + {"remote":True}) + if binary.left in parentcols: + binary.left = binary.left._annotate( + {"local":True}) + else: + if binary.left in targetcols: + binary.left = binary.left._annotate( + {"remote":True}) + if binary.right in parentcols: + binary.right = binary.right._annotate( + {"local":True}) + elif binary.right in targetcols: + binary.right = binary.right._annotate( + {"remote":True}) + if binary.left in parentcols: + binary.left = binary.left._annotate( + {"local":True}) + + self.primaryjoin = visitors.cloned_traverse( + self.primaryjoin, + {}, + {"binary":visit_binary} + ) + + def join_targets(self, source_selectable, + dest_selectable, + aliased): + """Given a source and destination selectable, create a + join between them. + + This takes into account aliasing the join clause + to reference the appropriate corresponding columns + in the target objects, as well as the extra child + criterion, equivalent column sets, etc. + + """ + + # place a barrier on the destination such that + # replacement traversals won't ever dig into it. + # its internal structure remains fixed + # regardless of context. + dest_selectable = _shallow_annotate( + dest_selectable, + {'no_replacement_traverse':True}) + + primaryjoin, secondaryjoin, secondary = self.primaryjoin, \ + self.secondaryjoin, self.secondary + + # adjust the join condition for single table inheritance, + # in the case that the join is to a subclass + # this is analogous to the "_adjust_for_single_table_inheritance()" + # method in Query. + + single_crit = self.extra_child_criterion + if single_crit is not None: + if secondaryjoin is not None: + secondaryjoin = secondaryjoin & single_crit + else: + primaryjoin = primaryjoin & single_crit + + if aliased: + if secondary is not None: + secondary = secondary.alias() + primary_aliasizer = ClauseAdapter(secondary) + secondary_aliasizer = \ + ClauseAdapter(dest_selectable, + equivalents=self.child_equivalents).\ + chain(primary_aliasizer) + if source_selectable is not None: + primary_aliasizer = \ + ClauseAdapter(secondary).\ + chain(ClauseAdapter(source_selectable, + equivalents=self.parent_equivalents)) + secondaryjoin = \ + secondary_aliasizer.traverse(secondaryjoin) + else: + primary_aliasizer = ClauseAdapter(dest_selectable, + exclude_fn=lambda c: "local" in c._annotations, + equivalents=self.child_equivalents) + if source_selectable is not None: + primary_aliasizer.chain( + ClauseAdapter(source_selectable, + exclude_fn=lambda c: "remote" in c._annotations, + equivalents=self.parent_equivalents)) + secondary_aliasizer = None + + primaryjoin = primary_aliasizer.traverse(primaryjoin) + target_adapter = secondary_aliasizer or primary_aliasizer + target_adapter.exclude_fn = None + else: + target_adapter = None + return primaryjoin, secondaryjoin, secondary, target_adapter + +################# everything below is TODO ################################ + +def _create_lazy_clause(cls, prop, reverse_direction=False): + binds = util.column_dict() + lookup = util.column_dict() + equated_columns = util.column_dict() + + if reverse_direction and prop.secondaryjoin is None: + for l, r in prop.local_remote_pairs: + _list = lookup.setdefault(r, []) + _list.append((r, l)) + equated_columns[l] = r + else: + for l, r in prop.local_remote_pairs: + _list = lookup.setdefault(l, []) + _list.append((l, r)) + equated_columns[r] = l + + def col_to_bind(col): + if col in lookup: + for tobind, equated in lookup[col]: + if equated in binds: + return None + if col not in binds: + binds[col] = sql.bindparam(None, None, type_=col.type, unique=True) + return binds[col] + return None + + lazywhere = prop.primaryjoin + + if prop.secondaryjoin is None or not reverse_direction: + lazywhere = visitors.replacement_traverse( + lazywhere, {}, col_to_bind) + + if prop.secondaryjoin is not None: + secondaryjoin = prop.secondaryjoin + if reverse_direction: + secondaryjoin = visitors.replacement_traverse( + secondaryjoin, {}, col_to_bind) + lazywhere = sql.and_(lazywhere, secondaryjoin) + + bind_to_col = dict((binds[col].key, col) for col in binds) + + return lazywhere, bind_to_col, equated_columns + +def _sync_pairs_from_join(self, join_condition, primary): + """Determine a list of "source"/"destination" column pairs + based on the given join condition, as well as the + foreign keys argument. + + "source" would be a column referenced by a foreign key, + and "destination" would be the column who has a foreign key + reference to "source". + + """ + + fks = self._user_defined_foreign_keys + # locate pairs + eq_pairs = criterion_as_pairs(join_condition, + consider_as_foreign_keys=fks, + any_operator=self.viewonly) + + # couldn't find any fks, but we have + # "secondary" - assume the "secondary" columns + # are the fks + if not eq_pairs and \ + self.secondary is not None and \ + not fks: + fks = set(self.secondary.c) + eq_pairs = criterion_as_pairs(join_condition, + consider_as_foreign_keys=fks, + any_operator=self.viewonly) + + if eq_pairs: + util.warn("No ForeignKey objects were present " + "in secondary table '%s'. Assumed referenced " + "foreign key columns %s for join condition '%s' " + "on relationship %s" % ( + self.secondary.description, + ", ".join(sorted(["'%s'" % col for col in fks])), + join_condition, + self + )) + + # Filter out just to columns that are mapped. + # If viewonly, allow pairs where the FK col + # was part of "foreign keys" - the column it references + # may be in an un-mapped table - see + # test.orm.test_relationships.ViewOnlyComplexJoin.test_basic + # for an example of this. + eq_pairs = [(l, r) for (l, r) in eq_pairs + if self._columns_are_mapped(l, r) + or self.viewonly and + r in fks] + + if eq_pairs: + return eq_pairs + + # from here below is just determining the best error message + # to report. Check for a join condition using any operator + # (not just ==), perhaps they need to turn on "viewonly=True". + if not self.viewonly and criterion_as_pairs(join_condition, + consider_as_foreign_keys=self._user_defined_foreign_keys, + any_operator=True): + + err = "Could not locate any "\ + "foreign-key-equated, locally mapped column "\ + "pairs for %s "\ + "condition '%s' on relationship %s." % ( + primary and 'primaryjoin' or 'secondaryjoin', + join_condition, + self + ) + + if not self._user_defined_foreign_keys: + err += " Ensure that the "\ + "referencing Column objects have a "\ + "ForeignKey present, or are otherwise part "\ + "of a ForeignKeyConstraint on their parent "\ + "Table, or specify the foreign_keys parameter "\ + "to this relationship." + + err += " For more "\ + "relaxed rules on join conditions, the "\ + "relationship may be marked as viewonly=True." + + raise sa_exc.ArgumentError(err) + else: + if self._user_defined_foreign_keys: + raise sa_exc.ArgumentError("Could not determine " + "relationship direction for %s condition " + "'%s', on relationship %s, using manual " + "'foreign_keys' setting. Do the columns " + "in 'foreign_keys' represent all, and " + "only, the 'foreign' columns in this join " + "condition? Does the %s Table already " + "have adequate ForeignKey and/or " + "ForeignKeyConstraint objects established " + "(in which case 'foreign_keys' is usually " + "unnecessary)?" + % ( + primary and 'primaryjoin' or 'secondaryjoin', + join_condition, + self, + primary and 'mapped' or 'secondary' + )) + else: + raise sa_exc.ArgumentError("Could not determine " + "relationship direction for %s condition " + "'%s', on relationship %s. Ensure that the " + "referencing Column objects have a " + "ForeignKey present, or are otherwise part " + "of a ForeignKeyConstraint on their parent " + "Table, or specify the foreign_keys parameter " + "to this relationship." + % ( + primary and 'primaryjoin' or 'secondaryjoin', + join_condition, + self + )) + +def _determine_synchronize_pairs(self): + """Resolve 'primary'/foreign' column pairs from the primaryjoin + and secondaryjoin arguments. + + """ + if self.local_remote_pairs: + if not self._user_defined_foreign_keys: + raise sa_exc.ArgumentError( + "foreign_keys argument is " + "required with _local_remote_pairs argument") + self.synchronize_pairs = [] + for l, r in self.local_remote_pairs: + if r in self._user_defined_foreign_keys: + self.synchronize_pairs.append((l, r)) + elif l in self._user_defined_foreign_keys: + self.synchronize_pairs.append((r, l)) + else: + self.synchronize_pairs = self._sync_pairs_from_join( + self.primaryjoin, + True) + + self._calculated_foreign_keys = util.column_set( + r for (l, r) in + self.synchronize_pairs) + + if self.secondaryjoin is not None: + self.secondary_synchronize_pairs = self._sync_pairs_from_join( + self.secondaryjoin, + False) + self._calculated_foreign_keys.update( + r for (l, r) in + self.secondary_synchronize_pairs) + else: + self.secondary_synchronize_pairs = None + +def _determine_direction(self): + """Determine if this relationship is one to many, many to one, + many to many. + + This is derived from the primaryjoin, presence of "secondary", + and in the case of self-referential the "remote side". + + """ + if self.secondaryjoin is not None: + self.direction = MANYTOMANY + elif self._refers_to_parent_table(): + + # self referential defaults to ONETOMANY unless the "remote" + # side is present and does not reference any foreign key + # columns + + if self.local_remote_pairs: + remote = [r for (l, r) in self.local_remote_pairs] + elif self.remote_side: + remote = self.remote_side + else: + remote = None + if not remote or self._calculated_foreign_keys.difference(l for (l, + r) in self.synchronize_pairs).intersection(remote): + self.direction = ONETOMANY + else: + self.direction = MANYTOONE + else: + parentcols = util.column_set(self.parent.mapped_table.c) + targetcols = util.column_set(self.mapper.mapped_table.c) + + # fk collection which suggests ONETOMANY. + onetomany_fk = targetcols.intersection( + self._calculated_foreign_keys) + + # fk collection which suggests MANYTOONE. + + manytoone_fk = parentcols.intersection( + self._calculated_foreign_keys) + + if onetomany_fk and manytoone_fk: + # fks on both sides. do the same test only based on the + # local side. + referents = [c for (c, f) in self.synchronize_pairs] + onetomany_local = parentcols.intersection(referents) + manytoone_local = targetcols.intersection(referents) + + if onetomany_local and not manytoone_local: + self.direction = ONETOMANY + elif manytoone_local and not onetomany_local: + self.direction = MANYTOONE + else: + raise sa_exc.ArgumentError( + "Can't determine relationship" + " direction for relationship '%s' - foreign " + "key columns are present in both the parent " + "and the child's mapped tables. Specify " + "'foreign_keys' argument." % self) + elif onetomany_fk: + self.direction = ONETOMANY + elif manytoone_fk: + self.direction = MANYTOONE + else: + raise sa_exc.ArgumentError("Can't determine relationship " + "direction for relationship '%s' - foreign " + "key columns are present in neither the parent " + "nor the child's mapped tables" % self) + + if self.cascade.delete_orphan and not self.single_parent \ + and (self.direction is MANYTOMANY or self.direction + is MANYTOONE): + util.warn('On %s, delete-orphan cascade is not supported ' + 'on a many-to-many or many-to-one relationship ' + 'when single_parent is not set. Set ' + 'single_parent=True on the relationship().' + % self) + if self.direction is MANYTOONE and self.passive_deletes: + util.warn("On %s, 'passive_deletes' is normally configured " + "on one-to-many, one-to-one, many-to-many " + "relationships only." + % self) + +def _determine_local_remote_pairs(self): + """Determine pairs of columns representing "local" to + "remote", where "local" columns are on the parent mapper, + "remote" are on the target mapper. + + These pairs are used on the load side only to generate + lazy loading clauses. + + """ + if not self.local_remote_pairs and not self.remote_side: + # the most common, trivial case. Derive + # local/remote pairs from the synchronize pairs. + eq_pairs = util.unique_list( + self.synchronize_pairs + + (self.secondary_synchronize_pairs or [])) + if self.direction is MANYTOONE: + self.local_remote_pairs = [(r, l) for l, r in eq_pairs] + else: + self.local_remote_pairs = eq_pairs + + # "remote_side" specified, derive from the primaryjoin + # plus remote_side, similarly to how synchronize_pairs + # were determined. + elif self.remote_side: + if self.local_remote_pairs: + raise sa_exc.ArgumentError('remote_side argument is ' + 'redundant against more detailed ' + '_local_remote_side argument.') + if self.direction is MANYTOONE: + self.local_remote_pairs = [(r, l) for (l, r) in + criterion_as_pairs(self.primaryjoin, + consider_as_referenced_keys=self.remote_side, + any_operator=True)] + + else: + self.local_remote_pairs = \ + criterion_as_pairs(self.primaryjoin, + consider_as_foreign_keys=self.remote_side, + any_operator=True) + if not self.local_remote_pairs: + raise sa_exc.ArgumentError('Relationship %s could ' + 'not determine any local/remote column ' + 'pairs from remote side argument %r' + % (self, self.remote_side)) + # else local_remote_pairs were sent explcitly via + # ._local_remote_pairs. + + # create local_side/remote_side accessors + self.local_side = util.ordered_column_set( + l for l, r in self.local_remote_pairs) + self.remote_side = util.ordered_column_set( + r for l, r in self.local_remote_pairs) + + # check that the non-foreign key column in the local/remote + # collection is mapped. The foreign key + # which the individual mapped column references directly may + # itself be in a non-mapped table; see + # test.orm.test_relationships.ViewOnlyComplexJoin.test_basic + # for an example of this. + if self.direction is ONETOMANY: + for col in self.local_side: + if not self._columns_are_mapped(col): + raise sa_exc.ArgumentError( + "Local column '%s' is not " + "part of mapping %s. Specify remote_side " + "argument to indicate which column lazy join " + "condition should compare against." % (col, + self.parent)) + elif self.direction is MANYTOONE: + for col in self.remote_side: + if not self._columns_are_mapped(col): + raise sa_exc.ArgumentError( + "Remote column '%s' is not " + "part of mapping %s. Specify remote_side " + "argument to indicate which column lazy join " + "condition should bind." % (col, self.mapper)) + + count = [0] + def clone(elem): + if set(['local', 'remote']).intersection(elem._annotations): + return None + elif elem in self.local_side and elem in self.remote_side: + # TODO: OK this still sucks. this is basically, + # refuse, refuse, refuse the temptation to guess! + # but crap we really have to guess don't we. we + # might want to traverse here with cloned_traverse + # so we can see the binary exprs and do it at that + # level.... + if count[0] % 2 == 0: + elem = elem._annotate({'local':True}) + else: + elem = elem._annotate({'remote':True}) + count[0] += 1 + elif elem in self.local_side: + elem = elem._annotate({'local':True}) + elif elem in self.remote_side: + elem = elem._annotate({'remote':True}) + else: + elem = None + return elem + + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, {}, clone + ) + + +def _criterion_exists(self, criterion=None, **kwargs): + if getattr(self, '_of_type', None): + target_mapper = self._of_type + to_selectable = target_mapper._with_polymorphic_selectable + if self.property._is_self_referential: + to_selectable = to_selectable.alias() + + single_crit = target_mapper._single_table_criterion + if single_crit is not None: + if criterion is not None: + criterion = single_crit & criterion + else: + criterion = single_crit + else: + to_selectable = None + + if self.adapter: + source_selectable = self.__clause_element__() + else: + source_selectable = None + + pj, sj, source, dest, secondary, target_adapter = \ + self.property._create_joins(dest_polymorphic=True, + dest_selectable=to_selectable, + source_selectable=source_selectable) + + for k in kwargs: + crit = getattr(self.property.mapper.class_, k) == kwargs[k] + if criterion is None: + criterion = crit + else: + criterion = criterion & crit + + # annotate the *local* side of the join condition, in the case + # of pj + sj this is the full primaryjoin, in the case of just + # pj its the local side of the primaryjoin. + if sj is not None: + j = _orm_annotate(pj) & sj + else: + j = _orm_annotate(pj, exclude=self.property.remote_side) + + # MARKMARK + if criterion is not None and target_adapter: + # limit this adapter to annotated only? + criterion = target_adapter.traverse(criterion) + + # only have the "joined left side" of what we + # return be subject to Query adaption. The right + # side of it is used for an exists() subquery and + # should not correlate or otherwise reach out + # to anything in the enclosing query. + if criterion is not None: + criterion = criterion._annotate({'no_replacement_traverse': True}) + + crit = j & criterion + + return sql.exists([1], crit, from_obj=dest).\ + correlate(source._annotate({'_orm_adapt':True})) + + +def __negated_contains_or_equals(self, other): + if self.property.direction == MANYTOONE: + state = attributes.instance_state(other) + + def state_bindparam(x, state, col): + o = state.obj() # strong ref + return sql.bindparam(x, unique=True, callable_=lambda : \ + self.property.mapper._get_committed_attr_by_column(o, + col)) + + def adapt(col): + if self.adapter: + return self.adapter(col) + else: + return col + + if self.property._use_get: + return sql.and_(*[ + sql.or_( + adapt(x) != state_bindparam(adapt(x), state, y), + adapt(x) == None) + for (x, y) in self.property.local_remote_pairs]) + + criterion = sql.and_(*[x==y for (x, y) in + zip( + self.property.mapper.primary_key, + self.property.\ + mapper.\ + primary_key_from_instance(other)) + ]) + return ~self._criterion_exists(criterion) -- cgit v1.2.1 From c0d04a2d264d8233c0acf6edfaf74c3389d9701c Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 6 Feb 2012 19:49:26 -0500 Subject: this version has easy cases going well. hard cases not so much --- lib/sqlalchemy/orm/properties.py | 155 ++++++++++------- lib/sqlalchemy/orm/relationships.py | 333 +++++++++++++++++++++++++++++++----- 2 files changed, 385 insertions(+), 103 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index a590ad706..5b883a8f5 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -33,9 +33,9 @@ from descriptor_props import CompositeProperty, SynonymProperty, \ class ColumnProperty(StrategizedProperty): """Describes an object attribute that corresponds to a table column. - + Public constructor is the :func:`.orm.column_property` function. - + """ def __init__(self, *columns, **kwargs): @@ -176,13 +176,13 @@ log.class_logger(ColumnProperty) class RelationshipProperty(StrategizedProperty): """Describes an object property that holds a single item or list of items that correspond to a related database table. - + Public constructor is the :func:`.orm.relationship` function. - + Of note here is the :class:`.RelationshipProperty.Comparator` class, which implements comparison operations for scalar- and collection-referencing mapped attributes. - + """ strategy_wildcard_key = 'relationship:*' @@ -292,7 +292,7 @@ class RelationshipProperty(StrategizedProperty): def __init__(self, prop, mapper, of_type=None, adapter=None): """Construction of :class:`.RelationshipProperty.Comparator` is internal to the ORM's attribute mechanics. - + """ self.prop = prop self.mapper = mapper @@ -331,10 +331,10 @@ class RelationshipProperty(StrategizedProperty): def of_type(self, cls): """Produce a construct that represents a particular 'subtype' of attribute for the parent class. - + Currently this is usable in conjunction with :meth:`.Query.join` and :meth:`.Query.outerjoin`. - + """ return RelationshipProperty.Comparator( self.property, @@ -344,7 +344,7 @@ class RelationshipProperty(StrategizedProperty): def in_(self, other): """Produce an IN clause - this is not implemented for :func:`~.orm.relationship`-based attributes at this time. - + """ raise NotImplementedError('in_() not yet supported for ' 'relationships. For a simple many-to-one, use ' @@ -361,15 +361,15 @@ class RelationshipProperty(StrategizedProperty): this will typically produce a clause such as:: - + mytable.related_id == - + Where ```` is the primary key of the given object. - + The ``==`` operator provides partial functionality for non- many-to-one comparisons: - + * Comparisons against collections are not supported. Use :meth:`~.RelationshipProperty.Comparator.contains`. * Compared to a scalar one-to-many, will produce a @@ -465,42 +465,42 @@ class RelationshipProperty(StrategizedProperty): def any(self, criterion=None, **kwargs): """Produce an expression that tests a collection against particular criterion, using EXISTS. - + An expression like:: - + session.query(MyClass).filter( MyClass.somereference.any(SomeRelated.x==2) ) - - + + Will produce a query like:: - + SELECT * FROM my_table WHERE EXISTS (SELECT 1 FROM related WHERE related.my_id=my_table.id AND related.x=2) - + Because :meth:`~.RelationshipProperty.Comparator.any` uses a correlated subquery, its performance is not nearly as good when compared against large target tables as that of using a join. - + :meth:`~.RelationshipProperty.Comparator.any` is particularly useful for testing for empty collections:: - + session.query(MyClass).filter( ~MyClass.somereference.any() ) - + will produce:: - + SELECT * FROM my_table WHERE NOT EXISTS (SELECT 1 FROM related WHERE related.my_id=my_table.id) - + :meth:`~.RelationshipProperty.Comparator.any` is only valid for collections, i.e. a :func:`.relationship` that has ``uselist=True``. For scalar references, use :meth:`~.RelationshipProperty.Comparator.has`. - + """ if not self.property.uselist: raise sa_exc.InvalidRequestError( @@ -515,14 +515,14 @@ class RelationshipProperty(StrategizedProperty): particular criterion, using EXISTS. An expression like:: - + session.query(MyClass).filter( MyClass.somereference.has(SomeRelated.x==2) ) - - + + Will produce a query like:: - + SELECT * FROM my_table WHERE EXISTS (SELECT 1 FROM related WHERE related.id==my_table.related_id AND related.x=2) @@ -531,12 +531,12 @@ class RelationshipProperty(StrategizedProperty): a correlated subquery, its performance is not nearly as good when compared against large target tables as that of using a join. - + :meth:`~.RelationshipProperty.Comparator.has` is only valid for scalar references, i.e. a :func:`.relationship` that has ``uselist=False``. For collection references, use :meth:`~.RelationshipProperty.Comparator.any`. - + """ if self.property.uselist: raise sa_exc.InvalidRequestError( @@ -547,44 +547,44 @@ class RelationshipProperty(StrategizedProperty): def contains(self, other, **kwargs): """Return a simple expression that tests a collection for containment of a particular item. - + :meth:`~.RelationshipProperty.Comparator.contains` is only valid for a collection, i.e. a :func:`~.orm.relationship` that implements one-to-many or many-to-many with ``uselist=True``. - + When used in a simple one-to-many context, an expression like:: - + MyClass.contains(other) - + Produces a clause like:: - + mytable.id == - + Where ```` is the value of the foreign key attribute on ``other`` which refers to the primary key of its parent object. From this it follows that :meth:`~.RelationshipProperty.Comparator.contains` is very useful when used with simple one-to-many operations. - + For many-to-many operations, the behavior of :meth:`~.RelationshipProperty.Comparator.contains` has more caveats. The association table will be rendered in the statement, producing an "implicit" join, that is, includes multiple tables in the FROM clause which are equated in the WHERE clause:: - + query(MyClass).filter(MyClass.contains(other)) - + Produces a query like:: - + SELECT * FROM my_table, my_association_table AS my_association_table_1 WHERE my_table.id = my_association_table_1.parent_id AND my_association_table_1.child_id = - + Where ```` would be the primary key of ``other``. From the above, it is clear that :meth:`~.RelationshipProperty.Comparator.contains` @@ -598,7 +598,7 @@ class RelationshipProperty(StrategizedProperty): a less-performant alternative using EXISTS, or refer to :meth:`.Query.outerjoin` as well as :ref:`ormtutorial_joins` for more details on constructing outer joins. - + """ if not self.property.uselist: raise sa_exc.InvalidRequestError( @@ -649,19 +649,19 @@ class RelationshipProperty(StrategizedProperty): """Implement the ``!=`` operator. In a many-to-one context, such as:: - + MyClass.some_prop != - + This will typically produce a clause such as:: - + mytable.related_id != - + Where ```` is the primary key of the given object. - + The ``!=`` operator provides partial functionality for non- many-to-one comparisons: - + * Comparisons against collections are not supported. Use :meth:`~.RelationshipProperty.Comparator.contains` @@ -682,7 +682,7 @@ class RelationshipProperty(StrategizedProperty): membership tests. * Comparisons against ``None`` given in a one-to-many or many-to-many context produce an EXISTS clause. - + """ if isinstance(other, (NoneType, expression._Null)): if self.property.direction == MANYTOONE: @@ -880,9 +880,9 @@ class RelationshipProperty(StrategizedProperty): def mapper(self): """Return the targeted :class:`.Mapper` for this :class:`.RelationshipProperty`. - + This is a lazy-initializing static attribute. - + """ if isinstance(self.argument, type): mapper_ = mapper.class_mapper(self.argument, @@ -914,14 +914,43 @@ class RelationshipProperty(StrategizedProperty): def do_init(self): self._check_conflicts() self._process_dependent_arguments() + self._create_new_thing() self._determine_joins() self._determine_synchronize_pairs() self._determine_direction() self._determine_local_remote_pairs() + self._test_new_thing() self._post_init() self._generate_backref() super(RelationshipProperty, self).do_init() + def _create_new_thing(self): + import relationships + self.jc = relationships.JoinCondition( + parent_selectable=self.parent.mapped_table, + child_selectable=self.mapper.mapped_table, + parent_local_selectable=self.parent.local_table, + child_local_selectable=self.mapper.local_table, + primaryjoin=self.primaryjoin, + secondary=self.secondary, + secondaryjoin=self.secondaryjoin, + parent_equivalents=self.parent._equivalent_columns, + child_equivalents=self.mapper._equivalent_columns, + consider_as_foreign_keys=self._user_defined_foreign_keys, + local_remote_pairs=self.local_remote_pairs, + remote_side=self.remote_side, + self_referential=self._is_self_referential, + prop=self, + support_sync=not self.viewonly, + can_be_synced_fn=self._columns_are_mapped + + ) + + def _test_new_thing(self): + assert self.jc.direction is self.direction + assert self.jc.remote_side == self.remote_side + assert self.jc.local_remote_pairs == self.local_remote_pairs + def _check_conflicts(self): """Test that this relationship is legal, warn about inheritance conflicts.""" @@ -952,9 +981,9 @@ class RelationshipProperty(StrategizedProperty): def _process_dependent_arguments(self): """Convert incoming configuration arguments to their proper form. - + Callables are resolved, ORM annotations removed. - + """ # accept callables for other attributes which may require # deferred initialization. This technique is used @@ -1011,10 +1040,10 @@ class RelationshipProperty(StrategizedProperty): def _determine_joins(self): """Determine the 'primaryjoin' and 'secondaryjoin' attributes, if not passed to the constructor already. - + This is based on analysis of the foreign key relationships between the parent and target mapped selectables. - + """ if self.secondaryjoin is not None and self.secondary is None: raise sa_exc.ArgumentError("Property '" + self.key @@ -1056,7 +1085,7 @@ class RelationshipProperty(StrategizedProperty): def _columns_are_mapped(self, *cols): """Return True if all columns in the given collection are mapped by the tables referenced by this :class:`.Relationship`. - + """ for c in cols: if self.secondary is not None \ @@ -1071,11 +1100,11 @@ class RelationshipProperty(StrategizedProperty): """Determine a list of "source"/"destination" column pairs based on the given join condition, as well as the foreign keys argument. - + "source" would be a column referenced by a foreign key, and "destination" would be the column who has a foreign key reference to "source". - + """ fks = self._user_defined_foreign_keys @@ -1186,7 +1215,7 @@ class RelationshipProperty(StrategizedProperty): def _determine_synchronize_pairs(self): """Resolve 'primary'/foreign' column pairs from the primaryjoin and secondaryjoin arguments. - + """ if self.local_remote_pairs: if not self._user_defined_foreign_keys: @@ -1221,10 +1250,10 @@ class RelationshipProperty(StrategizedProperty): def _determine_direction(self): """Determine if this relationship is one to many, many to one, many to many. - + This is derived from the primaryjoin, presence of "secondary", and in the case of self-referential the "remote side". - + """ if self.secondaryjoin is not None: self.direction = MANYTOMANY @@ -1304,7 +1333,7 @@ class RelationshipProperty(StrategizedProperty): """Determine pairs of columns representing "local" to "remote", where "local" columns are on the parent mapper, "remote" are on the target mapper. - + These pairs are used on the load side only to generate lazy loading clauses. diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index bb70f8a11..9aebc9f8a 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -33,29 +33,30 @@ class JoinCondition(object): consider_as_foreign_keys=None, local_remote_pairs=None, remote_side=None, - extra_child_criterion=None, self_referential=False, prop=None, - support_sync=True + support_sync=True, + can_be_synced_fn=lambda c: True ): self.parent_selectable = parent_selectable self.parent_local_selectable = parent_local_selectable self.child_selectable = child_selectable - self.child_local_selecatble = child_local_selectable + self.child_local_selectable = child_local_selectable self.parent_equivalents = parent_equivalents self.child_equivalents = child_equivalents self.primaryjoin = primaryjoin self.secondaryjoin = secondaryjoin self.secondary = secondary - self.extra_child_criterion = extra_child_criterion self.consider_as_foreign_keys = consider_as_foreign_keys - self.local_remote_pairs = local_remote_pairs - self.remote_side = remote_side + self._local_remote_pairs = local_remote_pairs + self._remote_side = remote_side self.prop = prop self.self_referential = self_referential self.support_sync = support_sync + self.can_be_synced_fn = can_be_synced_fn self._determine_joins() self._parse_joins() + self._determine_direction() def _determine_joins(self): """Determine the 'primaryjoin' and 'secondaryjoin' attributes, @@ -89,7 +90,7 @@ class JoinCondition(object): join_condition( self.parent_selectable, self.secondary, - a_subset=parent_local_selectable) + a_subset=self.parent_local_selectable) else: if self.primaryjoin is None: self.primaryjoin = \ @@ -108,14 +109,32 @@ class JoinCondition(object): def _parse_joins(self): """Apply 'remote', 'local' and 'foreign' annotations to the primary and secondary join conditions. - + """ parentcols = util.column_set(self.parent_selectable.c) targetcols = util.column_set(self.child_selectable.c) + if self.secondary is not None: + secondarycols = util.column_set(self.secondary.c) + else: + secondarycols = set() def col_is(a, b): return a.compare(b) + def refers_to_parent_table(binary): + pt = self.parent_selectable + mt = self.child_selectable + c, f = binary.left, binary.right + if ( + pt.is_derived_from(c.table) and \ + pt.is_derived_from(f.table) and \ + mt.is_derived_from(c.table) and \ + mt.is_derived_from(f.table) + ): + return True + else: + return False + def is_foreign(a, b): if self.consider_as_foreign_keys: if a in self.consider_as_foreign_keys and ( @@ -134,12 +153,49 @@ class JoinCondition(object): return a elif b.references(a): return b + elif secondarycols: + if a in secondarycols and b not in secondarycols: + return a + elif b in secondarycols and a not in secondarycols: + return b - any_operator = not self.support_sync + def _annotate_fk(binary, switch): + if switch: + right, left = binary.left, binary.right + else: + left, right = binary.left, binary.right + can_be_synced = self.can_be_synced_fn(left) + left = left._annotate({ + "equated":binary.operator is operators.eq, + "can_be_synced":can_be_synced and \ + binary.operator is operators.eq + }) + right = right._annotate({ + "equated":binary.operator is operators.eq, + "referent":True + }) + if switch: + binary.right, binary.left = left, right + else: + binary.left, binary.right = left, right + + def _annotate_remote(binary, switch): + if switch: + right, left = binary.left, binary.right + else: + left, right = binary.left, binary.right + left = left._annotate( + {"remote":True}) + if right in parentcols or \ + secondarycols and right in targetcols: + right = right._annotate( + {"local":True}) + if switch: + binary.right, binary.left = left, right + else: + binary.left, binary.right = left, right def visit_binary(binary): - #if not any_operator and binary.operator is not operators.eq: - # return if not isinstance(binary.left, sql.ColumnElement) or \ not isinstance(binary.right, sql.ColumnElement): return @@ -156,49 +212,247 @@ class JoinCondition(object): {"foreign":True}) # TODO: when the two cols are the same. + has_foreign = False + if "foreign" in binary.left._annotations: + _annotate_fk(binary, False) + has_foreign = True + if "foreign" in binary.right._annotations: + _annotate_fk(binary, True) + has_foreign = True + if "remote" not in binary.left._annotations and \ "remote" not in binary.right._annotations: - if self.local_remote_pairs: + if self._local_remote_pairs: raise NotImplementedError() - elif self.remote_side: - raise NotImplementedError() - elif self.self_referential: - # assume one to many - FKs are "Remote" + elif self._remote_side: + if binary.left in self._remote_side: + _annotate_remote(binary, False) + elif binary.right in self._remote_side: + _annotate_remote(binary, True) + elif refers_to_parent_table(binary): + # assume one to many - FKs are "remote" if "foreign" in binary.left._annotations: - binary.left = binary.left._annotate( - {"remote":True}) - if binary.right in parentcols: - binary.right = binary.right._annotate( - {"local":True}) + _annotate_remote(binary, False) elif "foreign" in binary.right._annotations: - binary.right = binary.right._annotate( - {"remote":True}) - if binary.left in parentcols: - binary.left = binary.left._annotate( - {"local":True}) + _annotate_remote(binary, True) + elif secondarycols: + if binary.left in secondarycols: + _annotate_remote(binary, False) + elif binary.right in secondarycols: + _annotate_remote(binary, True) else: - if binary.left in targetcols: - binary.left = binary.left._annotate( - {"remote":True}) - if binary.right in parentcols: - binary.right = binary.right._annotate( - {"local":True}) - elif binary.right in targetcols: - binary.right = binary.right._annotate( - {"remote":True}) - if binary.left in parentcols: - binary.left = binary.left._annotate( - {"local":True}) + if binary.left in targetcols and has_foreign: + _annotate_remote(binary, False) + elif binary.right in targetcols and has_foreign: + _annotate_remote(binary, True) self.primaryjoin = visitors.cloned_traverse( self.primaryjoin, {}, {"binary":visit_binary} ) + if self.secondaryjoin is not None: + self.secondaryjoin = visitors.cloned_traverse( + self.secondaryjoin, + {}, + {"binary":visit_binary} + ) + self._check_foreign_cols( + self.primaryjoin, True) + if self.secondaryjoin is not None: + self._check_foreign_cols( + self.secondaryjoin, False) + + + def _check_foreign_cols(self, join_condition, primary): + """Check the foreign key columns collected and emit error messages.""" + # TODO: don't worry, we can simplify this once we + # encourage configuration via direct annotation + + can_sync = False + + foreign_cols = self._gather_columns_with_annotation( + join_condition, "foreign") + + has_foreign = bool(foreign_cols) + + if self.support_sync: + for col in foreign_cols: + if col._annotations.get("can_be_synced"): + can_sync = True + break + + if self.support_sync and can_sync or \ + (not self.support_sync and has_foreign): + return + + # from here below is just determining the best error message + # to report. Check for a join condition using any operator + # (not just ==), perhaps they need to turn on "viewonly=True". + if self.support_sync and has_foreign and not can_sync: + + err = "Could not locate any "\ + "foreign-key-equated, locally mapped column "\ + "pairs for %s "\ + "condition '%s' on relationship %s." % ( + primary and 'primaryjoin' or 'secondaryjoin', + join_condition, + self.prop + ) + + # TODO: this needs to be changed to detect that + # annotations were present and whatnot. the future + # foreignkey(col) annotation will cover establishing + # the col as foreign to it's mate + if not self.consider_as_foreign_keys: + err += " Ensure that the "\ + "referencing Column objects have a "\ + "ForeignKey present, or are otherwise part "\ + "of a ForeignKeyConstraint on their parent "\ + "Table, or specify the foreign_keys parameter "\ + "to this relationship." + + err += " For more "\ + "relaxed rules on join conditions, the "\ + "relationship may be marked as viewonly=True." + + raise sa_exc.ArgumentError(err) + else: + if self.consider_as_foreign_keys: + raise sa_exc.ArgumentError("Could not determine " + "relationship direction for %s condition " + "'%s', on relationship %s, using manual " + "'foreign_keys' setting. Do the columns " + "in 'foreign_keys' represent all, and " + "only, the 'foreign' columns in this join " + "condition? Does the %s Table already " + "have adequate ForeignKey and/or " + "ForeignKeyConstraint objects established " + "(in which case 'foreign_keys' is usually " + "unnecessary)?" + % ( + primary and 'primaryjoin' or 'secondaryjoin', + join_condition, + self.prop, + primary and 'mapped' or 'secondary' + )) + else: + raise sa_exc.ArgumentError("Could not determine " + "relationship direction for %s condition " + "'%s', on relationship %s. Ensure that the " + "referencing Column objects have a " + "ForeignKey present, or are otherwise part " + "of a ForeignKeyConstraint on their parent " + "Table, or specify the foreign_keys parameter " + "to this relationship." + % ( + primary and 'primaryjoin' or 'secondaryjoin', + join_condition, + self.prop + )) + + def _determine_direction(self): + """Determine if this relationship is one to many, many to one, + many to many. + + """ + if self.secondaryjoin is not None: + self.direction = MANYTOMANY + else: + parentcols = util.column_set(self.parent_selectable.c) + targetcols = util.column_set(self.child_selectable.c) + + # fk collection which suggests ONETOMANY. + onetomany_fk = targetcols.intersection( + self.foreign_key_columns) + + # fk collection which suggests MANYTOONE. + + manytoone_fk = parentcols.intersection( + self.foreign_key_columns) + + if onetomany_fk and manytoone_fk: + # fks on both sides. test for overlap of local/remote + # with foreign key + onetomany_local = self.remote_side.intersection(self.foreign_key_columns) + manytoone_local = self.local_columns.intersection(self.foreign_key_columns) + if onetomany_local and not manytoone_local: + self.direction = ONETOMANY + elif manytoone_local and not onetomany_local: + self.direction = MANYTOONE + else: + raise sa_exc.ArgumentError( + "Can't determine relationship" + " direction for relationship '%s' - foreign " + "key columns are present in both the parent " + "and the child's mapped tables. Specify " + "'foreign_keys' argument." % self.prop) + elif onetomany_fk: + self.direction = ONETOMANY + elif manytoone_fk: + self.direction = MANYTOONE + else: + raise sa_exc.ArgumentError("Can't determine relationship " + "direction for relationship '%s' - foreign " + "key columns are present in neither the parent " + "nor the child's mapped tables" % self.prop) + + @util.memoized_property + def remote_columns(self): + return self._gather_join_annotations("remote") + + remote_side = remote_columns + + @util.memoized_property + def local_columns(self): + return self._gather_join_annotations("local") + + @util.memoized_property + def foreign_key_columns(self): + return self._gather_join_annotations("foreign") + + @util.memoized_property + def referent_columns(self): + return self._gather_join_annotations("referent") + + def _gather_join_annotations(self, annotation): + s = set( + self._gather_columns_with_annotation(self.primaryjoin, + annotation) + ) + if self.secondaryjoin is not None: + s.update( + self._gather_columns_with_annotation(self.secondaryjoin, + annotation) + ) + return s + + def _gather_columns_with_annotation(self, clause, *annotation): + annotation = set(annotation) + return set([ + col for col in visitors.iterate(clause, {}) + if annotation.issubset(col._annotations) + ]) + + @util.memoized_property + def local_remote_pairs(self): + lrp = [] + def visit_binary(binary): + if "remote" in binary.right._annotations and \ + "local" in binary.left._annotations: + lrp.append((binary.left, binary.right)) + elif "remote" in binary.left._annotations and \ + "local" in binary.right._annotations: + lrp.append((binary.right, binary.left)) + visitors.traverse(self.primaryjoin, {}, {"binary":visit_binary}) + if self.secondaryjoin is not None: + visitors.traverse(self.secondaryjoin, {}, {"binary":visit_binary}) + return lrp def join_targets(self, source_selectable, dest_selectable, - aliased): + aliased, + single_crit=None): """Given a source and destination selectable, create a join between them. @@ -225,7 +479,6 @@ class JoinCondition(object): # this is analogous to the "_adjust_for_single_table_inheritance()" # method in Query. - single_crit = self.extra_child_criterion if single_crit is not None: if secondaryjoin is not None: secondaryjoin = secondaryjoin & single_crit -- cgit v1.2.1 From f39c43083a612fdb77dbb3eb2c297cde9662fe81 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 6 Feb 2012 20:47:18 -0500 Subject: local_remote_pairs/remote_side are comparing against existing 100% --- lib/sqlalchemy/orm/properties.py | 1 - lib/sqlalchemy/orm/relationships.py | 301 +++++++----------------------------- 2 files changed, 53 insertions(+), 249 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 5b883a8f5..9bab0c2f4 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -943,7 +943,6 @@ class RelationshipProperty(StrategizedProperty): prop=self, support_sync=not self.viewonly, can_be_synced_fn=self._columns_are_mapped - ) def _test_new_thing(self): diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index 9aebc9f8a..02eab9c2d 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -147,23 +147,25 @@ class JoinCondition(object): a not in self.consider_as_foreign_keys ): return b - elif isinstance(a, schema.Column) and \ + + if isinstance(a, schema.Column) and \ isinstance(b, schema.Column): if a.references(b): return a elif b.references(a): return b - elif secondarycols: + + if secondarycols: if a in secondarycols and b not in secondarycols: return a elif b in secondarycols and a not in secondarycols: return b - def _annotate_fk(binary, switch): - if switch: - right, left = binary.left, binary.right - else: - left, right = binary.left, binary.right + def _run_w_switch(binary, fn): + binary.left, binary.right = fn(binary, binary.left, binary.right) + binary.right, binary.left = fn(binary, binary.right, binary.left) + + def _annotate_fk(binary, left, right): can_be_synced = self.can_be_synced_fn(left) left = left._annotate({ "equated":binary.operator is operators.eq, @@ -174,26 +176,16 @@ class JoinCondition(object): "equated":binary.operator is operators.eq, "referent":True }) - if switch: - binary.right, binary.left = left, right - else: - binary.left, binary.right = left, right + return left, right - def _annotate_remote(binary, switch): - if switch: - right, left = binary.left, binary.right - else: - left, right = binary.left, binary.right + def _annotate_remote(binary, left, right): left = left._annotate( {"remote":True}) if right in parentcols or \ - secondarycols and right in targetcols: + right in targetcols: right = right._annotate( {"local":True}) - if switch: - binary.right, binary.left = left, right - else: - binary.left, binary.right = left, right + return left, right def visit_binary(binary): if not isinstance(binary.left, sql.ColumnElement) or \ @@ -214,37 +206,39 @@ class JoinCondition(object): has_foreign = False if "foreign" in binary.left._annotations: - _annotate_fk(binary, False) + binary.left, binary.right = _annotate_fk( + binary, binary.left, binary.right) has_foreign = True if "foreign" in binary.right._annotations: - _annotate_fk(binary, True) + binary.right, binary.left = _annotate_fk( + binary, binary.right, binary.left) has_foreign = True if "remote" not in binary.left._annotations and \ "remote" not in binary.right._annotations: - if self._local_remote_pairs: - raise NotImplementedError() - elif self._remote_side: - if binary.left in self._remote_side: - _annotate_remote(binary, False) - elif binary.right in self._remote_side: - _annotate_remote(binary, True) - elif refers_to_parent_table(binary): - # assume one to many - FKs are "remote" - if "foreign" in binary.left._annotations: - _annotate_remote(binary, False) - elif "foreign" in binary.right._annotations: - _annotate_remote(binary, True) - elif secondarycols: - if binary.left in secondarycols: - _annotate_remote(binary, False) - elif binary.right in secondarycols: - _annotate_remote(binary, True) - else: - if binary.left in targetcols and has_foreign: - _annotate_remote(binary, False) - elif binary.right in targetcols and has_foreign: - _annotate_remote(binary, True) + + def go(binary, left, right): + if self._local_remote_pairs: + raise NotImplementedError() + elif self._remote_side: + if left in self._remote_side: + return _annotate_remote(binary, left, right) + elif refers_to_parent_table(binary): + # assume one to many - FKs are "remote" + if "foreign" in left._annotations: + return _annotate_remote(binary, left, right) + elif secondarycols: + if left in secondarycols: + return _annotate_remote(binary, left, right) + else: + # TODO: to support the X->Y->Z case + # we might need to look at parentcols + # and annotate "local" separately... + if left in targetcols and has_foreign \ + and right in parentcols or right in secondarycols: + return _annotate_remote(binary, left, right) + return left, right + _run_w_switch(binary, go) self.primaryjoin = visitors.cloned_traverse( self.primaryjoin, @@ -374,8 +368,15 @@ class JoinCondition(object): if onetomany_fk and manytoone_fk: # fks on both sides. test for overlap of local/remote # with foreign key - onetomany_local = self.remote_side.intersection(self.foreign_key_columns) - manytoone_local = self.local_columns.intersection(self.foreign_key_columns) + self_equated = self.remote_columns.intersection( + self.local_columns + ) + onetomany_local = self.remote_columns.\ + intersection(self.foreign_key_columns).\ + difference(self_equated) + manytoone_local = self.local_columns.\ + intersection(self.foreign_key_columns).\ + difference(self_equated) if onetomany_local and not manytoone_local: self.direction = ONETOMANY elif manytoone_local and not onetomany_local: @@ -436,18 +437,18 @@ class JoinCondition(object): @util.memoized_property def local_remote_pairs(self): - lrp = [] + lrp = util.OrderedSet() def visit_binary(binary): if "remote" in binary.right._annotations and \ "local" in binary.left._annotations: - lrp.append((binary.left, binary.right)) + lrp.add((binary.left, binary.right)) elif "remote" in binary.left._annotations and \ "local" in binary.right._annotations: - lrp.append((binary.right, binary.left)) + lrp.add((binary.right, binary.left)) visitors.traverse(self.primaryjoin, {}, {"binary":visit_binary}) if self.secondaryjoin is not None: visitors.traverse(self.secondaryjoin, {}, {"binary":visit_binary}) - return lrp + return list(lrp) def join_targets(self, source_selectable, dest_selectable, @@ -563,121 +564,6 @@ def _create_lazy_clause(cls, prop, reverse_direction=False): return lazywhere, bind_to_col, equated_columns -def _sync_pairs_from_join(self, join_condition, primary): - """Determine a list of "source"/"destination" column pairs - based on the given join condition, as well as the - foreign keys argument. - - "source" would be a column referenced by a foreign key, - and "destination" would be the column who has a foreign key - reference to "source". - - """ - - fks = self._user_defined_foreign_keys - # locate pairs - eq_pairs = criterion_as_pairs(join_condition, - consider_as_foreign_keys=fks, - any_operator=self.viewonly) - - # couldn't find any fks, but we have - # "secondary" - assume the "secondary" columns - # are the fks - if not eq_pairs and \ - self.secondary is not None and \ - not fks: - fks = set(self.secondary.c) - eq_pairs = criterion_as_pairs(join_condition, - consider_as_foreign_keys=fks, - any_operator=self.viewonly) - - if eq_pairs: - util.warn("No ForeignKey objects were present " - "in secondary table '%s'. Assumed referenced " - "foreign key columns %s for join condition '%s' " - "on relationship %s" % ( - self.secondary.description, - ", ".join(sorted(["'%s'" % col for col in fks])), - join_condition, - self - )) - - # Filter out just to columns that are mapped. - # If viewonly, allow pairs where the FK col - # was part of "foreign keys" - the column it references - # may be in an un-mapped table - see - # test.orm.test_relationships.ViewOnlyComplexJoin.test_basic - # for an example of this. - eq_pairs = [(l, r) for (l, r) in eq_pairs - if self._columns_are_mapped(l, r) - or self.viewonly and - r in fks] - - if eq_pairs: - return eq_pairs - - # from here below is just determining the best error message - # to report. Check for a join condition using any operator - # (not just ==), perhaps they need to turn on "viewonly=True". - if not self.viewonly and criterion_as_pairs(join_condition, - consider_as_foreign_keys=self._user_defined_foreign_keys, - any_operator=True): - - err = "Could not locate any "\ - "foreign-key-equated, locally mapped column "\ - "pairs for %s "\ - "condition '%s' on relationship %s." % ( - primary and 'primaryjoin' or 'secondaryjoin', - join_condition, - self - ) - - if not self._user_defined_foreign_keys: - err += " Ensure that the "\ - "referencing Column objects have a "\ - "ForeignKey present, or are otherwise part "\ - "of a ForeignKeyConstraint on their parent "\ - "Table, or specify the foreign_keys parameter "\ - "to this relationship." - - err += " For more "\ - "relaxed rules on join conditions, the "\ - "relationship may be marked as viewonly=True." - - raise sa_exc.ArgumentError(err) - else: - if self._user_defined_foreign_keys: - raise sa_exc.ArgumentError("Could not determine " - "relationship direction for %s condition " - "'%s', on relationship %s, using manual " - "'foreign_keys' setting. Do the columns " - "in 'foreign_keys' represent all, and " - "only, the 'foreign' columns in this join " - "condition? Does the %s Table already " - "have adequate ForeignKey and/or " - "ForeignKeyConstraint objects established " - "(in which case 'foreign_keys' is usually " - "unnecessary)?" - % ( - primary and 'primaryjoin' or 'secondaryjoin', - join_condition, - self, - primary and 'mapped' or 'secondary' - )) - else: - raise sa_exc.ArgumentError("Could not determine " - "relationship direction for %s condition " - "'%s', on relationship %s. Ensure that the " - "referencing Column objects have a " - "ForeignKey present, or are otherwise part " - "of a ForeignKeyConstraint on their parent " - "Table, or specify the foreign_keys parameter " - "to this relationship." - % ( - primary and 'primaryjoin' or 'secondaryjoin', - join_condition, - self - )) def _determine_synchronize_pairs(self): """Resolve 'primary'/foreign' column pairs from the primaryjoin @@ -714,87 +600,6 @@ def _determine_synchronize_pairs(self): else: self.secondary_synchronize_pairs = None -def _determine_direction(self): - """Determine if this relationship is one to many, many to one, - many to many. - - This is derived from the primaryjoin, presence of "secondary", - and in the case of self-referential the "remote side". - - """ - if self.secondaryjoin is not None: - self.direction = MANYTOMANY - elif self._refers_to_parent_table(): - - # self referential defaults to ONETOMANY unless the "remote" - # side is present and does not reference any foreign key - # columns - - if self.local_remote_pairs: - remote = [r for (l, r) in self.local_remote_pairs] - elif self.remote_side: - remote = self.remote_side - else: - remote = None - if not remote or self._calculated_foreign_keys.difference(l for (l, - r) in self.synchronize_pairs).intersection(remote): - self.direction = ONETOMANY - else: - self.direction = MANYTOONE - else: - parentcols = util.column_set(self.parent.mapped_table.c) - targetcols = util.column_set(self.mapper.mapped_table.c) - - # fk collection which suggests ONETOMANY. - onetomany_fk = targetcols.intersection( - self._calculated_foreign_keys) - - # fk collection which suggests MANYTOONE. - - manytoone_fk = parentcols.intersection( - self._calculated_foreign_keys) - - if onetomany_fk and manytoone_fk: - # fks on both sides. do the same test only based on the - # local side. - referents = [c for (c, f) in self.synchronize_pairs] - onetomany_local = parentcols.intersection(referents) - manytoone_local = targetcols.intersection(referents) - - if onetomany_local and not manytoone_local: - self.direction = ONETOMANY - elif manytoone_local and not onetomany_local: - self.direction = MANYTOONE - else: - raise sa_exc.ArgumentError( - "Can't determine relationship" - " direction for relationship '%s' - foreign " - "key columns are present in both the parent " - "and the child's mapped tables. Specify " - "'foreign_keys' argument." % self) - elif onetomany_fk: - self.direction = ONETOMANY - elif manytoone_fk: - self.direction = MANYTOONE - else: - raise sa_exc.ArgumentError("Can't determine relationship " - "direction for relationship '%s' - foreign " - "key columns are present in neither the parent " - "nor the child's mapped tables" % self) - - if self.cascade.delete_orphan and not self.single_parent \ - and (self.direction is MANYTOMANY or self.direction - is MANYTOONE): - util.warn('On %s, delete-orphan cascade is not supported ' - 'on a many-to-many or many-to-one relationship ' - 'when single_parent is not set. Set ' - 'single_parent=True on the relationship().' - % self) - if self.direction is MANYTOONE and self.passive_deletes: - util.warn("On %s, 'passive_deletes' is normally configured " - "on one-to-many, one-to-one, many-to-many " - "relationships only." - % self) def _determine_local_remote_pairs(self): """Determine pairs of columns representing "local" to -- cgit v1.2.1 From d1414ad20524c421aa78272c03dce5f839a0aab6 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 8 Feb 2012 10:14:36 -0500 Subject: simplify remote annotation significantly, and also catch the actual remote columns more accurately. --- lib/sqlalchemy/orm/properties.py | 27 ++++ lib/sqlalchemy/orm/relationships.py | 237 +++++++++++++++++------------------- lib/sqlalchemy/sql/expression.py | 4 + lib/sqlalchemy/sql/operators.py | 5 + 4 files changed, 148 insertions(+), 125 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 9bab0c2f4..953430162 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -949,6 +949,7 @@ class RelationshipProperty(StrategizedProperty): assert self.jc.direction is self.direction assert self.jc.remote_side == self.remote_side assert self.jc.local_remote_pairs == self.local_remote_pairs + pass def _check_conflicts(self): """Test that this relationship is legal, warn about @@ -1510,6 +1511,7 @@ class RelationshipProperty(StrategizedProperty): return strategy.use_get def _refers_to_parent_table(self): + alt = self._alt_refers_to_parent_table() pt = self.parent.mapped_table mt = self.mapper.mapped_table for c, f in self.synchronize_pairs: @@ -1519,10 +1521,35 @@ class RelationshipProperty(StrategizedProperty): mt.is_derived_from(c.table) and \ mt.is_derived_from(f.table) ): + assert alt return True else: + assert not alt return False + def _alt_refers_to_parent_table(self): + pt = self.parent.mapped_table + mt = self.mapper.mapped_table + result = [False] + def visit_binary(binary): + c, f = binary.left, binary.right + if ( + isinstance(c, expression.ColumnClause) and \ + isinstance(f, expression.ColumnClause) and \ + pt.is_derived_from(c.table) and \ + pt.is_derived_from(f.table) and \ + mt.is_derived_from(c.table) and \ + mt.is_derived_from(f.table) + ): + result[0] = True + + visitors.traverse( + self.primaryjoin, + {}, + {"binary":visit_binary} + ) + return result[0] + @util.memoized_property def _is_self_referential(self): return self.mapper.common_parent(self.parent) diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index 02eab9c2d..cb07f234a 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -19,6 +19,27 @@ from sqlalchemy.sql.util import ClauseAdapter, criterion_as_pairs, \ from sqlalchemy.sql import operators, expression, visitors from sqlalchemy.orm.interfaces import MANYTOMANY, MANYTOONE, ONETOMANY +def remote(expr): + return _annotate_columns(expr, {"remote":True}) + +def foreign(expr): + return _annotate_columns(expr, {"foreign":True}) + +def remote_foreign(expr): + return _annotate_columns(expr, {"foreign":True, + "remote":True}) + +def _annotate_columns(element, annotations): + def clone(elem): + if isinstance(elem, expression.ColumnClause): + elem = elem._annotate(annotations.copy()) + elem._copy_internals(clone=clone) + return elem + + if element is not None: + element = clone(element) + return element + class JoinCondition(object): def __init__(self, parent_selectable, @@ -55,7 +76,8 @@ class JoinCondition(object): self.support_sync = support_sync self.can_be_synced_fn = can_be_synced_fn self._determine_joins() - self._parse_joins() + self._annotate_fks() + self._annotate_remote() self._determine_direction() def _determine_joins(self): @@ -106,13 +128,7 @@ class JoinCondition(object): "'secondaryjoin' is needed as well." % self.prop) - def _parse_joins(self): - """Apply 'remote', 'local' and 'foreign' annotations - to the primary and secondary join conditions. - - """ - parentcols = util.column_set(self.parent_selectable.c) - targetcols = util.column_set(self.child_selectable.c) + def _annotate_fks(self): if self.secondary is not None: secondarycols = util.column_set(self.secondary.c) else: @@ -121,20 +137,6 @@ class JoinCondition(object): def col_is(a, b): return a.compare(b) - def refers_to_parent_table(binary): - pt = self.parent_selectable - mt = self.child_selectable - c, f = binary.left, binary.right - if ( - pt.is_derived_from(c.table) and \ - pt.is_derived_from(f.table) and \ - mt.is_derived_from(c.table) and \ - mt.is_derived_from(f.table) - ): - return True - else: - return False - def is_foreign(a, b): if self.consider_as_foreign_keys: if a in self.consider_as_foreign_keys and ( @@ -161,32 +163,19 @@ class JoinCondition(object): elif b in secondarycols and a not in secondarycols: return b - def _run_w_switch(binary, fn): - binary.left, binary.right = fn(binary, binary.left, binary.right) - binary.right, binary.left = fn(binary, binary.right, binary.left) - def _annotate_fk(binary, left, right): can_be_synced = self.can_be_synced_fn(left) left = left._annotate({ - "equated":binary.operator is operators.eq, + #"equated":binary.operator is operators.eq, "can_be_synced":can_be_synced and \ binary.operator is operators.eq }) right = right._annotate({ - "equated":binary.operator is operators.eq, + #"equated":binary.operator is operators.eq, "referent":True }) return left, right - def _annotate_remote(binary, left, right): - left = left._annotate( - {"remote":True}) - if right in parentcols or \ - right in targetcols: - right = right._annotate( - {"local":True}) - return left, right - def visit_binary(binary): if not isinstance(binary.left, sql.ColumnElement) or \ not isinstance(binary.right, sql.ColumnElement): @@ -204,41 +193,12 @@ class JoinCondition(object): {"foreign":True}) # TODO: when the two cols are the same. - has_foreign = False if "foreign" in binary.left._annotations: binary.left, binary.right = _annotate_fk( binary, binary.left, binary.right) - has_foreign = True if "foreign" in binary.right._annotations: binary.right, binary.left = _annotate_fk( binary, binary.right, binary.left) - has_foreign = True - - if "remote" not in binary.left._annotations and \ - "remote" not in binary.right._annotations: - - def go(binary, left, right): - if self._local_remote_pairs: - raise NotImplementedError() - elif self._remote_side: - if left in self._remote_side: - return _annotate_remote(binary, left, right) - elif refers_to_parent_table(binary): - # assume one to many - FKs are "remote" - if "foreign" in left._annotations: - return _annotate_remote(binary, left, right) - elif secondarycols: - if left in secondarycols: - return _annotate_remote(binary, left, right) - else: - # TODO: to support the X->Y->Z case - # we might need to look at parentcols - # and annotate "local" separately... - if left in targetcols and has_foreign \ - and right in parentcols or right in secondarycols: - return _annotate_remote(binary, left, right) - return left, right - _run_w_switch(binary, go) self.primaryjoin = visitors.cloned_traverse( self.primaryjoin, @@ -257,11 +217,63 @@ class JoinCondition(object): self._check_foreign_cols( self.secondaryjoin, False) + def _refers_to_parent_table(self): + pt = self.parent_selectable + mt = self.child_selectable + result = [False] + def visit_binary(binary): + c, f = binary.left, binary.right + if ( + isinstance(c, expression.ColumnClause) and \ + isinstance(f, expression.ColumnClause) and \ + pt.is_derived_from(c.table) and \ + pt.is_derived_from(f.table) and \ + mt.is_derived_from(c.table) and \ + mt.is_derived_from(f.table) + ): + result[0] = True + + visitors.traverse( + self.primaryjoin, + {}, + {"binary":visit_binary} + ) + return result[0] + + def _annotate_remote(self): + for col in visitors.iterate(self.primaryjoin, {}): + if "remote" in col._annotations: + return + + if self._local_remote_pairs: + raise NotImplementedError() + elif self._remote_side: + def repl(element): + if element in self._remote_side: + return element._annotate({"remote":True}) + elif self.secondary is not None: + def repl(element): + if self.secondary.c.contains_column(element): + return element._annotate({"remote":True}) + elif self._refers_to_parent_table(): + def repl(element): + # assume one to many - FKs are "remote" + if "foreign" in element._annotations: + return element._annotate({"remote":True}) + else: + def repl(element): + if self.child_selectable.c.contains_column(element): + return element._annotate({"remote":True}) + + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, {}, repl) + if self.secondaryjoin is not None: + self.secondaryjoin = visitors.replacement_traverse( + self.secondaryjoin, {}, repl) + def _check_foreign_cols(self, join_condition, primary): """Check the foreign key columns collected and emit error messages.""" - # TODO: don't worry, we can simplify this once we - # encourage configuration via direct annotation can_sync = False @@ -284,66 +296,30 @@ class JoinCondition(object): # to report. Check for a join condition using any operator # (not just ==), perhaps they need to turn on "viewonly=True". if self.support_sync and has_foreign and not can_sync: - - err = "Could not locate any "\ - "foreign-key-equated, locally mapped column "\ - "pairs for %s "\ - "condition '%s' on relationship %s." % ( + err = "Could not locate any simple equality expressions "\ + "involving foreign key columns for %s join condition "\ + "'%s' on relationship %s." % ( primary and 'primaryjoin' or 'secondaryjoin', join_condition, self.prop ) - - # TODO: this needs to be changed to detect that - # annotations were present and whatnot. the future - # foreignkey(col) annotation will cover establishing - # the col as foreign to it's mate - if not self.consider_as_foreign_keys: - err += " Ensure that the "\ - "referencing Column objects have a "\ - "ForeignKey present, or are otherwise part "\ - "of a ForeignKeyConstraint on their parent "\ - "Table, or specify the foreign_keys parameter "\ - "to this relationship." - - err += " For more "\ - "relaxed rules on join conditions, the "\ - "relationship may be marked as viewonly=True." + err += " Ensure that referencing columns are associated with a "\ + "ForeignKey or ForeignKeyConstraint, or are annotated "\ + "in the join condition with the foreign() annotation. "\ + "To allow comparison operators other than '==', "\ + "the relationship can be marked as viewonly=True." raise sa_exc.ArgumentError(err) else: - if self.consider_as_foreign_keys: - raise sa_exc.ArgumentError("Could not determine " - "relationship direction for %s condition " - "'%s', on relationship %s, using manual " - "'foreign_keys' setting. Do the columns " - "in 'foreign_keys' represent all, and " - "only, the 'foreign' columns in this join " - "condition? Does the %s Table already " - "have adequate ForeignKey and/or " - "ForeignKeyConstraint objects established " - "(in which case 'foreign_keys' is usually " - "unnecessary)?" - % ( - primary and 'primaryjoin' or 'secondaryjoin', - join_condition, - self.prop, - primary and 'mapped' or 'secondary' - )) - else: - raise sa_exc.ArgumentError("Could not determine " - "relationship direction for %s condition " - "'%s', on relationship %s. Ensure that the " - "referencing Column objects have a " - "ForeignKey present, or are otherwise part " - "of a ForeignKeyConstraint on their parent " - "Table, or specify the foreign_keys parameter " - "to this relationship." - % ( - primary and 'primaryjoin' or 'secondaryjoin', - join_condition, - self.prop - )) + err = "Could not locate any relevant foreign key columns "\ + "for %s join condition '%s' on relationship %s." % ( + primary and 'primaryjoin' or 'secondaryjoin', + join_condition, + self.prop + ) + err += "Ensure that referencing columns are associated with a "\ + "a ForeignKey or ForeignKeyConstraint, or are annotated "\ + "in the join condition with the foreign() annotation." def _determine_direction(self): """Determine if this relationship is one to many, many to one, @@ -399,14 +375,21 @@ class JoinCondition(object): "nor the child's mapped tables" % self.prop) @util.memoized_property - def remote_columns(self): + def liberal_remote_columns(self): + # this is temporary until we figure out + # which version of "remote" to use return self._gather_join_annotations("remote") + @util.memoized_property + def remote_columns(self): + return set([r for l, r in self.local_remote_pairs]) + #return self._gather_join_annotations("remote") + remote_side = remote_columns @util.memoized_property def local_columns(self): - return self._gather_join_annotations("local") + return set([l for l, r in self.local_remote_pairs]) @util.memoized_property def foreign_key_columns(self): @@ -440,10 +423,14 @@ class JoinCondition(object): lrp = util.OrderedSet() def visit_binary(binary): if "remote" in binary.right._annotations and \ - "local" in binary.left._annotations: + "remote" not in binary.left._annotations and \ + isinstance(binary.left, expression.ColumnClause) and \ + self.can_be_synced_fn(binary.left): lrp.add((binary.left, binary.right)) elif "remote" in binary.left._annotations and \ - "local" in binary.right._annotations: + "remote" not in binary.right._annotations and \ + isinstance(binary.right, expression.ColumnClause) and \ + self.can_be_synced_fn(binary.right): lrp.add((binary.right, binary.left)) visitors.traverse(self.primaryjoin, {}, {"binary":visit_binary}) if self.secondaryjoin is not None: diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py index 30e19bc68..72099a5f5 100644 --- a/lib/sqlalchemy/sql/expression.py +++ b/lib/sqlalchemy/sql/expression.py @@ -3384,6 +3384,10 @@ class _BinaryExpression(ColumnElement): except: raise TypeError("Boolean value of this clause is not defined") + @property + def is_comparison(self): + return operators.is_comparison(self.operator) + @property def _from_objects(self): return self.left._from_objects + self.right._from_objects diff --git a/lib/sqlalchemy/sql/operators.py b/lib/sqlalchemy/sql/operators.py index 89f0aaee1..b86b50db4 100644 --- a/lib/sqlalchemy/sql/operators.py +++ b/lib/sqlalchemy/sql/operators.py @@ -521,6 +521,11 @@ def nullslast_op(a): _commutative = set([eq, ne, add, mul]) +_comparison = set([eq, ne, lt, gt, ge, le]) + +def is_comparison(op): + return op in _comparison + def is_commutative(op): return op in _commutative -- cgit v1.2.1 From 83cd64f6cb12555cc76b6a19cb90247e08cc0547 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 8 Feb 2012 17:44:57 -0500 Subject: new test that illustrates the breakage of partial remote side when FKs are assumed --- lib/sqlalchemy/orm/properties.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 953430162..95a309357 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -947,8 +947,8 @@ class RelationshipProperty(StrategizedProperty): def _test_new_thing(self): assert self.jc.direction is self.direction - assert self.jc.remote_side == self.remote_side - assert self.jc.local_remote_pairs == self.local_remote_pairs + assert self.jc.remote_side.issuperset(self.remote_side) +# assert self.jc.local_remote_pairs.issuperset(self.local_remote_pairs) pass def _check_conflicts(self): -- cgit v1.2.1 From 91f4109dc3ec49686ba2393eb6b7bd9bb5b95fb3 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 8 Feb 2012 18:46:36 -0500 Subject: - hooks in the new object to RelationshipProperty, restores the "local" annotation. also added in sync pairs, etc. many-to-many is currently broken. --- lib/sqlalchemy/orm/properties.py | 417 +++--------------------------------- lib/sqlalchemy/orm/relationships.py | 128 +++++++---- 2 files changed, 114 insertions(+), 431 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 95a309357..77da33b5f 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -914,19 +914,15 @@ class RelationshipProperty(StrategizedProperty): def do_init(self): self._check_conflicts() self._process_dependent_arguments() - self._create_new_thing() - self._determine_joins() - self._determine_synchronize_pairs() - self._determine_direction() - self._determine_local_remote_pairs() - self._test_new_thing() + self._setup_join_conditions() + self._extra_determine_direction() self._post_init() self._generate_backref() super(RelationshipProperty, self).do_init() - def _create_new_thing(self): + def _setup_join_conditions(self): import relationships - self.jc = relationships.JoinCondition( + self._join_condition = jc = relationships.JoinCondition( parent_selectable=self.parent.mapped_table, child_selectable=self.mapper.mapped_table, parent_local_selectable=self.parent.local_table, @@ -944,12 +940,14 @@ class RelationshipProperty(StrategizedProperty): support_sync=not self.viewonly, can_be_synced_fn=self._columns_are_mapped ) - - def _test_new_thing(self): - assert self.jc.direction is self.direction - assert self.jc.remote_side.issuperset(self.remote_side) -# assert self.jc.local_remote_pairs.issuperset(self.local_remote_pairs) - pass + self.primaryjoin = jc.primaryjoin + self.secondaryjoin = jc.secondaryjoin + self.direction = jc.direction + self.local_remote_pairs = jc.local_remote_pairs + self.remote_side = jc.remote_columns + self.synchronize_pairs = jc.synchronize_pairs + self.secondary_synchronize_pairs = jc.secondary_synchronize_pairs + self._calculated_foreign_keys = jc.foreign_key_columns def _check_conflicts(self): """Test that this relationship is legal, warn about @@ -1037,284 +1035,7 @@ class RelationshipProperty(StrategizedProperty): (self.key, self.parent.class_) ) - def _determine_joins(self): - """Determine the 'primaryjoin' and 'secondaryjoin' attributes, - if not passed to the constructor already. - - This is based on analysis of the foreign key relationships - between the parent and target mapped selectables. - - """ - if self.secondaryjoin is not None and self.secondary is None: - raise sa_exc.ArgumentError("Property '" + self.key - + "' specified with secondary join condition but " - "no secondary argument") - - # if join conditions were not specified, figure them out based - # on foreign keys - - def _search_for_join(mapper, table): - # find a join between the given mapper's mapped table and - # the given table. will try the mapper's local table first - # for more specificity, then if not found will try the more - # general mapped table, which in the case of inheritance is - # a join. - return join_condition(mapper.mapped_table, table, - a_subset=mapper.local_table) - - try: - if self.secondary is not None: - if self.secondaryjoin is None: - self.secondaryjoin = _search_for_join(self.mapper, - self.secondary) - if self.primaryjoin is None: - self.primaryjoin = _search_for_join(self.parent, - self.secondary) - else: - if self.primaryjoin is None: - self.primaryjoin = _search_for_join(self.parent, - self.target) - except sa_exc.ArgumentError, e: - raise sa_exc.ArgumentError("Could not determine join " - "condition between parent/child tables on " - "relationship %s. Specify a 'primaryjoin' " - "expression. If 'secondary' is present, " - "'secondaryjoin' is needed as well." - % self) - - def _columns_are_mapped(self, *cols): - """Return True if all columns in the given collection are - mapped by the tables referenced by this :class:`.Relationship`. - - """ - for c in cols: - if self.secondary is not None \ - and self.secondary.c.contains_column(c): - continue - if not self.parent.mapped_table.c.contains_column(c) and \ - not self.target.c.contains_column(c): - return False - return True - - def _sync_pairs_from_join(self, join_condition, primary): - """Determine a list of "source"/"destination" column pairs - based on the given join condition, as well as the - foreign keys argument. - - "source" would be a column referenced by a foreign key, - and "destination" would be the column who has a foreign key - reference to "source". - - """ - - fks = self._user_defined_foreign_keys - # locate pairs - eq_pairs = criterion_as_pairs(join_condition, - consider_as_foreign_keys=fks, - any_operator=self.viewonly) - - # couldn't find any fks, but we have - # "secondary" - assume the "secondary" columns - # are the fks - if not eq_pairs and \ - self.secondary is not None and \ - not fks: - fks = set(self.secondary.c) - eq_pairs = criterion_as_pairs(join_condition, - consider_as_foreign_keys=fks, - any_operator=self.viewonly) - - if eq_pairs: - util.warn("No ForeignKey objects were present " - "in secondary table '%s'. Assumed referenced " - "foreign key columns %s for join condition '%s' " - "on relationship %s" % ( - self.secondary.description, - ", ".join(sorted(["'%s'" % col for col in fks])), - join_condition, - self - )) - - # Filter out just to columns that are mapped. - # If viewonly, allow pairs where the FK col - # was part of "foreign keys" - the column it references - # may be in an un-mapped table - see - # test.orm.test_relationships.ViewOnlyComplexJoin.test_basic - # for an example of this. - eq_pairs = [(l, r) for (l, r) in eq_pairs - if self._columns_are_mapped(l, r) - or self.viewonly and - r in fks] - - if eq_pairs: - return eq_pairs - - # from here below is just determining the best error message - # to report. Check for a join condition using any operator - # (not just ==), perhaps they need to turn on "viewonly=True". - if not self.viewonly and criterion_as_pairs(join_condition, - consider_as_foreign_keys=self._user_defined_foreign_keys, - any_operator=True): - - err = "Could not locate any "\ - "foreign-key-equated, locally mapped column "\ - "pairs for %s "\ - "condition '%s' on relationship %s." % ( - primary and 'primaryjoin' or 'secondaryjoin', - join_condition, - self - ) - - if not self._user_defined_foreign_keys: - err += " Ensure that the "\ - "referencing Column objects have a "\ - "ForeignKey present, or are otherwise part "\ - "of a ForeignKeyConstraint on their parent "\ - "Table, or specify the foreign_keys parameter "\ - "to this relationship." - - err += " For more "\ - "relaxed rules on join conditions, the "\ - "relationship may be marked as viewonly=True." - - raise sa_exc.ArgumentError(err) - else: - if self._user_defined_foreign_keys: - raise sa_exc.ArgumentError("Could not determine " - "relationship direction for %s condition " - "'%s', on relationship %s, using manual " - "'foreign_keys' setting. Do the columns " - "in 'foreign_keys' represent all, and " - "only, the 'foreign' columns in this join " - "condition? Does the %s Table already " - "have adequate ForeignKey and/or " - "ForeignKeyConstraint objects established " - "(in which case 'foreign_keys' is usually " - "unnecessary)?" - % ( - primary and 'primaryjoin' or 'secondaryjoin', - join_condition, - self, - primary and 'mapped' or 'secondary' - )) - else: - raise sa_exc.ArgumentError("Could not determine " - "relationship direction for %s condition " - "'%s', on relationship %s. Ensure that the " - "referencing Column objects have a " - "ForeignKey present, or are otherwise part " - "of a ForeignKeyConstraint on their parent " - "Table, or specify the foreign_keys parameter " - "to this relationship." - % ( - primary and 'primaryjoin' or 'secondaryjoin', - join_condition, - self - )) - - def _determine_synchronize_pairs(self): - """Resolve 'primary'/foreign' column pairs from the primaryjoin - and secondaryjoin arguments. - - """ - if self.local_remote_pairs: - if not self._user_defined_foreign_keys: - raise sa_exc.ArgumentError( - "foreign_keys argument is " - "required with _local_remote_pairs argument") - self.synchronize_pairs = [] - for l, r in self.local_remote_pairs: - if r in self._user_defined_foreign_keys: - self.synchronize_pairs.append((l, r)) - elif l in self._user_defined_foreign_keys: - self.synchronize_pairs.append((r, l)) - else: - self.synchronize_pairs = self._sync_pairs_from_join( - self.primaryjoin, - True) - - self._calculated_foreign_keys = util.column_set( - r for (l, r) in - self.synchronize_pairs) - - if self.secondaryjoin is not None: - self.secondary_synchronize_pairs = self._sync_pairs_from_join( - self.secondaryjoin, - False) - self._calculated_foreign_keys.update( - r for (l, r) in - self.secondary_synchronize_pairs) - else: - self.secondary_synchronize_pairs = None - - def _determine_direction(self): - """Determine if this relationship is one to many, many to one, - many to many. - - This is derived from the primaryjoin, presence of "secondary", - and in the case of self-referential the "remote side". - - """ - if self.secondaryjoin is not None: - self.direction = MANYTOMANY - elif self._refers_to_parent_table(): - - # self referential defaults to ONETOMANY unless the "remote" - # side is present and does not reference any foreign key - # columns - - if self.local_remote_pairs: - remote = [r for (l, r) in self.local_remote_pairs] - elif self.remote_side: - remote = self.remote_side - else: - remote = None - if not remote or self._calculated_foreign_keys.difference(l for (l, - r) in self.synchronize_pairs).intersection(remote): - self.direction = ONETOMANY - else: - self.direction = MANYTOONE - else: - parentcols = util.column_set(self.parent.mapped_table.c) - targetcols = util.column_set(self.mapper.mapped_table.c) - - # fk collection which suggests ONETOMANY. - onetomany_fk = targetcols.intersection( - self._calculated_foreign_keys) - - # fk collection which suggests MANYTOONE. - - manytoone_fk = parentcols.intersection( - self._calculated_foreign_keys) - - if onetomany_fk and manytoone_fk: - # fks on both sides. do the same test only based on the - # local side. - referents = [c for (c, f) in self.synchronize_pairs] - onetomany_local = parentcols.intersection(referents) - manytoone_local = targetcols.intersection(referents) - - if onetomany_local and not manytoone_local: - self.direction = ONETOMANY - elif manytoone_local and not onetomany_local: - self.direction = MANYTOONE - else: - raise sa_exc.ArgumentError( - "Can't determine relationship" - " direction for relationship '%s' - foreign " - "key columns are present in both the parent " - "and the child's mapped tables. Specify " - "'foreign_keys' argument." % self) - elif onetomany_fk: - self.direction = ONETOMANY - elif manytoone_fk: - self.direction = MANYTOONE - else: - raise sa_exc.ArgumentError("Can't determine relationship " - "direction for relationship '%s' - foreign " - "key columns are present in neither the parent " - "nor the child's mapped tables" % self) - + def _extra_determine_direction(self): if self.cascade.delete_orphan and not self.single_parent \ and (self.direction is MANYTOMANY or self.direction is MANYTOONE): @@ -1329,110 +1050,20 @@ class RelationshipProperty(StrategizedProperty): "relationships only." % self) - def _determine_local_remote_pairs(self): - """Determine pairs of columns representing "local" to - "remote", where "local" columns are on the parent mapper, - "remote" are on the target mapper. - - These pairs are used on the load side only to generate - lazy loading clauses. + def _columns_are_mapped(self, *cols): + """Return True if all columns in the given collection are + mapped by the tables referenced by this :class:`.Relationship`. """ - if not self.local_remote_pairs and not self.remote_side: - # the most common, trivial case. Derive - # local/remote pairs from the synchronize pairs. - eq_pairs = util.unique_list( - self.synchronize_pairs + - (self.secondary_synchronize_pairs or [])) - if self.direction is MANYTOONE: - self.local_remote_pairs = [(r, l) for l, r in eq_pairs] - else: - self.local_remote_pairs = eq_pairs - - # "remote_side" specified, derive from the primaryjoin - # plus remote_side, similarly to how synchronize_pairs - # were determined. - elif self.remote_side: - if self.local_remote_pairs: - raise sa_exc.ArgumentError('remote_side argument is ' - 'redundant against more detailed ' - '_local_remote_side argument.') - if self.direction is MANYTOONE: - self.local_remote_pairs = [(r, l) for (l, r) in - criterion_as_pairs(self.primaryjoin, - consider_as_referenced_keys=self.remote_side, - any_operator=True)] - - else: - self.local_remote_pairs = \ - criterion_as_pairs(self.primaryjoin, - consider_as_foreign_keys=self.remote_side, - any_operator=True) - if not self.local_remote_pairs: - raise sa_exc.ArgumentError('Relationship %s could ' - 'not determine any local/remote column ' - 'pairs from remote side argument %r' - % (self, self.remote_side)) - # else local_remote_pairs were sent explcitly via - # ._local_remote_pairs. - - # create local_side/remote_side accessors - self.local_side = util.ordered_column_set( - l for l, r in self.local_remote_pairs) - self.remote_side = util.ordered_column_set( - r for l, r in self.local_remote_pairs) - - # check that the non-foreign key column in the local/remote - # collection is mapped. The foreign key - # which the individual mapped column references directly may - # itself be in a non-mapped table; see - # test.orm.test_relationships.ViewOnlyComplexJoin.test_basic - # for an example of this. - if self.direction is ONETOMANY: - for col in self.local_side: - if not self._columns_are_mapped(col): - raise sa_exc.ArgumentError( - "Local column '%s' is not " - "part of mapping %s. Specify remote_side " - "argument to indicate which column lazy join " - "condition should compare against." % (col, - self.parent)) - elif self.direction is MANYTOONE: - for col in self.remote_side: - if not self._columns_are_mapped(col): - raise sa_exc.ArgumentError( - "Remote column '%s' is not " - "part of mapping %s. Specify remote_side " - "argument to indicate which column lazy join " - "condition should bind." % (col, self.mapper)) - - count = [0] - def clone(elem): - if set(['local', 'remote']).intersection(elem._annotations): - return None - elif elem in self.local_side and elem in self.remote_side: - # TODO: OK this still sucks. this is basically, - # refuse, refuse, refuse the temptation to guess! - # but crap we really have to guess don't we. we - # might want to traverse here with cloned_traverse - # so we can see the binary exprs and do it at that - # level.... - if count[0] % 2 == 0: - elem = elem._annotate({'local':True}) - else: - elem = elem._annotate({'remote':True}) - count[0] += 1 - elif elem in self.local_side: - elem = elem._annotate({'local':True}) - elif elem in self.remote_side: - elem = elem._annotate({'remote':True}) - else: - elem = None - return elem + for c in cols: + if self.secondary is not None \ + and self.secondary.c.contains_column(c): + continue + if not self.parent.mapped_table.c.contains_column(c) and \ + not self.target.c.contains_column(c): + return False + return True - self.primaryjoin = visitors.replacement_traverse( - self.primaryjoin, {}, clone - ) def _generate_backref(self): if not self.is_primary(): diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index cb07f234a..723d45295 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -241,36 +241,69 @@ class JoinCondition(object): return result[0] def _annotate_remote(self): + parentcols = util.column_set(self.parent_selectable.c) + for col in visitors.iterate(self.primaryjoin, {}): if "remote" in col._annotations: - return - - if self._local_remote_pairs: - raise NotImplementedError() - elif self._remote_side: - def repl(element): - if element in self._remote_side: - return element._annotate({"remote":True}) - elif self.secondary is not None: - def repl(element): - if self.secondary.c.contains_column(element): - return element._annotate({"remote":True}) - elif self._refers_to_parent_table(): - def repl(element): - # assume one to many - FKs are "remote" - if "foreign" in element._annotations: - return element._annotate({"remote":True}) + has_remote_annotations = True + break else: - def repl(element): - if self.child_selectable.c.contains_column(element): - return element._annotate({"remote":True}) + has_remote_annotations = False + + def _annotate_selfref(fn): + def visit_binary(binary): + equated = binary.left is binary.right + if isinstance(binary.left, sql.ColumnElement) and \ + isinstance(binary.right, sql.ColumnElement): + # assume one to many - FKs are "remote" + if fn(binary.left): + binary.left = binary.left._annotate({"remote":True}) + if fn(binary.right) and \ + not equated: + binary.right = binary.right._annotate( + {"remote":True}) + + self.primaryjoin = visitors.cloned_traverse( + self.primaryjoin, {}, + {"binary":visit_binary}) + + if not has_remote_annotations: + if self._local_remote_pairs: + raise NotImplementedError() + elif self._remote_side: + if self._refers_to_parent_table(): + _annotate_selfref(lambda col:col in self._remote_side) + else: + def repl(element): + if element in self._remote_side: + return element._annotate({"remote":True}) + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, {}, repl) + elif self.secondary is not None: + def repl(element): + if self.secondary.c.contains_column(element): + return element._annotate({"remote":True}) + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, {}, repl) + self.secondaryjoin = visitors.replacement_traverse( + self.secondaryjoin, {}, repl) + elif self._refers_to_parent_table(): + _annotate_selfref(lambda col:"foreign" in col._annotations) + else: + def repl(element): + if self.child_selectable.c.contains_column(element): + return element._annotate({"remote":True}) - self.primaryjoin = visitors.replacement_traverse( - self.primaryjoin, {}, repl) - if self.secondaryjoin is not None: - self.secondaryjoin = visitors.replacement_traverse( - self.secondaryjoin, {}, repl) + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, {}, repl) + def locals_(elem): + if "remote" not in elem._annotations and \ + elem in parentcols: + return elem._annotate({"local":True}) + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, {}, locals_ + ) def _check_foreign_cols(self, join_condition, primary): """Check the foreign key columns collected and emit error messages.""" @@ -375,30 +408,49 @@ class JoinCondition(object): "nor the child's mapped tables" % self.prop) @util.memoized_property - def liberal_remote_columns(self): - # this is temporary until we figure out - # which version of "remote" to use + def remote_columns(self): return self._gather_join_annotations("remote") @util.memoized_property - def remote_columns(self): - return set([r for l, r in self.local_remote_pairs]) - #return self._gather_join_annotations("remote") + def local_columns(self): + return self._gather_join_annotations("local") - remote_side = remote_columns + @util.memoized_property + def synchronize_pairs(self): + parentcols = util.column_set(self.parent_selectable.c) + targetcols = util.column_set(self.child_selectable.c) + result = [] + for l, r in self.local_remote_pairs: + if self.secondary is not None: + if "foreign" in r._annotations and \ + l in parentcols: + result.append((l, r)) + elif "foreign" in r._annotations and \ + "can_be_synced" in r._annotations: + result.append((l, r)) + elif "foreign" in l._annotations and \ + "can_be_synced" in l._annotations: + result.append((r, l)) + return result @util.memoized_property - def local_columns(self): - return set([l for l, r in self.local_remote_pairs]) + def secondary_synchronize_pairs(self): + parentcols = util.column_set(self.parent_selectable.c) + targetcols = util.column_set(self.child_selectable.c) + result = [] + if self.secondary is None: + return result + + for l, r in self.local_remote_pairs: + if "foreign" in l._annotations and \ + r in targetcols: + result.append((l, r)) + return result @util.memoized_property def foreign_key_columns(self): return self._gather_join_annotations("foreign") - @util.memoized_property - def referent_columns(self): - return self._gather_join_annotations("referent") - def _gather_join_annotations(self, annotation): s = set( self._gather_columns_with_annotation(self.primaryjoin, -- cgit v1.2.1 From bc45fa350a02da5f24d866078abed471cd98f15b Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 9 Feb 2012 21:16:53 -0500 Subject: - got m2m, local_remote_pairs, etc. working - using new traversal that returns the product of both sides of a binary, starting to work with (a+b) == (c+d) types of joins. primaryjoins on functions working - annotations working, including reversing local/remote when doing backref --- lib/sqlalchemy/orm/__init__.py | 8 + lib/sqlalchemy/orm/properties.py | 69 +----- lib/sqlalchemy/orm/relationships.py | 447 ++++++++++++++---------------------- lib/sqlalchemy/orm/strategies.py | 4 + lib/sqlalchemy/orm/util.py | 13 +- lib/sqlalchemy/sql/expression.py | 24 +- lib/sqlalchemy/sql/util.py | 76 +++++- 7 files changed, 294 insertions(+), 347 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py index 9fd969e3b..13bd18f08 100644 --- a/lib/sqlalchemy/orm/__init__.py +++ b/lib/sqlalchemy/orm/__init__.py @@ -44,6 +44,11 @@ from sqlalchemy.orm.properties import ( PropertyLoader, SynonymProperty, ) +from sqlalchemy.orm.relationships import ( + foreign, + remote, + remote_foreign +) from sqlalchemy.orm import mapper as mapperlib from sqlalchemy.orm.mapper import reconstructor, validates from sqlalchemy.orm import strategies @@ -81,6 +86,7 @@ __all__ = ( 'dynamic_loader', 'eagerload', 'eagerload_all', + 'foreign', 'immediateload', 'join', 'joinedload', @@ -96,6 +102,8 @@ __all__ = ( 'reconstructor', 'relationship', 'relation', + 'remote', + 'remote_foreign', 'scoped_session', 'sessionmaker', 'subqueryload', diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 77da33b5f..38237b2d4 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -16,7 +16,7 @@ from sqlalchemy.sql.util import ClauseAdapter, criterion_as_pairs, \ join_condition, _shallow_annotate from sqlalchemy.sql import operators, expression, visitors from sqlalchemy.orm import attributes, dependency, mapper, \ - object_mapper, strategies, configure_mappers + object_mapper, strategies, configure_mappers, relationships from sqlalchemy.orm.util import CascadeOptions, _class_to_mapper, \ _orm_annotate, _orm_deannotate @@ -915,13 +915,12 @@ class RelationshipProperty(StrategizedProperty): self._check_conflicts() self._process_dependent_arguments() self._setup_join_conditions() - self._extra_determine_direction() + self._check_cascade_settings() self._post_init() self._generate_backref() super(RelationshipProperty, self).do_init() def _setup_join_conditions(self): - import relationships self._join_condition = jc = relationships.JoinCondition( parent_selectable=self.parent.mapped_table, child_selectable=self.mapper.mapped_table, @@ -946,8 +945,8 @@ class RelationshipProperty(StrategizedProperty): self.local_remote_pairs = jc.local_remote_pairs self.remote_side = jc.remote_columns self.synchronize_pairs = jc.synchronize_pairs - self.secondary_synchronize_pairs = jc.secondary_synchronize_pairs self._calculated_foreign_keys = jc.foreign_key_columns + self.secondary_synchronize_pairs = jc.secondary_synchronize_pairs def _check_conflicts(self): """Test that this relationship is legal, warn about @@ -1035,7 +1034,7 @@ class RelationshipProperty(StrategizedProperty): (self.key, self.parent.class_) ) - def _extra_determine_direction(self): + def _check_cascade_settings(self): if self.cascade.delete_orphan and not self.single_parent \ and (self.direction is MANYTOMANY or self.direction is MANYTOONE): @@ -1064,7 +1063,6 @@ class RelationshipProperty(StrategizedProperty): return False return True - def _generate_backref(self): if not self.is_primary(): return @@ -1083,13 +1081,15 @@ class RelationshipProperty(StrategizedProperty): pj = kwargs.pop('primaryjoin', self.secondaryjoin) sj = kwargs.pop('secondaryjoin', self.primaryjoin) else: - pj = kwargs.pop('primaryjoin', self.primaryjoin) + pj = kwargs.pop('primaryjoin', + self._join_condition.primaryjoin_reverse_remote) sj = kwargs.pop('secondaryjoin', None) if sj: raise sa_exc.InvalidRequestError( "Can't assign 'secondaryjoin' on a backref against " "a non-secondary relationship." ) + foreign_keys = kwargs.pop('foreign_keys', self._user_defined_foreign_keys) parent = self.parent.primary_mapper() @@ -1112,21 +1112,6 @@ class RelationshipProperty(StrategizedProperty): self._add_reverse_property(self.back_populates) def _post_init(self): - self.logger.info('%s setup primary join %s', self, - self.primaryjoin) - self.logger.info('%s setup secondary join %s', self, - self.secondaryjoin) - self.logger.info('%s synchronize pairs [%s]', self, - ','.join('(%s => %s)' % (l, r) for (l, r) in - self.synchronize_pairs)) - self.logger.info('%s secondary synchronize pairs [%s]', self, - ','.join('(%s => %s)' % (l, r) for (l, r) in - self.secondary_synchronize_pairs or [])) - self.logger.info('%s local/remote pairs [%s]', self, - ','.join('(%s / %s)' % (l, r) for (l, r) in - self.local_remote_pairs)) - self.logger.info('%s relationship direction %s', self, - self.direction) if self.uselist is None: self.uselist = self.direction is not MANYTOONE if not self.viewonly: @@ -1141,46 +1126,6 @@ class RelationshipProperty(StrategizedProperty): strategy = self._get_strategy(strategies.LazyLoader) return strategy.use_get - def _refers_to_parent_table(self): - alt = self._alt_refers_to_parent_table() - pt = self.parent.mapped_table - mt = self.mapper.mapped_table - for c, f in self.synchronize_pairs: - if ( - pt.is_derived_from(c.table) and \ - pt.is_derived_from(f.table) and \ - mt.is_derived_from(c.table) and \ - mt.is_derived_from(f.table) - ): - assert alt - return True - else: - assert not alt - return False - - def _alt_refers_to_parent_table(self): - pt = self.parent.mapped_table - mt = self.mapper.mapped_table - result = [False] - def visit_binary(binary): - c, f = binary.left, binary.right - if ( - isinstance(c, expression.ColumnClause) and \ - isinstance(f, expression.ColumnClause) and \ - pt.is_derived_from(c.table) and \ - pt.is_derived_from(f.table) and \ - mt.is_derived_from(c.table) and \ - mt.is_derived_from(f.table) - ): - result[0] = True - - visitors.traverse( - self.primaryjoin, - {}, - {"binary":visit_binary} - ) - return result[0] - @util.memoized_property def _is_self_referential(self): return self.mapper.common_parent(self.parent) diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index 723d45295..d8c2659b6 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -15,7 +15,7 @@ and `secondaryjoin` aspects of :func:`.relationship`. from sqlalchemy import sql, util, log, exc as sa_exc, schema from sqlalchemy.sql.util import ClauseAdapter, criterion_as_pairs, \ - join_condition, _shallow_annotate + join_condition, _shallow_annotate, visit_binary_product from sqlalchemy.sql import operators, expression, visitors from sqlalchemy.orm.interfaces import MANYTOMANY, MANYTOONE, ONETOMANY @@ -78,7 +78,34 @@ class JoinCondition(object): self._determine_joins() self._annotate_fks() self._annotate_remote() + self._annotate_local() self._determine_direction() + self._setup_pairs() + self._check_foreign_cols(self.primaryjoin, True) + if self.secondaryjoin is not None: + self._check_foreign_cols(self.secondaryjoin, False) + self._check_remote_side() + self._log_joins() + + def _log_joins(self): + if self.prop is None: + return + log = self.prop.logger + log.info('%s setup primary join %s', self, + self.primaryjoin) + log.info('%s setup secondary join %s', self, + self.secondaryjoin) + log.info('%s synchronize pairs [%s]', self, + ','.join('(%s => %s)' % (l, r) for (l, r) in + self.synchronize_pairs)) + log.info('%s secondary synchronize pairs [%s]', self, + ','.join('(%s => %s)' % (l, r) for (l, r) in + self.secondary_synchronize_pairs or [])) + log.info('%s local/remote pairs [%s]', self, + ','.join('(%s / %s)' % (l, r) for (l, r) in + self.local_remote_pairs)) + log.info('%s relationship direction %s', self, + self.direction) def _determine_joins(self): """Determine the 'primaryjoin' and 'secondaryjoin' attributes, @@ -128,28 +155,60 @@ class JoinCondition(object): "'secondaryjoin' is needed as well." % self.prop) + @util.memoized_property + def primaryjoin_reverse_remote(self): + def replace(element): + if "remote" in element._annotations: + v = element._annotations.copy() + del v['remote'] + v['local'] = True + return element._with_annotations(v) + elif "local" in element._annotations: + v = element._annotations.copy() + del v['local'] + v['remote'] = True + return element._with_annotations(v) + return visitors.replacement_traverse(self.primaryjoin, {}, replace) + + def _has_annotation(self, clause, annotation): + for col in visitors.iterate(clause, {}): + if annotation in col._annotations: + return True + else: + return False + def _annotate_fks(self): + if self._has_annotation(self.primaryjoin, "foreign"): + return + + if self.consider_as_foreign_keys: + self._annotate_from_fk_list() + else: + self._annotate_present_fks() + + def _annotate_from_fk_list(self): + def check_fk(col): + if col in self.consider_as_foreign_keys: + return col._annotate({"foreign":True}) + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, + {}, + check_fk + ) + if self.secondaryjoin is not None: + self.secondaryjoin = visitors.replacement_traverse( + self.secondaryjoin, + {}, + check_fk + ) + + def _annotate_present_fks(self): if self.secondary is not None: secondarycols = util.column_set(self.secondary.c) else: secondarycols = set() - def col_is(a, b): - return a.compare(b) - def is_foreign(a, b): - if self.consider_as_foreign_keys: - if a in self.consider_as_foreign_keys and ( - col_is(a, b) or - b not in self.consider_as_foreign_keys - ): - return a - elif b in self.consider_as_foreign_keys and ( - col_is(a, b) or - a not in self.consider_as_foreign_keys - ): - return b - if isinstance(a, schema.Column) and \ isinstance(b, schema.Column): if a.references(b): @@ -163,19 +222,6 @@ class JoinCondition(object): elif b in secondarycols and a not in secondarycols: return b - def _annotate_fk(binary, left, right): - can_be_synced = self.can_be_synced_fn(left) - left = left._annotate({ - #"equated":binary.operator is operators.eq, - "can_be_synced":can_be_synced and \ - binary.operator is operators.eq - }) - right = right._annotate({ - #"equated":binary.operator is operators.eq, - "referent":True - }) - return left, right - def visit_binary(binary): if not isinstance(binary.left, sql.ColumnElement) or \ not isinstance(binary.right, sql.ColumnElement): @@ -185,20 +231,12 @@ class JoinCondition(object): "foreign" not in binary.right._annotations: col = is_foreign(binary.left, binary.right) if col is not None: - if col is binary.left: + if col.compare(binary.left): binary.left = binary.left._annotate( {"foreign":True}) - elif col is binary.right: + elif col.compare(binary.right): binary.right = binary.right._annotate( {"foreign":True}) - # TODO: when the two cols are the same. - - if "foreign" in binary.left._annotations: - binary.left, binary.right = _annotate_fk( - binary, binary.left, binary.right) - if "foreign" in binary.right._annotations: - binary.right, binary.left = _annotate_fk( - binary, binary.right, binary.left) self.primaryjoin = visitors.cloned_traverse( self.primaryjoin, @@ -211,11 +249,6 @@ class JoinCondition(object): {}, {"binary":visit_binary} ) - self._check_foreign_cols( - self.primaryjoin, True) - if self.secondaryjoin is not None: - self._check_foreign_cols( - self.secondaryjoin, False) def _refers_to_parent_table(self): pt = self.parent_selectable @@ -241,18 +274,14 @@ class JoinCondition(object): return result[0] def _annotate_remote(self): - parentcols = util.column_set(self.parent_selectable.c) + if self._has_annotation(self.primaryjoin, "remote"): + return - for col in visitors.iterate(self.primaryjoin, {}): - if "remote" in col._annotations: - has_remote_annotations = True - break - else: - has_remote_annotations = False + parentcols = util.column_set(self.parent_selectable.c) def _annotate_selfref(fn): def visit_binary(binary): - equated = binary.left is binary.right + equated = binary.left.compare(binary.right) if isinstance(binary.left, sql.ColumnElement) and \ isinstance(binary.right, sql.ColumnElement): # assume one to many - FKs are "remote" @@ -267,44 +296,72 @@ class JoinCondition(object): self.primaryjoin, {}, {"binary":visit_binary}) - if not has_remote_annotations: + if self.secondary is not None: + def repl(element): + if self.secondary.c.contains_column(element): + return element._annotate({"remote":True}) + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, {}, repl) + self.secondaryjoin = visitors.replacement_traverse( + self.secondaryjoin, {}, repl) + elif self._local_remote_pairs or self._remote_side: + if self._local_remote_pairs: - raise NotImplementedError() - elif self._remote_side: - if self._refers_to_parent_table(): - _annotate_selfref(lambda col:col in self._remote_side) - else: - def repl(element): - if element in self._remote_side: - return element._annotate({"remote":True}) - self.primaryjoin = visitors.replacement_traverse( - self.primaryjoin, {}, repl) - elif self.secondary is not None: - def repl(element): - if self.secondary.c.contains_column(element): - return element._annotate({"remote":True}) - self.primaryjoin = visitors.replacement_traverse( - self.primaryjoin, {}, repl) - self.secondaryjoin = visitors.replacement_traverse( - self.secondaryjoin, {}, repl) - elif self._refers_to_parent_table(): - _annotate_selfref(lambda col:"foreign" in col._annotations) + if self._remote_side: + raise sa_exc.ArgumentError( + "remote_side argument is redundant " + "against more detailed _local_remote_side " + "argument.") + + remote_side = [r for (l, r) in self._local_remote_pairs] + else: + remote_side = self._remote_side + + if self._refers_to_parent_table(): + _annotate_selfref(lambda col:col in remote_side) else: def repl(element): - if self.child_selectable.c.contains_column(element): + if element in remote_side: return element._annotate({"remote":True}) - self.primaryjoin = visitors.replacement_traverse( self.primaryjoin, {}, repl) + elif self._refers_to_parent_table(): + _annotate_selfref(lambda col:"foreign" in col._annotations) + else: + def repl(element): + if self.child_selectable.c.contains_column(element): + return element._annotate({"remote":True}) + + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, {}, repl) + + def _annotate_local(self): + if self._has_annotation(self.primaryjoin, "local"): + return + + parentcols = util.column_set(self.parent_selectable.c) + + if self._local_remote_pairs: + local_side = util.column_set([l for (l, r) + in self._local_remote_pairs]) + else: + local_side = util.column_set(self.parent_selectable.c) def locals_(elem): if "remote" not in elem._annotations and \ - elem in parentcols: + elem in local_side: return elem._annotate({"local":True}) self.primaryjoin = visitors.replacement_traverse( self.primaryjoin, {}, locals_ ) + def _check_remote_side(self): + if not self.local_remote_pairs: + raise sa_exc.ArgumentError('Relationship %s could ' + 'not determine any local/remote column ' + 'pairs from remote side argument %r' + % (self.prop, self._remote_side)) + def _check_foreign_cols(self, join_condition, primary): """Check the foreign key columns collected and emit error messages.""" @@ -315,11 +372,10 @@ class JoinCondition(object): has_foreign = bool(foreign_cols) - if self.support_sync: - for col in foreign_cols: - if col._annotations.get("can_be_synced"): - can_sync = True - break + if primary: + can_sync = bool(self.synchronize_pairs) + else: + can_sync = bool(self.secondary_synchronize_pairs) if self.support_sync and can_sync or \ (not self.support_sync and has_foreign): @@ -407,6 +463,44 @@ class JoinCondition(object): "key columns are present in neither the parent " "nor the child's mapped tables" % self.prop) + def _setup_pairs(self): + sync_pairs = [] + lrp = util.OrderedSet([]) + secondary_sync_pairs = [] + + def go(joincond, collection): + def visit_binary(binary, left, right): + if "remote" in right._annotations and \ + "remote" not in left._annotations and \ + self.can_be_synced_fn(left): + lrp.add((left, right)) + elif "remote" in left._annotations and \ + "remote" not in right._annotations and \ + self.can_be_synced_fn(right): + lrp.add((right, left)) + if binary.operator is operators.eq: + # and \ + #binary.left.compare(left) and \ + #binary.right.compare(right): + if "foreign" in right._annotations: + collection.append((left, right)) + elif "foreign" in left._annotations: + collection.append((right, left)) + visit_binary_product(visit_binary, joincond) + + for joincond, collection in [ + (self.primaryjoin, sync_pairs), + (self.secondaryjoin, secondary_sync_pairs) + ]: + if joincond is None: + continue + go(joincond, collection) + + self.local_remote_pairs = list(lrp) + self.synchronize_pairs = sync_pairs + self.secondary_synchronize_pairs = secondary_sync_pairs + + @util.memoized_property def remote_columns(self): return self._gather_join_annotations("remote") @@ -415,38 +509,6 @@ class JoinCondition(object): def local_columns(self): return self._gather_join_annotations("local") - @util.memoized_property - def synchronize_pairs(self): - parentcols = util.column_set(self.parent_selectable.c) - targetcols = util.column_set(self.child_selectable.c) - result = [] - for l, r in self.local_remote_pairs: - if self.secondary is not None: - if "foreign" in r._annotations and \ - l in parentcols: - result.append((l, r)) - elif "foreign" in r._annotations and \ - "can_be_synced" in r._annotations: - result.append((l, r)) - elif "foreign" in l._annotations and \ - "can_be_synced" in l._annotations: - result.append((r, l)) - return result - - @util.memoized_property - def secondary_synchronize_pairs(self): - parentcols = util.column_set(self.parent_selectable.c) - targetcols = util.column_set(self.child_selectable.c) - result = [] - if self.secondary is None: - return result - - for l, r in self.local_remote_pairs: - if "foreign" in l._annotations and \ - r in targetcols: - result.append((l, r)) - return result - @util.memoized_property def foreign_key_columns(self): return self._gather_join_annotations("foreign") @@ -470,24 +532,6 @@ class JoinCondition(object): if annotation.issubset(col._annotations) ]) - @util.memoized_property - def local_remote_pairs(self): - lrp = util.OrderedSet() - def visit_binary(binary): - if "remote" in binary.right._annotations and \ - "remote" not in binary.left._annotations and \ - isinstance(binary.left, expression.ColumnClause) and \ - self.can_be_synced_fn(binary.left): - lrp.add((binary.left, binary.right)) - elif "remote" in binary.left._annotations and \ - "remote" not in binary.right._annotations and \ - isinstance(binary.right, expression.ColumnClause) and \ - self.can_be_synced_fn(binary.right): - lrp.add((binary.right, binary.left)) - visitors.traverse(self.primaryjoin, {}, {"binary":visit_binary}) - if self.secondaryjoin is not None: - visitors.traverse(self.secondaryjoin, {}, {"binary":visit_binary}) - return list(lrp) def join_targets(self, source_selectable, dest_selectable, @@ -604,147 +648,6 @@ def _create_lazy_clause(cls, prop, reverse_direction=False): return lazywhere, bind_to_col, equated_columns -def _determine_synchronize_pairs(self): - """Resolve 'primary'/foreign' column pairs from the primaryjoin - and secondaryjoin arguments. - - """ - if self.local_remote_pairs: - if not self._user_defined_foreign_keys: - raise sa_exc.ArgumentError( - "foreign_keys argument is " - "required with _local_remote_pairs argument") - self.synchronize_pairs = [] - for l, r in self.local_remote_pairs: - if r in self._user_defined_foreign_keys: - self.synchronize_pairs.append((l, r)) - elif l in self._user_defined_foreign_keys: - self.synchronize_pairs.append((r, l)) - else: - self.synchronize_pairs = self._sync_pairs_from_join( - self.primaryjoin, - True) - - self._calculated_foreign_keys = util.column_set( - r for (l, r) in - self.synchronize_pairs) - - if self.secondaryjoin is not None: - self.secondary_synchronize_pairs = self._sync_pairs_from_join( - self.secondaryjoin, - False) - self._calculated_foreign_keys.update( - r for (l, r) in - self.secondary_synchronize_pairs) - else: - self.secondary_synchronize_pairs = None - - -def _determine_local_remote_pairs(self): - """Determine pairs of columns representing "local" to - "remote", where "local" columns are on the parent mapper, - "remote" are on the target mapper. - - These pairs are used on the load side only to generate - lazy loading clauses. - - """ - if not self.local_remote_pairs and not self.remote_side: - # the most common, trivial case. Derive - # local/remote pairs from the synchronize pairs. - eq_pairs = util.unique_list( - self.synchronize_pairs + - (self.secondary_synchronize_pairs or [])) - if self.direction is MANYTOONE: - self.local_remote_pairs = [(r, l) for l, r in eq_pairs] - else: - self.local_remote_pairs = eq_pairs - - # "remote_side" specified, derive from the primaryjoin - # plus remote_side, similarly to how synchronize_pairs - # were determined. - elif self.remote_side: - if self.local_remote_pairs: - raise sa_exc.ArgumentError('remote_side argument is ' - 'redundant against more detailed ' - '_local_remote_side argument.') - if self.direction is MANYTOONE: - self.local_remote_pairs = [(r, l) for (l, r) in - criterion_as_pairs(self.primaryjoin, - consider_as_referenced_keys=self.remote_side, - any_operator=True)] - - else: - self.local_remote_pairs = \ - criterion_as_pairs(self.primaryjoin, - consider_as_foreign_keys=self.remote_side, - any_operator=True) - if not self.local_remote_pairs: - raise sa_exc.ArgumentError('Relationship %s could ' - 'not determine any local/remote column ' - 'pairs from remote side argument %r' - % (self, self.remote_side)) - # else local_remote_pairs were sent explcitly via - # ._local_remote_pairs. - - # create local_side/remote_side accessors - self.local_side = util.ordered_column_set( - l for l, r in self.local_remote_pairs) - self.remote_side = util.ordered_column_set( - r for l, r in self.local_remote_pairs) - - # check that the non-foreign key column in the local/remote - # collection is mapped. The foreign key - # which the individual mapped column references directly may - # itself be in a non-mapped table; see - # test.orm.test_relationships.ViewOnlyComplexJoin.test_basic - # for an example of this. - if self.direction is ONETOMANY: - for col in self.local_side: - if not self._columns_are_mapped(col): - raise sa_exc.ArgumentError( - "Local column '%s' is not " - "part of mapping %s. Specify remote_side " - "argument to indicate which column lazy join " - "condition should compare against." % (col, - self.parent)) - elif self.direction is MANYTOONE: - for col in self.remote_side: - if not self._columns_are_mapped(col): - raise sa_exc.ArgumentError( - "Remote column '%s' is not " - "part of mapping %s. Specify remote_side " - "argument to indicate which column lazy join " - "condition should bind." % (col, self.mapper)) - - count = [0] - def clone(elem): - if set(['local', 'remote']).intersection(elem._annotations): - return None - elif elem in self.local_side and elem in self.remote_side: - # TODO: OK this still sucks. this is basically, - # refuse, refuse, refuse the temptation to guess! - # but crap we really have to guess don't we. we - # might want to traverse here with cloned_traverse - # so we can see the binary exprs and do it at that - # level.... - if count[0] % 2 == 0: - elem = elem._annotate({'local':True}) - else: - elem = elem._annotate({'remote':True}) - count[0] += 1 - elif elem in self.local_side: - elem = elem._annotate({'local':True}) - elif elem in self.remote_side: - elem = elem._annotate({'remote':True}) - else: - elem = None - return elem - - self.primaryjoin = visitors.replacement_traverse( - self.primaryjoin, {}, clone - ) - def _criterion_exists(self, criterion=None, **kwargs): if getattr(self, '_of_type', None): diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 5f4b182d0..320234281 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -785,6 +785,7 @@ class SubqueryLoader(AbstractRelationshipLoader): leftmost_mapper, leftmost_prop = \ subq_mapper, \ subq_mapper._props[subq_path[1]] + # TODO: local cols might not be unique here leftmost_cols, remote_cols = self._local_remote_columns(leftmost_prop) leftmost_attr = [ @@ -846,6 +847,7 @@ class SubqueryLoader(AbstractRelationshipLoader): # self.parent is more specific than subq_path[-2] parent_alias = mapperutil.AliasedClass(self.parent) + # TODO: local cols might not be unique here local_cols, remote_cols = \ self._local_remote_columns(self.parent_property) @@ -885,6 +887,7 @@ class SubqueryLoader(AbstractRelationshipLoader): if prop.secondary is None: return zip(*prop.local_remote_pairs) else: + # TODO: this isn't going to work for readonly.... return \ [p[0] for p in prop.synchronize_pairs],\ [ @@ -930,6 +933,7 @@ class SubqueryLoader(AbstractRelationshipLoader): if ('subquery', reduced_path) not in context.attributes: return None, None, None + # TODO: local_cols might not be unique here local_cols, remote_cols = self._local_remote_columns(self.parent_property) q = context.attributes[('subquery', reduced_path)] diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index 0cd5b0594..f17f675f4 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -366,7 +366,18 @@ def _orm_annotate(element, exclude=None): """ return sql_util._deep_annotate(element, {'_orm_adapt':True}, exclude) -_orm_deannotate = sql_util._deep_deannotate +def _orm_deannotate(element): + """Remove annotations that link a column to a particular mapping. + + Note this doesn't affect "remote" and "foreign" annotations + passed by the :func:`.orm.foreign` and :func:`.orm.remote` + annotators. + + """ + + return sql_util._deep_deannotate(element, + values=("_orm_adapt", "parententity") + ) class _ORMJoin(expression.Join): """Extend Join to support ORM constructs as input.""" diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py index 72099a5f5..ebf4de9a2 100644 --- a/lib/sqlalchemy/sql/expression.py +++ b/lib/sqlalchemy/sql/expression.py @@ -1576,18 +1576,30 @@ class ClauseElement(Visitable): return id(self) def _annotate(self, values): - """return a copy of this ClauseElement with the given annotations - dictionary. + """return a copy of this ClauseElement with annotations + updated by the given dictionary. """ return sqlutil.Annotated(self, values) - def _deannotate(self): - """return a copy of this ClauseElement with an empty annotations - dictionary. + def _with_annotations(self, values): + """return a copy of this ClauseElement with annotations + replaced by the given dictionary. """ - return self._clone() + return sqlutil.Annotated(self, values) + + def _deannotate(self, values=None): + """return a copy of this :class:`.ClauseElement` with annotations + removed. + + :param values: optional tuple of individual values + to remove. + + """ + # since we have no annotations we return + # self + return self def unique_params(self, *optionaldict, **kwargs): """Return a copy with :func:`bindparam()` elments replaced. diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py index f0509c16f..9a45a5777 100644 --- a/lib/sqlalchemy/sql/util.py +++ b/lib/sqlalchemy/sql/util.py @@ -62,6 +62,61 @@ def find_join_source(clauses, join_to): else: return None, None + +def visit_binary_product(fn, expr): + """Produce a traversal of the given expression, delivering + column comparisons to the given function. + + The function is of the form:: + + def my_fn(binary, left, right) + + For each binary expression located which has a + comparison operator, the product of "left" and + "right" will be delivered to that function, + in terms of that binary. + + Hence an expression like:: + + and_( + (a + b) == q + func.sum(e + f), + j == r + ) + + would have the traversal:: + + a q + a e + a f + b q + b e + b f + j r + + That is, every combination of "left" and + "right" that doesn't further contain + a binary comparison is passed as pairs. + + """ + stack = [] + def visit(element): + if element.__visit_name__ == 'binary' and \ + operators.is_comparison(element.operator): + stack.insert(0, element) + for l in visit(element.left): + for r in visit(element.right): + fn(stack[0], l, r) + stack.pop(0) + for elem in element.get_children(): + visit(elem) + else: + if isinstance(element, expression.ColumnClause): + yield element + for elem in element.get_children(): + for e in visit(elem): + yield e + list(visit(expr)) + def find_tables(clause, check_columns=False, include_aliases=False, include_joins=False, include_selects=False, include_crud=False): @@ -357,13 +412,22 @@ class Annotated(object): def _annotate(self, values): _values = self._annotations.copy() _values.update(values) + return self._with_annotations(_values) + + def _with_annotations(self, values): clone = self.__class__.__new__(self.__class__) clone.__dict__ = self.__dict__.copy() - clone._annotations = _values + clone._annotations = values return clone - def _deannotate(self): - return self.__element + def _deannotate(self, values=None): + if values is None: + return self.__element + else: + _values = self._annotations.copy() + for v in values: + _values.pop(v, None) + return self._with_annotations(_values) def _compiler_dispatch(self, visitor, **kw): return self.__element.__class__._compiler_dispatch(self, visitor, **kw) @@ -426,11 +490,11 @@ def _deep_annotate(element, annotations, exclude=None): element = clone(element) return element -def _deep_deannotate(element): - """Deep copy the given element, removing all annotations.""" +def _deep_deannotate(element, values=None): + """Deep copy the given element, removing annotations.""" def clone(elem): - elem = elem._deannotate() + elem = elem._deannotate(values=values) elem._copy_internals(clone=clone) return elem -- cgit v1.2.1 From 9c5ab04c574ac39d51d5995bb32f1fa2fd11b803 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 10 Feb 2012 11:54:18 -0500 Subject: fix up this test --- lib/sqlalchemy/orm/relationships.py | 1 - 1 file changed, 1 deletion(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index d8c2659b6..515ca773b 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -207,7 +207,6 @@ class JoinCondition(object): secondarycols = util.column_set(self.secondary.c) else: secondarycols = set() - def is_foreign(a, b): if isinstance(a, schema.Column) and \ isinstance(b, schema.Column): -- cgit v1.2.1 From 089e9ce5978d081e9cb579b0298042a0efb57edd Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 10 Feb 2012 17:50:15 -0500 Subject: - move properties to use the new create_joins - fix up subquery eager loading --- lib/sqlalchemy/orm/properties.py | 197 +++++++++++++----------------------- lib/sqlalchemy/orm/relationships.py | 3 +- lib/sqlalchemy/orm/strategies.py | 23 +---- 3 files changed, 74 insertions(+), 149 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 38237b2d4..f7a979d0e 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -920,6 +920,61 @@ class RelationshipProperty(StrategizedProperty): self._generate_backref() super(RelationshipProperty, self).do_init() + def _process_dependent_arguments(self): + """Convert incoming configuration arguments to their + proper form. + + Callables are resolved, ORM annotations removed. + + """ + # accept callables for other attributes which may require + # deferred initialization. This technique is used + # by declarative "string configs" and some recipes. + for attr in ( + 'order_by', 'primaryjoin', 'secondaryjoin', + 'secondary', '_user_defined_foreign_keys', 'remote_side', + ): + attr_value = getattr(self, attr) + if util.callable(attr_value): + setattr(self, attr, attr_value()) + + # remove "annotations" which are present if mapped class + # descriptors are used to create the join expression. + for attr in 'primaryjoin', 'secondaryjoin': + val = getattr(self, attr) + if val is not None: + setattr(self, attr, _orm_deannotate( + expression._only_column_elements(val, attr)) + ) + + # ensure expressions in self.order_by, foreign_keys, + # remote_side are all columns, not strings. + if self.order_by is not False and self.order_by is not None: + self.order_by = [ + expression._only_column_elements(x, "order_by") + for x in + util.to_list(self.order_by)] + + self._user_defined_foreign_keys = \ + util.column_set( + expression._only_column_elements(x, "foreign_keys") + for x in util.to_column_set( + self._user_defined_foreign_keys + )) + + self.remote_side = \ + util.column_set( + expression._only_column_elements(x, "remote_side") + for x in + util.to_column_set(self.remote_side)) + + self.target = self.mapper.mapped_table + + if self.cascade.delete_orphan: + self.mapper.primary_mapper().delete_orphans.append( + (self.key, self.parent.class_) + ) + def _setup_join_conditions(self): self._join_condition = jc = relationships.JoinCondition( parent_selectable=self.parent.mapped_table, @@ -944,6 +999,7 @@ class RelationshipProperty(StrategizedProperty): self.direction = jc.direction self.local_remote_pairs = jc.local_remote_pairs self.remote_side = jc.remote_columns + self.local_columns = jc.local_columns self.synchronize_pairs = jc.synchronize_pairs self._calculated_foreign_keys = jc.foreign_key_columns self.secondary_synchronize_pairs = jc.secondary_synchronize_pairs @@ -975,64 +1031,6 @@ class RelationshipProperty(StrategizedProperty): "cause dependency issues during flush" % (self.key, self.parent, inheriting)) - def _process_dependent_arguments(self): - """Convert incoming configuration arguments to their - proper form. - - Callables are resolved, ORM annotations removed. - - """ - # accept callables for other attributes which may require - # deferred initialization. This technique is used - # by declarative "string configs" and some recipes. - for attr in ( - 'order_by', - 'primaryjoin', - 'secondaryjoin', - 'secondary', - '_user_defined_foreign_keys', - 'remote_side', - ): - attr_value = getattr(self, attr) - if util.callable(attr_value): - setattr(self, attr, attr_value()) - - # remove "annotations" which are present if mapped class - # descriptors are used to create the join expression. - for attr in 'primaryjoin', 'secondaryjoin': - val = getattr(self, attr) - if val is not None: - setattr(self, attr, _orm_deannotate( - expression._only_column_elements(val, attr)) - ) - - # ensure expressions in self.order_by, foreign_keys, - # remote_side are all columns, not strings. - if self.order_by is not False and self.order_by is not None: - self.order_by = [ - expression._only_column_elements(x, "order_by") - for x in - util.to_list(self.order_by)] - - self._user_defined_foreign_keys = \ - util.column_set( - expression._only_column_elements(x, "foreign_keys") - for x in util.to_column_set( - self._user_defined_foreign_keys - )) - - self.remote_side = \ - util.column_set( - expression._only_column_elements(x, "remote_side") - for x in - util.to_column_set(self.remote_side)) - - self.target = self.mapper.mapped_table - - if self.cascade.delete_orphan: - self.mapper.primary_mapper().delete_orphans.append( - (self.key, self.parent.class_) - ) def _check_cascade_settings(self): if self.cascade.delete_orphan and not self.single_parent \ @@ -1098,14 +1096,11 @@ class RelationshipProperty(StrategizedProperty): kwargs.setdefault('passive_updates', self.passive_updates) self.back_populates = backref_key relationship = RelationshipProperty( - parent, - self.secondary, - pj, - sj, + parent, self.secondary, + pj, sj, foreign_keys=foreign_keys, back_populates=self.key, - **kwargs - ) + **kwargs) mapper._configure_property(backref_key, relationship) if self.back_populates: @@ -1155,78 +1150,22 @@ class RelationshipProperty(StrategizedProperty): else: aliased = True - # place a barrier on the destination such that - # replacement traversals won't ever dig into it. - # its internal structure remains fixed - # regardless of context. - dest_selectable = _shallow_annotate( - dest_selectable, - {'no_replacement_traverse':True}) - - aliased = aliased or (source_selectable is not None) - - primaryjoin, secondaryjoin, secondary = self.primaryjoin, \ - self.secondaryjoin, self.secondary - - # adjust the join condition for single table inheritance, - # in the case that the join is to a subclass - # this is analogous to the "_adjust_for_single_table_inheritance()" - # method in Query. - dest_mapper = of_type or self.mapper single_crit = dest_mapper._single_table_criterion - if single_crit is not None: - if secondaryjoin is not None: - secondaryjoin = secondaryjoin & single_crit - else: - primaryjoin = primaryjoin & single_crit - - if aliased: - if secondary is not None: - secondary = secondary.alias() - primary_aliasizer = ClauseAdapter(secondary) - secondary_aliasizer = \ - ClauseAdapter(dest_selectable, - equivalents=self.mapper._equivalent_columns).\ - chain(primary_aliasizer) - if source_selectable is not None: - primary_aliasizer = \ - ClauseAdapter(secondary).\ - chain(ClauseAdapter(source_selectable, - equivalents=self.parent._equivalent_columns)) - secondaryjoin = \ - secondary_aliasizer.traverse(secondaryjoin) - else: - primary_aliasizer = ClauseAdapter(dest_selectable, - #exclude=self.local_side, - exclude_fn=lambda c: "local" in c._annotations, - equivalents=self.mapper._equivalent_columns) - if source_selectable is not None: - primary_aliasizer.chain( - ClauseAdapter(source_selectable, - #exclude=self.remote_side, - exclude_fn=lambda c: "remote" in c._annotations, - equivalents=self.parent._equivalent_columns)) - secondary_aliasizer = None - - primaryjoin = primary_aliasizer.traverse(primaryjoin) - target_adapter = secondary_aliasizer or primary_aliasizer - target_adapter.include = target_adapter.exclude = target_adapter.exclude_fn = None - else: - target_adapter = None + aliased = aliased or (source_selectable is not None) + + primaryjoin, secondaryjoin, secondary, target_adapter, dest_selectable = \ + self._join_condition.join_targets( + source_selectable, dest_selectable, aliased, single_crit + ) if source_selectable is None: source_selectable = self.parent.local_table if dest_selectable is None: dest_selectable = self.mapper.local_table - return ( - primaryjoin, - secondaryjoin, - source_selectable, - dest_selectable, - secondary, - target_adapter, - ) + return (primaryjoin, secondaryjoin, source_selectable, + dest_selectable, secondary, target_adapter) + PropertyLoader = RelationProperty = RelationshipProperty log.class_logger(RelationshipProperty) diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index 515ca773b..413397fda 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -599,7 +599,8 @@ class JoinCondition(object): target_adapter.exclude_fn = None else: target_adapter = None - return primaryjoin, secondaryjoin, secondary, target_adapter + return primaryjoin, secondaryjoin, secondary, target_adapter, dest_selectable + ################# everything below is TODO ################################ diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 320234281..00739ea03 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -785,8 +785,8 @@ class SubqueryLoader(AbstractRelationshipLoader): leftmost_mapper, leftmost_prop = \ subq_mapper, \ subq_mapper._props[subq_path[1]] - # TODO: local cols might not be unique here - leftmost_cols, remote_cols = self._local_remote_columns(leftmost_prop) + + leftmost_cols = leftmost_prop.local_columns leftmost_attr = [ leftmost_mapper._columntoproperty[c].class_attribute @@ -847,9 +847,7 @@ class SubqueryLoader(AbstractRelationshipLoader): # self.parent is more specific than subq_path[-2] parent_alias = mapperutil.AliasedClass(self.parent) - # TODO: local cols might not be unique here - local_cols, remote_cols = \ - self._local_remote_columns(self.parent_property) + local_cols = self.parent_property.local_columns local_attr = [ getattr(parent_alias, self.parent._columntoproperty[c].key) @@ -883,18 +881,6 @@ class SubqueryLoader(AbstractRelationshipLoader): q = q.join(attr, aliased=middle, from_joinpoint=True) return q - def _local_remote_columns(self, prop): - if prop.secondary is None: - return zip(*prop.local_remote_pairs) - else: - # TODO: this isn't going to work for readonly.... - return \ - [p[0] for p in prop.synchronize_pairs],\ - [ - p[0] for p in prop. - secondary_synchronize_pairs - ] - def _setup_options(self, q, subq_path, orig_query): # propagate loader options etc. to the new query. # these will fire relative to subq_path. @@ -933,8 +919,7 @@ class SubqueryLoader(AbstractRelationshipLoader): if ('subquery', reduced_path) not in context.attributes: return None, None, None - # TODO: local_cols might not be unique here - local_cols, remote_cols = self._local_remote_columns(self.parent_property) + local_cols = self.parent_property.local_columns q = context.attributes[('subquery', reduced_path)] -- cgit v1.2.1 From 7d693180be8c7f9db79831351751a15d786b86a7 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 10 Feb 2012 17:59:06 -0500 Subject: tweak for correlated subqueries here, seems to work for test_eager_relations:CorrelatedSubqueryTest but need some more testing here --- lib/sqlalchemy/sql/util.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py index 9a45a5777..511a5b0c2 100644 --- a/lib/sqlalchemy/sql/util.py +++ b/lib/sqlalchemy/sql/util.py @@ -100,7 +100,12 @@ def visit_binary_product(fn, expr): """ stack = [] def visit(element): - if element.__visit_name__ == 'binary' and \ + if isinstance(element, (expression.FromClause, + expression._ScalarSelect)): + # we dont want to dig into correlated subqueries, + # those are just column elements by themselves + yield element + elif element.__visit_name__ == 'binary' and \ operators.is_comparison(element.operator): stack.insert(0, element) for l in visit(element.left): -- cgit v1.2.1 From 0634ea79b1a23a8b88c886a8a3f434ed300691e2 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 11 Feb 2012 15:43:05 -0500 Subject: many fixes but still can't get heuristics to work as well as what's existing, tests still failing --- lib/sqlalchemy/orm/properties.py | 4 ++-- lib/sqlalchemy/orm/relationships.py | 6 +++++- lib/sqlalchemy/orm/util.py | 3 +++ lib/sqlalchemy/sql/util.py | 3 +-- 4 files changed, 11 insertions(+), 5 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index f7a979d0e..17b12e50f 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -18,7 +18,7 @@ from sqlalchemy.sql import operators, expression, visitors from sqlalchemy.orm import attributes, dependency, mapper, \ object_mapper, strategies, configure_mappers, relationships from sqlalchemy.orm.util import CascadeOptions, _class_to_mapper, \ - _orm_annotate, _orm_deannotate + _orm_annotate, _orm_deannotate, _orm_full_deannotate from sqlalchemy.orm.interfaces import MANYTOMANY, MANYTOONE, \ MapperProperty, ONETOMANY, PropComparator, StrategizedProperty @@ -62,7 +62,7 @@ class ColumnProperty(StrategizedProperty): """ self._orig_columns = [expression._labeled(c) for c in columns] - self.columns = [expression._labeled(_orm_deannotate(c)) + self.columns = [expression._labeled(_orm_full_deannotate(c)) for c in columns] self.group = kwargs.pop('group', None) self.deferred = kwargs.pop('deferred', False) diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index 413397fda..adba2d542 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -328,7 +328,11 @@ class JoinCondition(object): _annotate_selfref(lambda col:"foreign" in col._annotations) else: def repl(element): - if self.child_selectable.c.contains_column(element): + if self.child_selectable.c.contains_column(element) and \ + ( + not self.parent_local_selectable.c.contains_column(element) + or self.child_local_selectable.c.contains_column(element) + ): return element._annotate({"remote":True}) self.primaryjoin = visitors.replacement_traverse( diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index f17f675f4..aaff6ce4a 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -379,6 +379,9 @@ def _orm_deannotate(element): values=("_orm_adapt", "parententity") ) +def _orm_full_deannotate(element): + return sql_util._deep_deannotate(element) + class _ORMJoin(expression.Join): """Extend Join to support ORM constructs as input.""" diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py index 511a5b0c2..e4e2c00e1 100644 --- a/lib/sqlalchemy/sql/util.py +++ b/lib/sqlalchemy/sql/util.py @@ -100,8 +100,7 @@ def visit_binary_product(fn, expr): """ stack = [] def visit(element): - if isinstance(element, (expression.FromClause, - expression._ScalarSelect)): + if isinstance(element, (expression._ScalarSelect)): # we dont want to dig into correlated subqueries, # those are just column elements by themselves yield element -- cgit v1.2.1 From d934ea23e24880a5c784c9e5edf9ead5bc965a83 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 11 Feb 2012 20:33:56 -0500 Subject: - figured out again why deannotate must clone() - got everything working. just need to update error strings --- lib/sqlalchemy/orm/properties.py | 16 +- lib/sqlalchemy/orm/relationships.py | 357 ++++++++++++++++++++---------------- lib/sqlalchemy/sql/expression.py | 13 +- lib/sqlalchemy/sql/util.py | 4 +- lib/sqlalchemy/sql/visitors.py | 6 +- 5 files changed, 221 insertions(+), 175 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 17b12e50f..527a3def0 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -1062,6 +1062,9 @@ class RelationshipProperty(StrategizedProperty): return True def _generate_backref(self): + """Interpret the 'backref' instruction to create a + :func:`.relationship` complementary to this one.""" + if not self.is_primary(): return if self.backref is not None and not self.back_populates: @@ -1075,7 +1078,14 @@ class RelationshipProperty(StrategizedProperty): "'%s' on relationship '%s': property of that " "name exists on mapper '%s'" % (backref_key, self, mapper)) + + # determine primaryjoin/secondaryjoin for the + # backref. Use the one we had, so that + # a custom join doesn't have to be specified in + # both directions. if self.secondary is not None: + # for many to many, just switch primaryjoin/ + # secondaryjoin. pj = kwargs.pop('primaryjoin', self.secondaryjoin) sj = kwargs.pop('secondaryjoin', self.primaryjoin) else: @@ -1084,9 +1094,9 @@ class RelationshipProperty(StrategizedProperty): sj = kwargs.pop('secondaryjoin', None) if sj: raise sa_exc.InvalidRequestError( - "Can't assign 'secondaryjoin' on a backref against " - "a non-secondary relationship." - ) + "Can't assign 'secondaryjoin' on a backref " + "against a non-secondary relationship." + ) foreign_keys = kwargs.pop('foreign_keys', self._user_defined_foreign_keys) diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index adba2d542..edb7498e0 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -15,7 +15,8 @@ and `secondaryjoin` aspects of :func:`.relationship`. from sqlalchemy import sql, util, log, exc as sa_exc, schema from sqlalchemy.sql.util import ClauseAdapter, criterion_as_pairs, \ - join_condition, _shallow_annotate, visit_binary_product + join_condition, _shallow_annotate, visit_binary_product,\ + _deep_deannotate from sqlalchemy.sql import operators, expression, visitors from sqlalchemy.orm.interfaces import MANYTOMANY, MANYTOONE, ONETOMANY @@ -157,18 +158,36 @@ class JoinCondition(object): @util.memoized_property def primaryjoin_reverse_remote(self): - def replace(element): - if "remote" in element._annotations: - v = element._annotations.copy() - del v['remote'] - v['local'] = True - return element._with_annotations(v) - elif "local" in element._annotations: - v = element._annotations.copy() - del v['local'] - v['remote'] = True - return element._with_annotations(v) - return visitors.replacement_traverse(self.primaryjoin, {}, replace) + """Return the primaryjoin condition suitable for the + "reverse" direction. + + If the primaryjoin was delivered here with pre-existing + "remote" annotations, the local/remote annotations + are reversed. Otherwise, the local/remote annotations + are removed. + + """ + if self._has_remote_annotations: + def replace(element): + if "remote" in element._annotations: + v = element._annotations.copy() + del v['remote'] + v['local'] = True + return element._with_annotations(v) + elif "local" in element._annotations: + v = element._annotations.copy() + del v['local'] + v['remote'] = True + return element._with_annotations(v) + return visitors.replacement_traverse( + self.primaryjoin, {}, replace) + else: + if self._has_foreign_annotations: + # TODO: coverage + return _deep_deannotate(self.primaryjoin, + values=("local", "remote")) + else: + return _deep_deannotate(self.primaryjoin) def _has_annotation(self, clause, annotation): for col in visitors.iterate(clause, {}): @@ -177,8 +196,21 @@ class JoinCondition(object): else: return False + @util.memoized_property + def _has_foreign_annotations(self): + return self._has_annotation(self.primaryjoin, "foreign") + + @util.memoized_property + def _has_remote_annotations(self): + return self._has_annotation(self.primaryjoin, "remote") + def _annotate_fks(self): - if self._has_annotation(self.primaryjoin, "foreign"): + """Annotate the primaryjoin and secondaryjoin + structures with 'foreign' annotations marking columns + considered as foreign. + + """ + if self._has_foreign_annotations: return if self.consider_as_foreign_keys: @@ -250,6 +282,10 @@ class JoinCondition(object): ) def _refers_to_parent_table(self): + """Return True if the join condition contains column + comparisons where both columns are in both tables. + + """ pt = self.parent_selectable mt = self.child_selectable result = [False] @@ -264,7 +300,6 @@ class JoinCondition(object): mt.is_derived_from(f.table) ): result[0] = True - visitors.traverse( self.primaryjoin, {}, @@ -272,73 +307,162 @@ class JoinCondition(object): ) return result[0] + def _tables_overlap(self): + """Return True if parent/child tables have some overlap.""" + + return self.parent_selectable.is_derived_from( + self.child_local_selectable) or \ + self.child_selectable.is_derived_from( + self.parent_local_selectable) + def _annotate_remote(self): - if self._has_annotation(self.primaryjoin, "remote"): + """Annotate the primaryjoin and secondaryjoin + structures with 'remote' annotations marking columns + considered as part of the 'remote' side. + + """ + if self._has_remote_annotations: return parentcols = util.column_set(self.parent_selectable.c) - def _annotate_selfref(fn): - def visit_binary(binary): - equated = binary.left.compare(binary.right) - if isinstance(binary.left, sql.ColumnElement) and \ - isinstance(binary.right, sql.ColumnElement): - # assume one to many - FKs are "remote" - if fn(binary.left): - binary.left = binary.left._annotate({"remote":True}) - if fn(binary.right) and \ - not equated: - binary.right = binary.right._annotate( - {"remote":True}) - - self.primaryjoin = visitors.cloned_traverse( - self.primaryjoin, {}, - {"binary":visit_binary}) - if self.secondary is not None: - def repl(element): - if self.secondary.c.contains_column(element): - return element._annotate({"remote":True}) - self.primaryjoin = visitors.replacement_traverse( - self.primaryjoin, {}, repl) - self.secondaryjoin = visitors.replacement_traverse( - self.secondaryjoin, {}, repl) + self._annotate_remote_secondary() elif self._local_remote_pairs or self._remote_side: + self._annotate_remote_from_args() + elif self._refers_to_parent_table(): + self._annotate_selfref(lambda col:"foreign" in col._annotations) + elif self._tables_overlap(): + self._annotate_remote_with_overlap() + else: + self._annotate_remote_distinct_selectables() - if self._local_remote_pairs: - if self._remote_side: - raise sa_exc.ArgumentError( - "remote_side argument is redundant " - "against more detailed _local_remote_side " - "argument.") - - remote_side = [r for (l, r) in self._local_remote_pairs] + def _annotate_remote_secondary(self): + """annotate 'remote' in primaryjoin, secondaryjoin + when 'secondary' is present. + + """ + def repl(element): + if self.secondary.c.contains_column(element): + return element._annotate({"remote":True}) + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, {}, repl) + self.secondaryjoin = visitors.replacement_traverse( + self.secondaryjoin, {}, repl) + + def _annotate_selfref(self, fn): + """annotate 'remote' in primaryjoin, secondaryjoin + when the relationship is detected as self-referential. + + """ + def visit_binary(binary): + equated = binary.left.compare(binary.right) + if isinstance(binary.left, expression.ColumnClause) and \ + isinstance(binary.right, expression.ColumnClause): + # assume one to many - FKs are "remote" + if fn(binary.left): + binary.left = binary.left._annotate({"remote":True}) + if fn(binary.right) and \ + not equated: + binary.right = binary.right._annotate( + {"remote":True}) else: - remote_side = self._remote_side + self._warn_non_column_elements() - if self._refers_to_parent_table(): - _annotate_selfref(lambda col:col in remote_side) - else: - def repl(element): - if element in remote_side: - return element._annotate({"remote":True}) - self.primaryjoin = visitors.replacement_traverse( - self.primaryjoin, {}, repl) - elif self._refers_to_parent_table(): - _annotate_selfref(lambda col:"foreign" in col._annotations) + self.primaryjoin = visitors.cloned_traverse( + self.primaryjoin, {}, + {"binary":visit_binary}) + + def _annotate_remote_from_args(self): + """annotate 'remote' in primaryjoin, secondaryjoin + when the 'remote_side' or '_local_remote_pairs' + arguments are used. + + """ + if self._local_remote_pairs: + if self._remote_side: + raise sa_exc.ArgumentError( + "remote_side argument is redundant " + "against more detailed _local_remote_side " + "argument.") + + remote_side = [r for (l, r) in self._local_remote_pairs] + else: + remote_side = self._remote_side + + if self._refers_to_parent_table(): + self._annotate_selfref(lambda col:col in remote_side) else: def repl(element): - if self.child_selectable.c.contains_column(element) and \ - ( - not self.parent_local_selectable.c.contains_column(element) - or self.child_local_selectable.c.contains_column(element) - ): + if element in remote_side: return element._annotate({"remote":True}) - self.primaryjoin = visitors.replacement_traverse( self.primaryjoin, {}, repl) + def _annotate_remote_with_overlap(self): + """annotate 'remote' in primaryjoin, secondaryjoin + when the parent/child tables have some set of + tables in common, though is not a fully self-referential + relationship. + + """ + def visit_binary(binary): + binary.left, binary.right = proc_left_right(binary.left, + binary.right) + binary.right, binary.left = proc_left_right(binary.right, + binary.left) + def proc_left_right(left, right): + if isinstance(left, expression.ColumnClause) and \ + isinstance(right, expression.ColumnClause): + if self.child_selectable.c.contains_column(right) and \ + self.parent_selectable.c.contains_column(left): + right = right._annotate({"remote":True}) + else: + self._warn_non_column_elements() + + return left, right + + self.primaryjoin = visitors.cloned_traverse( + self.primaryjoin, {}, + {"binary":visit_binary}) + + def _annotate_remote_distinct_selectables(self): + """annotate 'remote' in primaryjoin, secondaryjoin + when the parent/child tables are entirely + separate. + + """ + def repl(element): + if self.child_selectable.c.contains_column(element) and \ + ( + not self.parent_local_selectable.c.\ + contains_column(element) + or self.child_local_selectable.c.\ + contains_column(element) + ): + return element._annotate({"remote":True}) + self.primaryjoin = visitors.replacement_traverse( + self.primaryjoin, {}, repl) + + def _warn_non_column_elements(self): + util.warn( + "Non-simple column elements in primary " + "join condition for property %s - consider using " + "remote() annotations to mark the remote side." + % self.prop + ) + def _annotate_local(self): + """Annotate the primaryjoin and secondaryjoin + structures with 'local' annotations. + + This annotates all column elements found + simultaneously in the parent table + and the join condition that don't have a + 'remote' annotation set up from + _annotate_remote() or user-defined. + + """ if self._has_annotation(self.primaryjoin, "local"): return @@ -362,8 +486,8 @@ class JoinCondition(object): if not self.local_remote_pairs: raise sa_exc.ArgumentError('Relationship %s could ' 'not determine any local/remote column ' - 'pairs from remote side argument %r' - % (self.prop, self._remote_side)) + 'pairs.' + % (self.prop, )) def _check_foreign_cols(self, join_condition, primary): """Check the foreign key columns collected and emit error messages.""" @@ -412,6 +536,7 @@ class JoinCondition(object): err += "Ensure that referencing columns are associated with a "\ "a ForeignKey or ForeignKeyConstraint, or are annotated "\ "in the join condition with the foreign() annotation." + raise sa_exc.ArgumentError(err) def _determine_direction(self): """Determine if this relationship is one to many, many to one, @@ -482,9 +607,6 @@ class JoinCondition(object): self.can_be_synced_fn(right): lrp.add((right, left)) if binary.operator is operators.eq: - # and \ - #binary.left.compare(left) and \ - #binary.right.compare(right): if "foreign" in right._annotations: collection.append((left, right)) elif "foreign" in left._annotations: @@ -503,7 +625,6 @@ class JoinCondition(object): self.synchronize_pairs = sync_pairs self.secondary_synchronize_pairs = secondary_sync_pairs - @util.memoized_property def remote_columns(self): return self._gather_join_annotations("remote") @@ -603,7 +724,8 @@ class JoinCondition(object): target_adapter.exclude_fn = None else: target_adapter = None - return primaryjoin, secondaryjoin, secondary, target_adapter, dest_selectable + return primaryjoin, secondaryjoin, secondary, \ + target_adapter, dest_selectable ################# everything below is TODO ################################ @@ -653,94 +775,3 @@ def _create_lazy_clause(cls, prop, reverse_direction=False): -def _criterion_exists(self, criterion=None, **kwargs): - if getattr(self, '_of_type', None): - target_mapper = self._of_type - to_selectable = target_mapper._with_polymorphic_selectable - if self.property._is_self_referential: - to_selectable = to_selectable.alias() - - single_crit = target_mapper._single_table_criterion - if single_crit is not None: - if criterion is not None: - criterion = single_crit & criterion - else: - criterion = single_crit - else: - to_selectable = None - - if self.adapter: - source_selectable = self.__clause_element__() - else: - source_selectable = None - - pj, sj, source, dest, secondary, target_adapter = \ - self.property._create_joins(dest_polymorphic=True, - dest_selectable=to_selectable, - source_selectable=source_selectable) - - for k in kwargs: - crit = getattr(self.property.mapper.class_, k) == kwargs[k] - if criterion is None: - criterion = crit - else: - criterion = criterion & crit - - # annotate the *local* side of the join condition, in the case - # of pj + sj this is the full primaryjoin, in the case of just - # pj its the local side of the primaryjoin. - if sj is not None: - j = _orm_annotate(pj) & sj - else: - j = _orm_annotate(pj, exclude=self.property.remote_side) - - # MARKMARK - if criterion is not None and target_adapter: - # limit this adapter to annotated only? - criterion = target_adapter.traverse(criterion) - - # only have the "joined left side" of what we - # return be subject to Query adaption. The right - # side of it is used for an exists() subquery and - # should not correlate or otherwise reach out - # to anything in the enclosing query. - if criterion is not None: - criterion = criterion._annotate({'no_replacement_traverse': True}) - - crit = j & criterion - - return sql.exists([1], crit, from_obj=dest).\ - correlate(source._annotate({'_orm_adapt':True})) - - -def __negated_contains_or_equals(self, other): - if self.property.direction == MANYTOONE: - state = attributes.instance_state(other) - - def state_bindparam(x, state, col): - o = state.obj() # strong ref - return sql.bindparam(x, unique=True, callable_=lambda : \ - self.property.mapper._get_committed_attr_by_column(o, - col)) - - def adapt(col): - if self.adapter: - return self.adapter(col) - else: - return col - - if self.property._use_get: - return sql.and_(*[ - sql.or_( - adapt(x) != state_bindparam(adapt(x), state, y), - adapt(x) == None) - for (x, y) in self.property.local_remote_pairs]) - - criterion = sql.and_(*[x==y for (x, y) in - zip( - self.property.mapper.primary_key, - self.property.\ - mapper.\ - primary_key_from_instance(other)) - ]) - return ~self._criterion_exists(criterion) diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py index ebf4de9a2..573ace47f 100644 --- a/lib/sqlalchemy/sql/expression.py +++ b/lib/sqlalchemy/sql/expression.py @@ -1589,7 +1589,7 @@ class ClauseElement(Visitable): """ return sqlutil.Annotated(self, values) - def _deannotate(self, values=None): + def _deannotate(self, values=None, clone=False): """return a copy of this :class:`.ClauseElement` with annotations removed. @@ -1597,9 +1597,14 @@ class ClauseElement(Visitable): to remove. """ - # since we have no annotations we return - # self - return self + if clone: + # clone is used when we are also copying + # the expression for a deep deannotation + return self._clone() + else: + # if no clone, since we have no annotations we return + # self + return self def unique_params(self, *optionaldict, **kwargs): """Return a copy with :func:`bindparam()` elments replaced. diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py index e4e2c00e1..2862e9af9 100644 --- a/lib/sqlalchemy/sql/util.py +++ b/lib/sqlalchemy/sql/util.py @@ -424,7 +424,7 @@ class Annotated(object): clone._annotations = values return clone - def _deannotate(self, values=None): + def _deannotate(self, values=None, clone=True): if values is None: return self.__element else: @@ -498,7 +498,7 @@ def _deep_deannotate(element, values=None): """Deep copy the given element, removing annotations.""" def clone(elem): - elem = elem._deannotate(values=values) + elem = elem._deannotate(values=values, clone=True) elem._copy_internals(clone=clone) return elem diff --git a/lib/sqlalchemy/sql/visitors.py b/lib/sqlalchemy/sql/visitors.py index 75e099f0d..cd178b716 100644 --- a/lib/sqlalchemy/sql/visitors.py +++ b/lib/sqlalchemy/sql/visitors.py @@ -222,13 +222,13 @@ def cloned_traverse(obj, opts, visitors): if elem in stop_on: return elem else: - if elem not in cloned: - cloned[elem] = newelem = elem._clone() + if id(elem) not in cloned: + cloned[id(elem)] = newelem = elem._clone() newelem._copy_internals(clone=clone) meth = visitors.get(newelem.__visit_name__, None) if meth: meth(newelem) - return cloned[elem] + return cloned[id(elem)] if obj is not None: obj = clone(obj) -- cgit v1.2.1 From d37320306560c3d758ed65563d53aa9500095a49 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 25 Feb 2012 17:10:06 -0500 Subject: start to work on error messages, allow foreign_keys as only argument if otherwise can't determine join condition due to no fks --- lib/sqlalchemy/exc.py | 7 ++++ lib/sqlalchemy/orm/relationships.py | 71 +++++++++++++++++++++++++++++-------- lib/sqlalchemy/sql/util.py | 14 ++++++-- 3 files changed, 74 insertions(+), 18 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/exc.py b/lib/sqlalchemy/exc.py index 91ffc2811..f28bd8a07 100644 --- a/lib/sqlalchemy/exc.py +++ b/lib/sqlalchemy/exc.py @@ -25,6 +25,13 @@ class ArgumentError(SQLAlchemyError): """ +class NoForeignKeysError(ArgumentError): + """Raised when no foreign keys can be located between two selectables + during a join.""" + +class AmbiguousForeignKeysError(ArgumentError): + """Raised when more than one foreign key matching can be located + between two selectables during a join.""" class CircularDependencyError(SQLAlchemyError): """Raised by topological sorts when a circular dependency is detected. diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index edb7498e0..76e219efe 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -80,11 +80,11 @@ class JoinCondition(object): self._annotate_fks() self._annotate_remote() self._annotate_local() - self._determine_direction() self._setup_pairs() self._check_foreign_cols(self.primaryjoin, True) if self.secondaryjoin is not None: self._check_foreign_cols(self.secondaryjoin, False) + self._determine_direction() self._check_remote_side() self._log_joins() @@ -134,27 +134,68 @@ class JoinCondition(object): join_condition( self.child_selectable, self.secondary, - a_subset=self.child_local_selectable) + a_subset=self.child_local_selectable, + consider_as_foreign_keys=\ + self.consider_as_foreign_keys or None + ) if self.primaryjoin is None: self.primaryjoin = \ join_condition( self.parent_selectable, self.secondary, - a_subset=self.parent_local_selectable) + a_subset=self.parent_local_selectable, + consider_as_foreign_keys=\ + self.consider_as_foreign_keys or None + ) else: if self.primaryjoin is None: self.primaryjoin = \ join_condition( self.parent_selectable, self.child_selectable, - a_subset=self.parent_local_selectable) - except sa_exc.ArgumentError, e: - raise sa_exc.ArgumentError("Could not determine join " - "condition between parent/child tables on " - "relationship %s. Specify a 'primaryjoin' " - "expression. If 'secondary' is present, " - "'secondaryjoin' is needed as well." - % self.prop) + a_subset=self.parent_local_selectable, + consider_as_foreign_keys=\ + self.consider_as_foreign_keys or None + ) + except sa_exc.NoForeignKeysError, nfke: + if self.secondary is not None: + raise sa_exc.NoForeignKeysError("Could not determine join " + "condition between parent/child tables on " + "relationship %s - there are no foreign keys " + "linking these tables via secondary table '%s'. " + "Ensure that referencing columns are associated with a "\ + "ForeignKey or ForeignKeyConstraint, or specify 'primaryjoin' "\ + "and 'secondaryjoin' expressions." + % (self.prop, self.secondary)) + else: + raise sa_exc.NoForeignKeysError("Could not determine join " + "condition between parent/child tables on " + "relationship %s - there are no foreign keys " + "linking these tables. " + "Ensure that referencing columns are associated with a " + "ForeignKey or ForeignKeyConstraint, or specify a 'primaryjoin' " + "expression." + % self.prop) + except sa_exc.AmbiguousForeignKeysError, afke: + if self.secondary is not None: + raise sa_exc.AmbiguousForeignKeysError("Could not determine join " + "condition between parent/child tables on " + "relationship %s - there are multiple foreign key " + "paths linking the tables via secondary table '%s'. " + "Specify the 'foreign_keys' " + "argument, providing a list of those columns which " + "should be counted as containing a foreign key reference " + "from the secondary table to each of the parent and child tables." + % (self.prop, self.secondary)) + else: + raise sa_exc.AmbiguousForeignKeysError("Could not determine join " + "condition between parent/child tables on " + "relationship %s - there are multiple foreign key " + "paths linking the tables. Specify the 'foreign_keys' " + "argument, providing a list of those columns which " + "should be counted as containing a foreign key reference " + "to the parent table." + % self.prop) @util.memoized_property def primaryjoin_reverse_remote(self): @@ -515,7 +556,7 @@ class JoinCondition(object): err = "Could not locate any simple equality expressions "\ "involving foreign key columns for %s join condition "\ "'%s' on relationship %s." % ( - primary and 'primaryjoin' or 'secondaryjoin', + primary and 'primary' or 'secondary', join_condition, self.prop ) @@ -529,12 +570,12 @@ class JoinCondition(object): else: err = "Could not locate any relevant foreign key columns "\ "for %s join condition '%s' on relationship %s." % ( - primary and 'primaryjoin' or 'secondaryjoin', + primary and 'primary' or 'secondary', join_condition, self.prop ) - err += "Ensure that referencing columns are associated with a "\ - "a ForeignKey or ForeignKeyConstraint, or are annotated "\ + err += " Ensure that referencing columns are associated with a "\ + "ForeignKey or ForeignKeyConstraint, or are annotated "\ "in the join condition with the foreign() annotation." raise sa_exc.ArgumentError(err) diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py index 2862e9af9..38d95dde5 100644 --- a/lib/sqlalchemy/sql/util.py +++ b/lib/sqlalchemy/sql/util.py @@ -284,8 +284,10 @@ def adapt_criterion_to_null(crit, nulls): return visitors.cloned_traverse(crit, {}, {'binary':visit_binary}) + def join_condition(a, b, ignore_nonexistent_tables=False, - a_subset=None): + a_subset=None, + consider_as_foreign_keys=None): """create a join condition between two tables or selectables. e.g.:: @@ -321,6 +323,9 @@ def join_condition(a, b, ignore_nonexistent_tables=False, for fk in sorted( b.foreign_keys, key=lambda fk:fk.parent._creation_order): + if consider_as_foreign_keys is not None and \ + fk.parent not in consider_as_foreign_keys: + continue try: col = fk.get_referent(left) except exc.NoReferenceError, nrte: @@ -336,6 +341,9 @@ def join_condition(a, b, ignore_nonexistent_tables=False, for fk in sorted( left.foreign_keys, key=lambda fk:fk.parent._creation_order): + if consider_as_foreign_keys is not None and \ + fk.parent not in consider_as_foreign_keys: + continue try: col = fk.get_referent(b) except exc.NoReferenceError, nrte: @@ -358,11 +366,11 @@ def join_condition(a, b, ignore_nonexistent_tables=False, "subquery using alias()?" else: hint = "" - raise exc.ArgumentError( + raise exc.NoForeignKeysError( "Can't find any foreign key relationships " "between '%s' and '%s'.%s" % (a.description, b.description, hint)) elif len(constraints) > 1: - raise exc.ArgumentError( + raise exc.AmbiguousForeignKeysError( "Can't determine join between '%s' and '%s'; " "tables have more than one foreign key " "constraint relationship between them. " -- cgit v1.2.1 From acdeaf43f79e7f881e6a105bc3c5149266a3c580 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 25 Feb 2012 18:04:40 -0500 Subject: almost through all the fine tuning --- lib/sqlalchemy/orm/relationships.py | 44 +++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 17 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index 76e219efe..adf3b0cde 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -58,7 +58,7 @@ class JoinCondition(object): self_referential=False, prop=None, support_sync=True, - can_be_synced_fn=lambda c: True + can_be_synced_fn=lambda *c: True ): self.parent_selectable = parent_selectable self.parent_local_selectable = parent_local_selectable @@ -531,7 +531,8 @@ class JoinCondition(object): % (self.prop, )) def _check_foreign_cols(self, join_condition, primary): - """Check the foreign key columns collected and emit error messages.""" + """Check the foreign key columns collected and emit error + messages.""" can_sync = False @@ -554,17 +555,19 @@ class JoinCondition(object): # (not just ==), perhaps they need to turn on "viewonly=True". if self.support_sync and has_foreign and not can_sync: err = "Could not locate any simple equality expressions "\ - "involving foreign key columns for %s join condition "\ + "involving locally mapped foreign key columns for "\ + "%s join condition "\ "'%s' on relationship %s." % ( primary and 'primary' or 'secondary', join_condition, self.prop ) - err += " Ensure that referencing columns are associated with a "\ - "ForeignKey or ForeignKeyConstraint, or are annotated "\ - "in the join condition with the foreign() annotation. "\ - "To allow comparison operators other than '==', "\ - "the relationship can be marked as viewonly=True." + err += \ + " Ensure that referencing columns are associated "\ + "with a ForeignKey or ForeignKeyConstraint, or are "\ + "annotated in the join condition with the foreign() "\ + "annotation. To allow comparison operators other than "\ + "'==', the relationship can be marked as viewonly=True." raise sa_exc.ArgumentError(err) else: @@ -574,9 +577,11 @@ class JoinCondition(object): join_condition, self.prop ) - err += " Ensure that referencing columns are associated with a "\ - "ForeignKey or ForeignKeyConstraint, or are annotated "\ - "in the join condition with the foreign() annotation." + err += \ + ' Ensure that referencing columns are associated '\ + 'with a ForeignKey or ForeignKeyConstraint, or are '\ + 'annotated in the join condition with the foreign() '\ + 'annotation.' raise sa_exc.ArgumentError(err) def _determine_direction(self): @@ -617,11 +622,15 @@ class JoinCondition(object): self.direction = MANYTOONE else: raise sa_exc.ArgumentError( - "Can't determine relationship" - " direction for relationship '%s' - foreign " - "key columns are present in both the parent " - "and the child's mapped tables. Specify " - "'foreign_keys' argument." % self.prop) + "Can't determine relationship" + " direction for relationship '%s' - foreign " + "key columns within the join condition are present " + "in both the parent and the child's mapped tables. " + "Ensure that only those columns referring " + "to a parent column are marked as foreign, " + "either via the foreign() annotation or " + "via the foreign_keys argument." + % self.prop) elif onetomany_fk: self.direction = ONETOMANY elif manytoone_fk: @@ -647,7 +656,8 @@ class JoinCondition(object): "remote" not in right._annotations and \ self.can_be_synced_fn(right): lrp.add((right, left)) - if binary.operator is operators.eq: + if binary.operator is operators.eq and \ + self.can_be_synced_fn(left, right): if "foreign" in right._annotations: collection.append((left, right)) elif "foreign" in left._annotations: -- cgit v1.2.1 From 4ffcc52c465c9e3b95f2b802099fb8a98fe98cb3 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sun, 1 Apr 2012 18:18:12 -0400 Subject: - move create_lazy_clause() to relationships - add foreign, remote annotations to declarative --- lib/sqlalchemy/ext/declarative.py | 6 +- lib/sqlalchemy/orm/relationships.py | 130 ++++++++++++++++++------------------ lib/sqlalchemy/orm/strategies.py | 49 +------------- 3 files changed, 74 insertions(+), 111 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/ext/declarative.py b/lib/sqlalchemy/ext/declarative.py index 891130a48..7e5ae03d9 100755 --- a/lib/sqlalchemy/ext/declarative.py +++ b/lib/sqlalchemy/ext/declarative.py @@ -1392,6 +1392,10 @@ class _GetTable(object): def _deferred_relationship(cls, prop): def resolve_arg(arg): import sqlalchemy + from sqlalchemy.orm import foreign, remote + + fallback = sqlalchemy.__dict__.copy() + fallback.update({'foreign':foreign, 'remote':remote}) def access_cls(key): if key in cls._decl_class_registry: @@ -1401,7 +1405,7 @@ def _deferred_relationship(cls, prop): elif key in cls.metadata._schemas: return _GetTable(key, cls.metadata) else: - return sqlalchemy.__dict__[key] + return fallback[key] d = util.PopulateDict(access_cls) def return_cls(): diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index adf3b0cde..c1503a79a 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -163,38 +163,42 @@ class JoinCondition(object): "condition between parent/child tables on " "relationship %s - there are no foreign keys " "linking these tables via secondary table '%s'. " - "Ensure that referencing columns are associated with a "\ - "ForeignKey or ForeignKeyConstraint, or specify 'primaryjoin' "\ - "and 'secondaryjoin' expressions." + "Ensure that referencing columns are associated " + "with a ForeignKey or ForeignKeyConstraint, or " + "specify 'primaryjoin' and 'secondaryjoin' " + "expressions." % (self.prop, self.secondary)) else: raise sa_exc.NoForeignKeysError("Could not determine join " "condition between parent/child tables on " "relationship %s - there are no foreign keys " "linking these tables. " - "Ensure that referencing columns are associated with a " - "ForeignKey or ForeignKeyConstraint, or specify a 'primaryjoin' " - "expression." + "Ensure that referencing columns are associated " + "with a ForeignKey or ForeignKeyConstraint, or " + "specify a 'primaryjoin' expression." % self.prop) except sa_exc.AmbiguousForeignKeysError, afke: if self.secondary is not None: - raise sa_exc.AmbiguousForeignKeysError("Could not determine join " + raise sa_exc.AmbiguousForeignKeysError( + "Could not determine join " "condition between parent/child tables on " "relationship %s - there are multiple foreign key " "paths linking the tables via secondary table '%s'. " "Specify the 'foreign_keys' " "argument, providing a list of those columns which " - "should be counted as containing a foreign key reference " - "from the secondary table to each of the parent and child tables." + "should be counted as containing a foreign key " + "reference from the secondary table to each of the " + "parent and child tables." % (self.prop, self.secondary)) else: - raise sa_exc.AmbiguousForeignKeysError("Could not determine join " + raise sa_exc.AmbiguousForeignKeysError( + "Could not determine join " "condition between parent/child tables on " "relationship %s - there are multiple foreign key " - "paths linking the tables. Specify the 'foreign_keys' " - "argument, providing a list of those columns which " - "should be counted as containing a foreign key reference " - "to the parent table." + "paths linking the tables. Specify the " + "'foreign_keys' argument, providing a list of those " + "columns which should be counted as containing a " + "foreign key reference to the parent table." % self.prop) @util.memoized_property @@ -690,13 +694,13 @@ class JoinCondition(object): def _gather_join_annotations(self, annotation): s = set( - self._gather_columns_with_annotation(self.primaryjoin, - annotation) + self._gather_columns_with_annotation( + self.primaryjoin, annotation) ) if self.secondaryjoin is not None: s.update( - self._gather_columns_with_annotation(self.secondaryjoin, - annotation) + self._gather_columns_with_annotation( + self.secondaryjoin, annotation) ) return s @@ -735,8 +739,8 @@ class JoinCondition(object): # adjust the join condition for single table inheritance, # in the case that the join is to a subclass - # this is analogous to the "_adjust_for_single_table_inheritance()" - # method in Query. + # this is analogous to the + # "_adjust_for_single_table_inheritance()" method in Query. if single_crit is not None: if secondaryjoin is not None: @@ -778,51 +782,49 @@ class JoinCondition(object): return primaryjoin, secondaryjoin, secondary, \ target_adapter, dest_selectable + def create_lazy_clause(self, reverse_direction=False): + binds = util.column_dict() + lookup = util.column_dict() + equated_columns = util.column_dict() + + if reverse_direction and self.secondaryjoin is None: + for l, r in self.local_remote_pairs: + _list = lookup.setdefault(r, []) + _list.append((r, l)) + equated_columns[l] = r + else: + for l, r in self.local_remote_pairs: + _list = lookup.setdefault(l, []) + _list.append((l, r)) + equated_columns[r] = l + + def col_to_bind(col): + if col in lookup: + for tobind, equated in lookup[col]: + if equated in binds: + return None + if col not in binds: + binds[col] = sql.bindparam( + None, None, type_=col.type, unique=True) + return binds[col] + return None + + lazywhere = self.primaryjoin + + if self.secondaryjoin is None or not reverse_direction: + lazywhere = visitors.replacement_traverse( + lazywhere, {}, col_to_bind) + + if self.secondaryjoin is not None: + secondaryjoin = self.secondaryjoin + if reverse_direction: + secondaryjoin = visitors.replacement_traverse( + secondaryjoin, {}, col_to_bind) + lazywhere = sql.and_(lazywhere, secondaryjoin) + + bind_to_col = dict((binds[col].key, col) for col in binds) -################# everything below is TODO ################################ - -def _create_lazy_clause(cls, prop, reverse_direction=False): - binds = util.column_dict() - lookup = util.column_dict() - equated_columns = util.column_dict() - - if reverse_direction and prop.secondaryjoin is None: - for l, r in prop.local_remote_pairs: - _list = lookup.setdefault(r, []) - _list.append((r, l)) - equated_columns[l] = r - else: - for l, r in prop.local_remote_pairs: - _list = lookup.setdefault(l, []) - _list.append((l, r)) - equated_columns[r] = l - - def col_to_bind(col): - if col in lookup: - for tobind, equated in lookup[col]: - if equated in binds: - return None - if col not in binds: - binds[col] = sql.bindparam(None, None, type_=col.type, unique=True) - return binds[col] - return None - - lazywhere = prop.primaryjoin - - if prop.secondaryjoin is None or not reverse_direction: - lazywhere = visitors.replacement_traverse( - lazywhere, {}, col_to_bind) - - if prop.secondaryjoin is not None: - secondaryjoin = prop.secondaryjoin - if reverse_direction: - secondaryjoin = visitors.replacement_traverse( - secondaryjoin, {}, col_to_bind) - lazywhere = sql.and_(lazywhere, secondaryjoin) - - bind_to_col = dict((binds[col].key, col) for col in binds) - - return lazywhere, bind_to_col, equated_columns + return lazywhere, bind_to_col, equated_columns diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 00739ea03..a4cdfba1a 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -324,14 +324,14 @@ class LazyLoader(AbstractRelationshipLoader): def init(self): super(LazyLoader, self).init() + join_condition = self.parent_property._join_condition self._lazywhere, \ self._bind_to_col, \ - self._equated_columns = self._create_lazy_clause(self.parent_property) + self._equated_columns = join_condition.create_lazy_clause() self._rev_lazywhere, \ self._rev_bind_to_col, \ - self._rev_equated_columns = self._create_lazy_clause( - self.parent_property, + self._rev_equated_columns = join_condition.create_lazy_clause( reverse_direction=True) self.logger.info("%s lazy loading clause %s", self, self._lazywhere) @@ -617,49 +617,6 @@ class LazyLoader(AbstractRelationshipLoader): return reset_for_lazy_callable, None, None - @classmethod - def _create_lazy_clause(cls, prop, reverse_direction=False): - binds = util.column_dict() - lookup = util.column_dict() - equated_columns = util.column_dict() - - if reverse_direction and prop.secondaryjoin is None: - for l, r in prop.local_remote_pairs: - _list = lookup.setdefault(r, []) - _list.append((r, l)) - equated_columns[l] = r - else: - for l, r in prop.local_remote_pairs: - _list = lookup.setdefault(l, []) - _list.append((l, r)) - equated_columns[r] = l - - def col_to_bind(col): - if col in lookup: - for tobind, equated in lookup[col]: - if equated in binds: - return None - if col not in binds: - binds[col] = sql.bindparam(None, None, type_=col.type, unique=True) - return binds[col] - return None - - lazywhere = prop.primaryjoin - - if prop.secondaryjoin is None or not reverse_direction: - lazywhere = visitors.replacement_traverse( - lazywhere, {}, col_to_bind) - - if prop.secondaryjoin is not None: - secondaryjoin = prop.secondaryjoin - if reverse_direction: - secondaryjoin = visitors.replacement_traverse( - secondaryjoin, {}, col_to_bind) - lazywhere = sql.and_(lazywhere, secondaryjoin) - - bind_to_col = dict((binds[col].key, col) for col in binds) - - return lazywhere, bind_to_col, equated_columns log.class_logger(LazyLoader) -- cgit v1.2.1 From 35adeb95bf917330e1366f8a7252999419819fb1 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 3 Apr 2012 11:24:16 -0400 Subject: then spiff up that error msg --- lib/sqlalchemy/orm/relationships.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index c1503a79a..1a6b8a608 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -530,8 +530,13 @@ class JoinCondition(object): def _check_remote_side(self): if not self.local_remote_pairs: raise sa_exc.ArgumentError('Relationship %s could ' - 'not determine any local/remote column ' - 'pairs.' + 'not determine any unambiguous local/remote column ' + 'pairs based on join condition and remote_side ' + 'arguments. ' + 'Consider using the remote() annotation to ' + 'accurately mark those elements of the join ' + 'condition that are on the remote side of ' + 'the relationship.' % (self.prop, )) def _check_foreign_cols(self, join_condition, primary): -- cgit v1.2.1