- added "ddl" argument to the "on" callable of DDLElement [ticket:1538]

- fixed the imports in the "postgres" cleanup dialect
- renamed "schema_item" attribute/argument of DDLElement
  to "target".

7 files changed, 98 insertions, 77 deletions

diff --git a/CHANGES b/CHANGES
index 44bd7e577..f6d3ef5e6 100644
--- a/CHANGES
+++ b/CHANGES
@@ -183,7 +183,10 @@ CHANGES
       is compiled into Table objects so that consistency is at a maximum.
       
 - DDL
-    - the DDL() system has been greatly expanded:
+    - the DDL system has been greatly expanded.  the DDL() class
+      now extends the more generic DDLElement(), which forms the basis
+      of many new constructs:
+      
         - CreateTable()
         - DropTable()
         - AddConstraint()
@@ -192,15 +195,27 @@ CHANGES
         - DropIndex()
         - CreateSequence()
         - DropSequence()
-        - these support "on" and "execute-at()" just like
-          plain DDL() does.
-
-    - The "on" callable passed to DDL() needs to accept **kw
-      arguments. In the case of MetaData before/after
-      create/drop, the list of Table objects for which
-      CREATE/DROP DDL is to be issued is passed as the kw
-      argument "tables". This is necessary for metadata-level
-      DDL that is dependent on the presence of specific tables.
+        
+       These support "on" and "execute-at()" just like plain DDL() 
+       does.  User-defined DDLElement subclasses can be created and 
+       linked to a compiler using the sqlalchemy.ext.compiler extension.
+
+    - The signature of the "on" callable passed to DDL() and
+      DDLElement() is revised as follows:
+      
+        "ddl" - the DDLElement object itself.
+        "event" - the string event name.
+        "target" - previously "schema_item", the Table or 
+        MetaData object triggering the event.
+        "connection" - the Connection object in use for the operation.
+        **kw - keyword arguments.  In the case of MetaData before/after
+          create/drop, the list of Table objects for which
+          CREATE/DROP DDL is to be issued is passed as the kw
+          argument "tables". This is necessary for metadata-level
+          DDL that is dependent on the presence of specific tables.
+
+      - the "schema_item" attribute of DDL has been renamed to 
+        "target".
 
 - dialect refactor
     - Dialect modules are now broken into database dialects
diff --git a/doc/build/metadata.rst b/doc/build/metadata.rst
index 38309f08d..996fecdd0 100644
--- a/doc/build/metadata.rst
+++ b/doc/build/metadata.rst
@@ -758,16 +758,16 @@ Or to any set of dialects::
     AddConstraint(constraint, on=('postgresql', 'mysql')).execute_at("after-create", users)
     DropConstraint(constraint, on=('postgresql', 'mysql')).execute_at("before-drop", users)
     
-When using a callable, the callable is passed the event name, the schema object operated upon, and the ``Connection`` object being used for the operation, as well as additional information as keyword arguments.  The callable can perform checks, such as whether or not a given item already exists:
+When using a callable, the callable is passed the ddl element, event name, the ``Table`` or ``MetaData`` object whose "create" or "drop" event is in progress, and the ``Connection`` object being used for the operation, as well as additional information as keyword arguments.  The callable can perform checks, such as whether or not a given item already exists.  Below we define ``should_create()`` and ``should_drop()`` callables that check for the presence of our named constraint:
 
 .. sourcecode:: python+sql
 
-    def should_create(event, schema_item, connection, **kw):
-        row = connection.execute("select relname from pg_class where relname='%s'" % schema_item.name).scalar()
-        return bool(row)
-    
-    def should_drop(event, schema_item, connection, **kw):
-        return not should_create(event, schema_item, connection, **kw)
+    def should_create(ddl, event, target, connection, **kw):
+        row = connection.execute("select conname from pg_constraint where conname='%s'" % ddl.element.name).scalar()
+        return not bool(row)
+
+    def should_drop(ddl, event, target, connection, **kw):
+        return not should_create(ddl, event, target, connection, **kw)
         
     AddConstraint(constraint, on=should_create).execute_at("after-create", users)
     DropConstraint(constraint, on=should_drop).execute_at("before-drop", users)
diff --git a/lib/sqlalchemy/dialects/postgres.py b/lib/sqlalchemy/dialects/postgres.py
index e66989fa7..0c1d3fd25 100644
--- a/lib/sqlalchemy/dialects/postgres.py
+++ b/lib/sqlalchemy/dialects/postgres.py
@@ -6,4 +6,5 @@ warn_deprecated(
     "The new URL format is postgresql[+driver]://<user>:<pass>@<host>/<dbname>"
     )
     
-from sqlalchemy.dialects.postgresql import *
\ No newline at end of file
+from sqlalchemy.dialects.postgresql import *
+from sqlalchemy.dialects.postgresql import base
diff --git a/lib/sqlalchemy/schema.py b/lib/sqlalchemy/schema.py
index b91764da1..845459e81 100644
--- a/lib/sqlalchemy/schema.py
+++ b/lib/sqlalchemy/schema.py
@@ -360,7 +360,7 @@ class Table(SchemaItem, expression.TableClause):
 
           event
             The event currently being handled
-          schema_item
+          target
             The ``Table`` object being created or dropped
           bind
             The ``Connection`` bueing used for DDL execution.
@@ -1250,10 +1250,12 @@ class Constraint(SchemaItem):
 
     @property
     def table(self):
-        if isinstance(self.parent, Table):
-            return self.parent
-        else:
-            raise exc.InvalidRequestError("This constraint is not bound to a table.")
+        try:
+            if isinstance(self.parent, Table):
+                return self.parent
+        except AttributeError:
+            pass
+        raise exc.InvalidRequestError("This constraint is not bound to a table.  Did you mean to call table.add_constraint(constraint) ?")
 
     def _set_parent(self, parent):
         self.parent = parent
@@ -1451,7 +1453,7 @@ class ForeignKeyConstraint(Constraint):
             fk._set_parent(col)
             
         if self.use_alter:
-            def supports_alter(event, schema_item, bind, **kw):
+            def supports_alter(ddl, event, schema_item, bind, **kw):
                 return table in set(kw['tables']) and bind.dialect.supports_alter
             AddConstraint(self, on=supports_alter).execute_at('after-create', table.metadata)
             DropConstraint(self, on=supports_alter).execute_at('before-drop', table.metadata)
@@ -1770,7 +1772,7 @@ class MetaData(SchemaItem):
 
           event
             The event currently being handled
-          schema_item
+          target
             The ``MetaData`` object being operated upon
           bind
             The ``Connection`` bueing used for DDL execution.
@@ -1917,10 +1919,10 @@ class DDLElement(expression.ClauseElement):
     supports_execution = True
     _autocommit = True
 
-    schema_item = None
+    target = None
     on = None
     
-    def execute(self, bind=None, schema_item=None):
+    def execute(self, bind=None, target=None):
         """Execute this DDL immediately.
 
         Executes the DDL statement in isolation using the supplied
@@ -1932,24 +1934,25 @@ class DDLElement(expression.ClauseElement):
           Optional, an ``Engine`` or ``Connection``.  If not supplied, a
           valid :class:`~sqlalchemy.engine.base.Connectable` must be present in the ``.bind`` property.
 
-        schema_item
-          Optional, defaults to None.  Will be passed to the ``on`` callable
-          criteria, if any, and may provide string expansion data for the
+        target
+          Optional, defaults to None.  The target SchemaItem for the 
+          execute call.  Will be passed to the ``on`` callable if any, 
+          and may also provide string expansion data for the
           statement. See ``execute_at`` for more information.
         """
 
         if bind is None:
             bind = _bind_or_error(self)
 
-        if self._should_execute(None, schema_item, bind):
-            return bind.execute(self.against(schema_item))
+        if self._should_execute(None, target, bind):
+            return bind.execute(self.against(target))
         else:
             bind.engine.logger.info("DDL execution skipped, criteria not met.")
 
-    def execute_at(self, event, schema_item):
+    def execute_at(self, event, target):
         """Link execution of this DDL to the DDL lifecycle of a SchemaItem.
 
-        Links this ``DDL`` to a ``Table`` or ``MetaData`` instance, executing
+        Links this ``DDLElement`` to a ``Table`` or ``MetaData`` instance, executing
         it when that schema item is created or dropped.  The DDL statement
         will be executed using the same Connection and transactional context
         as the Table create/drop itself.  The ``.bind`` property of this
@@ -1959,23 +1962,11 @@ class DDLElement(expression.ClauseElement):
           One of the events defined in the schema item's ``.ddl_events``;
           e.g. 'before-create', 'after-create', 'before-drop' or 'after-drop'
 
-        schema_item
-          A Table or MetaData instance
-
-        When operating on Table events, the following additional ``statement``
-        string substitions are available::
+        target
+          The Table or MetaData instance for which this DDLElement will
+          be associated with.
 
-            %(table)s  - the Table name, with any required quoting applied
-            %(schema)s - the schema name, with any required quoting applied
-            %(fullname)s - the Table name including schema, quoted if needed
-
-        The DDL's ``context``, if any, will be combined with the standard
-        substutions noted above.  Keys present in the context will override
-        the standard substitutions.
-
-        A DDL instance can be linked to any number of schema items. The
-        statement subsitution support allows for DDL instances to be used in a
-        template fashion.
+        A DDLElement instance can be linked to any number of schema items. 
 
         ``execute_at`` builds on the ``append_ddl_listener`` interface of
         MetaDta and Table objects.
@@ -1985,27 +1976,27 @@ class DDLElement(expression.ClauseElement):
         in a future release.
         """
 
-        if not hasattr(schema_item, 'ddl_listeners'):
+        if not hasattr(target, 'ddl_listeners'):
             raise exc.ArgumentError(
-                "%s does not support DDL events" % type(schema_item).__name__)
-        if event not in schema_item.ddl_events:
+                "%s does not support DDL events" % type(target).__name__)
+        if event not in target.ddl_events:
             raise exc.ArgumentError(
                 "Unknown event, expected one of (%s), got '%r'" %
-                (', '.join(schema_item.ddl_events), event))
-        schema_item.ddl_listeners[event].append(self)
+                (', '.join(target.ddl_events), event))
+        target.ddl_listeners[event].append(self)
         return self
 
     @expression._generative
-    def against(self, schema_item):
+    def against(self, target):
         """Return a copy of this DDL against a specific schema item."""
 
-        self.schema_item = schema_item
+        self.target = target
 
-    def __call__(self, event, schema_item, bind, **kw):
+    def __call__(self, event, target, bind, **kw):
         """Execute the DDL as a ddl_listener."""
 
-        if self._should_execute(event, schema_item, bind, **kw):
-            return bind.execute(self.against(schema_item))
+        if self._should_execute(event, target, bind, **kw):
+            return bind.execute(self.against(target))
 
     def _check_ddl_on(self, on):
         if (on is not None and
@@ -2014,7 +2005,7 @@ class DDLElement(expression.ClauseElement):
                 "Expected the name of a database dialect, a tuple of names, or a callable for "
                 "'on' criteria, got type '%s'." % type(on).__name__)
 
-    def _should_execute(self, event, schema_item, bind, **kw):
+    def _should_execute(self, event, target, bind, **kw):
         if self.on is None:
             return True
         elif isinstance(self.on, basestring):
@@ -2022,7 +2013,7 @@ class DDLElement(expression.ClauseElement):
         elif isinstance(self.on, (tuple, list, set)):
             return bind.engine.name in self.on
         else:
-            return self.on(event, schema_item, bind, **kw)
+            return self.on(self, event, target, bind, **kw)
 
     def bind(self):
         if self._bind:
@@ -2060,7 +2051,18 @@ class DDL(DDLElement):
 
       drop_spow = DDL('ALTER TABLE users SET secretpowers FALSE')
       connection.execute(drop_spow)
-      
+
+    When operating on Table events, the following ``statement``
+    string substitions are available::
+
+      %(table)s  - the Table name, with any required quoting applied
+      %(schema)s - the schema name, with any required quoting applied
+      %(fullname)s - the Table name including schema, quoted if needed
+
+    The DDL's ``context``, if any, will be combined with the standard
+    substutions noted above.  Keys present in the context will override
+    the standard substitutions.
+
     """
 
     __visit_name__ = "ddl"
@@ -2088,16 +2090,19 @@ class DDL(DDLElement):
 
             DDL('something', on=('postgresql', 'mysql'))
 
-          If a callable, it will be invoked with three positional arguments
+          If a callable, it will be invoked with four positional arguments
           as well as optional keyword arguments:
-
+            
+            ddl
+              This DDL element.
+              
             event
               The name of the event that has triggered this DDL, such as
               'after-create' Will be None if the DDL is executed explicitly.
 
-            schema_item
-              A SchemaItem instance, such as ``Table`` or ``MetaData``. May be
-              None if the DDL is executed explicitly.
+            target
+              The ``Table`` or ``MetaData`` object which is the target of 
+              this event. May be None if the DDL is executed explicitly.
 
             connection
               The ``Connection`` being used for DDL execution
diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index b4b901067..4cf4bd869 100644
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -922,11 +922,11 @@ class DDLCompiler(engine.Compiled):
     def visit_ddl(self, ddl, **kwargs):
         # table events can substitute table and schema name
         context = ddl.context
-        if isinstance(ddl.schema_item, schema.Table):
+        if isinstance(ddl.target, schema.Table):
             context = context.copy()
 
             preparer = self.dialect.identifier_preparer
-            path = preparer.format_table_seq(ddl.schema_item)
+            path = preparer.format_table_seq(ddl.target)
             if len(path) == 1:
                 table, sch = path[0], ''
             else:
@@ -934,7 +934,7 @@ class DDLCompiler(engine.Compiled):
 
             context.setdefault('table', table)
             context.setdefault('schema', sch)
-            context.setdefault('fullname', preparer.format_table(ddl.schema_item))
+            context.setdefault('fullname', preparer.format_table(ddl.target))
         
         return ddl.statement % context
 
diff --git a/test/engine/test_ddlevents.py b/test/engine/test_ddlevents.py
index 6fe170a23..2e5817c01 100644
--- a/test/engine/test_ddlevents.py
+++ b/test/engine/test_ddlevents.py
@@ -302,7 +302,7 @@ class DDLExecutionTest(TestBase):
             assert list(r) == [(1,)], py
 
         for py in ('ddl.execute()',
-                   'ddl.execute(schema_item=table)'):
+                   'ddl.execute(target=table)'):
             try:
                 r = eval(py)
                 assert False
@@ -312,7 +312,7 @@ class DDLExecutionTest(TestBase):
         for bind in engine, cx:
             ddl.bind = bind
             for py in ('ddl.execute()',
-                       'ddl.execute(schema_item=table)'):
+                       'ddl.execute(target=table)'):
                 r = eval(py)
                 assert list(r) == [(1,)], py
 
@@ -358,8 +358,8 @@ class DDLTest(TestBase, AssertsCompiledSQL):
         assert DDL('')._should_execute('x', tbl, cx)
         assert DDL('', on=target)._should_execute('x', tbl, cx)
         assert not DDL('', on='bogus')._should_execute('x', tbl, cx)
-        assert DDL('', on=lambda x,y,z: True)._should_execute('x', tbl, cx)
-        assert(DDL('', on=lambda x,y,z: z.engine.name != 'bogus').
+        assert DDL('', on=lambda d, x,y,z: True)._should_execute('x', tbl, cx)
+        assert(DDL('', on=lambda d, x,y,z: z.engine.name != 'bogus').
                _should_execute('x', tbl, cx))
 
     def test_repr(self):
diff --git a/test/orm/test_defaults.py b/test/orm/test_defaults.py
index 5379c9714..0e7c6d08a 100644
--- a/test/orm/test_defaults.py
+++ b/test/orm/test_defaults.py
@@ -42,7 +42,7 @@ class TriggerDefaultsTest(_base.MappedTest):
             sa.DDL("CREATE TRIGGER dt_ins BEFORE INSERT ON dt "
                          "FOR EACH ROW BEGIN "
                          "SET NEW.col2='ins'; SET NEW.col4='ins'; END",
-                         on=lambda event, schema_item, bind, **kw: 
+                         on=lambda ddl, event, target, bind, **kw: 
                                 bind.engine.name not in ('oracle', 'mssql', 'sqlite')
                 ),
             ):
@@ -67,7 +67,7 @@ class TriggerDefaultsTest(_base.MappedTest):
             sa.DDL("CREATE TRIGGER dt_up BEFORE UPDATE ON dt "
                         "FOR EACH ROW BEGIN "
                         "SET NEW.col3='up'; SET NEW.col4='up'; END",
-                        on=lambda event, schema_item, bind, **kw: 
+                        on=lambda ddl, event, target, bind, **kw: 
                                 bind.engine.name not in ('oracle', 'mssql', 'sqlite')
                     ),
             ):