defer construction of select._froms to allow for uninitialized columns

Issue #2261 resolved
Mike Bayer repo owner created an issue

test:

from sqlalchemy import *

def test_one():
    c1 = Column('c1', Integer)
    c2 = Column('c2', Integer)

    s = select([c1](c1))

    t = Table('t', MetaData(), c1, c2)

    assert c1._from_objects == [t](t)
    assert c2._from_objects == [t](t)

    assert str(select([c1](c1))) == "SELECT t.c1 \nFROM t"
    assert str(select([c2](c2))) == "SELECT t.c2 \nFROM t"

def test_two():
    c1 = Column('c1', Integer)
    c2 = Column('c2', Integer)

    s = select([c1](c1))

    # force a compile.  
    assert str(s) == "SELECT c1"

    # this will emit a warning
    t = Table('t', MetaData(), c1, c2)

    assert c1._from_objects == [t](t)
    assert c2._from_objects == [t](t)

    # 's' has been baked.  Can't afford
    # not caching select._froms.
    # hopefully the warning will clue the user
    assert str(s) == "SELECT t.c1"

    assert str(select([c1](c1))) == "SELECT t.c1 \nFROM t"
    assert str(select([c2](c2))) == "SELECT t.c2 \nFROM t"


test_one()
test_two()

note the second test tests that the Column resets it's internal cached items any time table is set.

patch:

diff -r 647a28a03e21c70c2195c2a0ea6f6c3e21d72958 lib/sqlalchemy/sql/compiler.py
--- a/lib/sqlalchemy/sql/compiler.py    Thu Aug 18 13:03:30 2011 -0400
+++ b/lib/sqlalchemy/sql/compiler.py    Thu Aug 18 16:51:37 2011 -0400
@@ -965,7 +965,7 @@
         if colparams or not supports_default_values:
             text += " (%s)" % ', '.join([preparer.format_column(c[0](preparer.format_column(c[0))
                        for c in colparams])
-        
+
         if self.returning or insert_stmt._returning:
             self.returning = self.returning or insert_stmt._returning
             returning_clause = self.returning_clause(
diff -r 647a28a03e21c70c2195c2a0ea6f6c3e21d72958 lib/sqlalchemy/sql/expression.py
--- a/lib/sqlalchemy/sql/expression.py  Thu Aug 18 13:03:30 2011 -0400
+++ b/lib/sqlalchemy/sql/expression.py  Thu Aug 18 16:51:37 2011 -0400
@@ -3821,19 +3821,36 @@

     onupdate = default = server_default = server_onupdate = None

+    _memoized_property = util.group_expirable_memoized_property()
+
     def __init__(self, text, selectable=None, type_=None, is_literal=False):
         self.key = self.name = text
         self.table = selectable
         self.type = sqltypes.to_instance(type_)
         self.is_literal = is_literal

-    @util.memoized_property
+    def _get_table(self):
+        return self.__dict__['table']('table')
+    def _set_table(self, table):
+        if '_from_objects' in self.__dict__:
+            util.warn("%s being associated with %s object after "
+                        "the %s has already been used in a SQL "
+                        "generation; previously generated "
+                        "constructs may contain stale state." % 
+                        (type(table), type(self), type(self)))
+        self._memoized_property.expire_instance(self)
+        self.__dict__['table']('table') = table
+
+    table = property(_get_table, _set_table)
+
+    @_memoized_property
     def _from_objects(self):
         if self.table is not None:
             return [self.table](self.table)
         else:
             return [
+
     @util.memoized_property
     def description(self):
         # Py3K
@@ -3842,7 +3859,7 @@
         return self.name.encode('ascii', 'backslashreplace')
         # end Py2K

-    @util.memoized_property
+    @_memoized_property
     def _label(self):
         if self.is_literal:
             return None
@@ -4344,7 +4361,6 @@
                             ](]
)

         self._correlate = set()
-        self._froms = util.OrderedSet()

         try:
             cols_present = bool(columns)
@@ -4359,23 +4375,15 @@
                 if isinstance(c, _ScalarSelect):
                     c = c.self_group(against=operators.comma_op)
                 self._raw_columns.append(c)
-
-            self._froms.update(_from_objects(*self._raw_columns))
         else:
             self._raw_columns = []

         if whereclause is not None:
             self._whereclause = _literal_as_text(whereclause)
-            self._froms.update(_from_objects(self._whereclause))
         else:
             self._whereclause = None

-        if from_obj is not None:
-            for f in util.to_list(from_obj):
-                if _is_literal(f):
-                    self._froms.add(_TextClause(f))
-                else:
-                    self._froms.add(f)
+        self._from_obj = from_obj

         if having is not None:
             self._having = _literal_as_text(having)
@@ -4387,6 +4395,20 @@

         _SelectBase.__init__(self, **kwargs)

+    @util.memoized_property
+    def _froms(self):
+        froms = util.OrderedSet()
+        froms.update(_from_objects(*self._raw_columns))
+        if self._whereclause is not None:
+            froms.update(_from_objects(self._whereclause))
+        if self._from_obj is not None:
+            for f in util.to_list(self._from_obj):
+                if _is_literal(f):
+                    froms.add(_TextClause(f))
+                else:
+                    froms.add(f)
+        return froms
+
     def _get_display_froms(self, existing_froms=None):
         """Return the full list of 'from' clauses to be displayed.

Comments (4)

  1. Mike Bayer reporter
    • changed milestone to 0.7.3

    the fix for this is spreading a little more and I'm making more usage of _memoized_property. As it's starting to deal with internals that were already modified a bunch in 0.7 I'm going to move this off of the 0.6 series.

  2. Mike Bayer reporter

    attached patch passes almost every test, though these changes are extremely volatile - large blocks of tests fail with tiny adjustments.

  3. Log in to comment