- getting slightly more consistent behavior for the edge case of pk columns

with server default - autoincrement is now false with any server_default,
so these all return None, applies consistency to [ticket:2020], [ticket:2021].
if prefetch is desired a "default" should be used instead of server_default.

8 files changed, 58 insertions, 38 deletions

diff --git a/lib/sqlalchemy/dialects/oracle/base.py b/lib/sqlalchemy/dialects/oracle/base.py
index 7ed4ca07e..63ad37ce9 100644
--- a/lib/sqlalchemy/dialects/oracle/base.py
+++ b/lib/sqlalchemy/dialects/oracle/base.py
@@ -594,9 +594,9 @@ class OracleIdentifierPreparer(compiler.IdentifierPreparer):
 
 class OracleExecutionContext(default.DefaultExecutionContext):
     def fire_sequence(self, seq, type_):
-        return int(self._execute_scalar("SELECT " + 
+        return self._execute_scalar("SELECT " + 
                     self.dialect.identifier_preparer.format_sequence(seq) + 
-                    ".nextval FROM DUAL"), type_)
+                    ".nextval FROM DUAL", type_)
 
 class OracleDialect(default.DefaultDialect):
     name = 'oracle'
diff --git a/lib/sqlalchemy/dialects/postgresql/base.py b/lib/sqlalchemy/dialects/postgresql/base.py
index 7c712e8aa..a8fb4e51a 100644
--- a/lib/sqlalchemy/dialects/postgresql/base.py
+++ b/lib/sqlalchemy/dialects/postgresql/base.py
@@ -508,13 +508,15 @@ class PGDDLCompiler(compiler.DDLCompiler):
         colspec = self.preparer.format_column(column)
         type_affinity = column.type._type_affinity
         if column.primary_key and \
-            len(column.foreign_keys)==0 and \
-            column.autoincrement and \
-            issubclass(type_affinity, sqltypes.Integer) and \
+            column is column.table._autoincrement_column and \
             not issubclass(type_affinity, sqltypes.SmallInteger) and \
-            (column.default is None or 
-                (isinstance(column.default, schema.Sequence) and
-                column.default.optional)):
+            (
+                column.default is None or 
+                (
+                    isinstance(column.default, schema.Sequence) and
+                    column.default.optional
+                )
+            ):
             if issubclass(type_affinity, sqltypes.BigInteger):
                 colspec += " BIGSERIAL"
             else:
@@ -689,7 +691,7 @@ class PGExecutionContext(default.DefaultExecutionContext):
             return None
 
     def get_insert_default(self, column):
-        if column.primary_key:
+        if column.primary_key and column is column.table._autoincrement_column:
             if (isinstance(column.server_default, schema.DefaultClause) and
                 column.server_default.arg is not None):
 
@@ -697,8 +699,7 @@ class PGExecutionContext(default.DefaultExecutionContext):
                 return self._execute_scalar("select %s" %
                                         column.server_default.arg, column.type)
 
-            elif column is column.table._autoincrement_column \
-                    and (column.default is None or 
+            elif (column.default is None or 
                         (isinstance(column.default, schema.Sequence) and
                         column.default.optional)):
 
diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py
index 3bdcad2ac..9eb1b8b40 100644
--- a/lib/sqlalchemy/engine/base.py
+++ b/lib/sqlalchemy/engine/base.py
@@ -2460,9 +2460,23 @@ class ResultProxy(object):
     @util.memoized_property
     def inserted_primary_key(self):
         """Return the primary key for the row just inserted.
-
-        This only applies to single row insert() constructs which
-        did not explicitly specify returning().
+        
+        The return value is a list of scalar values 
+        corresponding to the list of primary key columns
+        in the target table.
+
+        This only applies to single row :func:`.insert` 
+        constructs which did not explicitly specify 
+        :meth:`.Insert.returning`.
+        
+        Note that primary key columns which specify a
+        server_default clause, 
+        or otherwise do not qualify as "autoincrement"
+        columns (see the notes at :class:`.Column`), and were
+        generated using the database-side default, will
+        appear in this list as ``None`` unless the backend 
+        supports "returning" and the insert statement executed
+        with the "implicit returning" enabled.
 
         """
 
diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py
index e21ec1c40..da6ed12a6 100644
--- a/lib/sqlalchemy/engine/default.py
+++ b/lib/sqlalchemy/engine/default.py
@@ -101,7 +101,7 @@ class DefaultDialect(base.Dialect):
 
         if not getattr(self, 'ported_sqla_06', True):
             util.warn(
-                "The %s dialect is not yet ported to SQLAlchemy 0.6" %
+                "The %s dialect is not yet ported to SQLAlchemy 0.6/0.7" %
                 self.name)
 
         self.convert_unicode = convert_unicode
@@ -625,7 +625,8 @@ class DefaultExecutionContext(base.ExecutionContext):
         return self.dialect.supports_sane_multi_rowcount
 
     def post_insert(self):
-        if self.dialect.postfetch_lastrowid and \
+        if not self._is_implicit_returning and \
+            self.dialect.postfetch_lastrowid and \
             (not self.inserted_primary_key or \
                         None in self.inserted_primary_key):
 
diff --git a/lib/sqlalchemy/engine/reflection.py b/lib/sqlalchemy/engine/reflection.py
index cf254cba6..00b2fd1bf 100644
--- a/lib/sqlalchemy/engine/reflection.py
+++ b/lib/sqlalchemy/engine/reflection.py
@@ -398,7 +398,11 @@ class Inspector(object):
             if col_d.get('default') is not None:
                 # the "default" value is assumed to be a literal SQL expression,
                 # so is wrapped in text() so that no quoting occurs on re-issuance.
-                colargs.append(sa_schema.DefaultClause(sql.text(col_d['default'])))
+                colargs.append(
+                    sa_schema.DefaultClause(
+                        sql.text(col_d['default']), _reflected=True
+                    )
+                )
 
             if 'sequence' in col_d:
                 # TODO: mssql, maxdb and sybase are using this.
diff --git a/lib/sqlalchemy/schema.py b/lib/sqlalchemy/schema.py
index a530a1a7a..26f607512 100644
--- a/lib/sqlalchemy/schema.py
+++ b/lib/sqlalchemy/schema.py
@@ -325,11 +325,8 @@ class Table(SchemaItem, expression.TableClause):
             if col.autoincrement and \
                 issubclass(col.type._type_affinity, types.Integer) and \
                 not col.foreign_keys and \
-                isinstance(col.default, (type(None), Sequence)):
-                # don't look at server_default here since different backends may
-                # or may not have a server_default, e.g. postgresql reflected
-                # SERIAL cols will have a DefaultClause here but are still
-                # autoincrement. 
+                isinstance(col.default, (type(None), Sequence)) and \
+                (col.server_default is None or col.server_default.reflected):
                 return col
 
     @property
@@ -1231,6 +1228,7 @@ class DefaultGenerator(SchemaItem):
     __visit_name__ = 'default_generator'
 
     is_sequence = False
+    is_server_default = False
 
     def __init__(self, for_update=False):
         self.for_update = for_update
@@ -1423,6 +1421,8 @@ class FetchedValue(object):
     INSERT.
 
     """
+    is_server_default = True
+    reflected = False
 
     def __init__(self, for_update=False):
         self.for_update = for_update
@@ -1460,12 +1460,13 @@ class DefaultClause(FetchedValue):
 
     """
 
-    def __init__(self, arg, for_update=False):
+    def __init__(self, arg, for_update=False, _reflected=False):
         util.assert_arg_type(arg, (basestring,
                                    expression.ClauseElement,
                                    expression._TextClause), 'arg')
         super(DefaultClause, self).__init__(for_update)
         self.arg = arg
+        self.reflected = _reflected
 
     def __repr__(self):
         return "DefaultClause(%r, for_update=%r)" % \
diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index ce98dfb83..d906bf5d4 100644
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -1102,18 +1102,16 @@ class SQLCompiler(engine.Compiled):
                         else:
                             self.returning.append(c)
                     else:
-                        if (
-                            c.default is not None and \
-                                (
-                                    self.dialect.supports_sequences or 
-                                    not c.default.is_sequence
-                                )
-                            ) or \
-                             self.dialect.preexecute_autoincrement_sequences:
+                        if c.default is not None or \
+                            c is stmt.table._autoincrement_column and (
+                                self.dialect.supports_sequences or
+                                self.dialect.preexecute_autoincrement_sequences
+                            ):
 
                             values.append(
                                 (c, self._create_crud_bind_param(c, None))
                             )
+
                             self.prefetch.append(c)
 
                 elif c.default is not None:
diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py
index ede194f7c..6a368b8c0 100644
--- a/lib/sqlalchemy/sql/expression.py
+++ b/lib/sqlalchemy/sql/expression.py
@@ -4436,7 +4436,7 @@ class Select(_SelectBase):
         self._bind = bind
     bind = property(bind, _set_bind)
 
-class _UpdateBase(Executable, ClauseElement):
+class UpdateBase(Executable, ClauseElement):
     """Form the base for ``INSERT``, ``UPDATE``, and ``DELETE`` statements."""
 
     __visit_name__ = 'update_base'
@@ -4513,7 +4513,8 @@ class _UpdateBase(Executable, ClauseElement):
         """
         self._returning = cols
 
-class _ValuesBase(_UpdateBase):
+class ValuesBase(UpdateBase):
+    """Supplies support for :meth:`.ValuesBase.values` to INSERT and UPDATE constructs."""
 
     __visit_name__ = 'values_base'
 
@@ -4548,7 +4549,7 @@ class _ValuesBase(_UpdateBase):
             self.parameters.update(self._process_colparams(v))
             self.parameters.update(kwargs)
 
-class Insert(_ValuesBase):
+class Insert(ValuesBase):
     """Represent an INSERT construct.
 
     The :class:`Insert` object is created using the :func:`insert()` function.
@@ -4566,7 +4567,7 @@ class Insert(_ValuesBase):
                 prefixes=None, 
                 returning=None,
                 **kwargs):
-        _ValuesBase.__init__(self, table, values)
+        ValuesBase.__init__(self, table, values)
         self._bind = bind
         self.select = None
         self.inline = inline
@@ -4598,7 +4599,7 @@ class Insert(_ValuesBase):
         clause = _literal_as_text(clause)
         self._prefixes = self._prefixes + (clause,)
 
-class Update(_ValuesBase):
+class Update(ValuesBase):
     """Represent an Update construct.
 
     The :class:`Update` object is created using the :func:`update()` function.
@@ -4614,7 +4615,7 @@ class Update(_ValuesBase):
                 bind=None, 
                 returning=None,
                 **kwargs):
-        _ValuesBase.__init__(self, table, values)
+        ValuesBase.__init__(self, table, values)
         self._bind = bind
         self._returning = returning
         if whereclause is not None:
@@ -4650,7 +4651,7 @@ class Update(_ValuesBase):
             self._whereclause = _literal_as_text(whereclause)
 
 
-class Delete(_UpdateBase):
+class Delete(UpdateBase):
     """Represent a DELETE construct.
 
     The :class:`Delete` object is created using the :func:`delete()` function.