Merge "Fix LegacyRow/Row index access"

4 files changed, 44 insertions, 12 deletions

diff --git a/lib/sqlalchemy/cextension/resultproxy.c b/lib/sqlalchemy/cextension/resultproxy.c
index 89fd6947a..596b264d0 100644
--- a/lib/sqlalchemy/cextension/resultproxy.c
+++ b/lib/sqlalchemy/cextension/resultproxy.c
@@ -53,7 +53,7 @@ static PyObject *sqlalchemy_engine_row = NULL;
 static PyObject *sqlalchemy_engine_result = NULL;
 
 
-//static int KEY_INTEGER_ONLY = 0;
+static int KEY_INTEGER_ONLY = 0;
 static int KEY_OBJECTS_ONLY = 1;
 static int KEY_OBJECTS_BUT_WARN = 2;
 //static int KEY_OBJECTS_NO_WARN = 3;
@@ -415,6 +415,7 @@ BaseRow_subscript_impl(BaseRow *self, PyObject *key, int asmapping)
     PyObject *values;
     PyObject *result;
     long index;
+    PyObject *tmp;
 
 #if PY_MAJOR_VERSION < 3
     if (PyInt_CheckExact(key)) {
@@ -466,6 +467,14 @@ BaseRow_subscript_impl(BaseRow *self, PyObject *key, int asmapping)
         result = BaseRow_valuescollection(values, 1);
         Py_DECREF(values);
         return result;
+    }
+    else if (!asmapping && self->key_style == KEY_INTEGER_ONLY) {
+        tmp = PyObject_CallMethod(self->parent, "_raise_for_nonint", "(O)", key);
+        if (tmp == NULL) {
+            return NULL;
+        }
+        Py_DECREF(tmp);
+        return NULL;
     } else {
         return BaseRow_getitem_by_object(self, key, asmapping);
     }
@@ -480,7 +489,7 @@ BaseRow_subscript(BaseRow *self, PyObject *key)
 static PyObject *
 BaseRow_subscript_mapping(BaseRow *self, PyObject *key)
 {
-    if (self->key_style == KEY_OBJECTS_BUT_WARN) {
+    if (self->key_style == KEY_OBJECTS_BUT_WARN || self->key_style == KEY_INTEGER_ONLY) {
         return BaseRow_subscript_impl(self, key, 0);
     }
     else {
diff --git a/lib/sqlalchemy/engine/result.py b/lib/sqlalchemy/engine/result.py
index 10dfaa904..205efe441 100644
--- a/lib/sqlalchemy/engine/result.py
+++ b/lib/sqlalchemy/engine/result.py
@@ -80,6 +80,13 @@ class ResultMetaData(object):
         util.raise_(KeyError(key), replace_context=err)
 
     def _warn_for_nonint(self, key):
+        util.warn_deprecated_20(
+            "Retrieving row members using strings or other non-integers is "
+            "deprecated; use row._mapping for a dictionary interface "
+            "to the row"
+        )
+
+    def _raise_for_nonint(self, key):
         raise TypeError(
             "TypeError: tuple indices must be integers or slices, not %s"
             % type(key).__name__
diff --git a/lib/sqlalchemy/engine/row.py b/lib/sqlalchemy/engine/row.py
index b870e6534..7c15b7f6e 100644
--- a/lib/sqlalchemy/engine/row.py
+++ b/lib/sqlalchemy/engine/row.py
@@ -40,9 +40,18 @@ except ImportError:
 
 
 KEY_INTEGER_ONLY = 0
+"""__getitem__ only allows integer values, raises TypeError otherwise"""
+
 KEY_OBJECTS_ONLY = 1
+"""__getitem__ only allows string/object values, raises TypeError otherwise"""
+
 KEY_OBJECTS_BUT_WARN = 2
+"""__getitem__ allows integer or string/object values, but emits a 2.0
+deprecation warning if string/object is passed"""
+
 KEY_OBJECTS_NO_WARN = 3
+"""__getitem__ allows integer or string/object values with no warnings
+or errors."""
 
 try:
     from sqlalchemy.cresultproxy import BaseRow
@@ -100,15 +109,16 @@ except ImportError:
         def __hash__(self):
             return hash(self._data)
 
-        def __getitem__(self, key):
+        def _get_by_int_impl(self, key):
             return self._data[key]
 
-        _get_by_int_impl = __getitem__
-
         def _get_by_key_impl(self, key):
             if int in key.__class__.__mro__:
                 return self._data[key]
 
+            if self._key_style == KEY_INTEGER_ONLY:
+                self._parent._raise_for_nonint(key)
+
             # the following is all LegacyRow support.   none of this
             # should be called if not LegacyRow
             # assert isinstance(self, LegacyRow)
@@ -132,6 +142,12 @@ except ImportError:
 
             return self._data[mdindex]
 
+        # The original 1.4 plan was that Row would not allow row["str"]
+        # access, however as the C extensions were inadvertently allowing
+        # this coupled with the fact that orm Session sets future=True,
+        # this allows a softer upgrade path.  see #6218
+        __getitem__ = _get_by_key_impl
+
         def _get_by_key_impl_mapping(self, key):
             try:
                 rec = self._keymap[key]
@@ -192,7 +208,8 @@ class Row(BaseRow, collections_abc.Sequence):
 
     __slots__ = ()
 
-    _default_key_style = KEY_INTEGER_ONLY
+    # in 2.0, this should be KEY_INTEGER_ONLY
+    _default_key_style = KEY_OBJECTS_BUT_WARN
 
     @property
     def _mapping(self):
@@ -384,8 +401,11 @@ class LegacyRow(Row):
     def __contains__(self, key):
         return self._parent._contains(key, self)
 
-    if not _baserow_usecext:
-        __getitem__ = BaseRow._get_by_key_impl
+    # prior to #6218, LegacyRow would redirect the behavior of __getitem__
+    # for the non C version of BaseRow. This is now set up by Python BaseRow
+    # in all cases
+    # if not _baserow_usecext:
+    #    __getitem__ = BaseRow._get_by_key_impl
 
     @util.deprecated(
         "1.4",
diff --git a/lib/sqlalchemy/testing/warnings.py b/lib/sqlalchemy/testing/warnings.py
index 5699cd035..3236ecf89 100644
--- a/lib/sqlalchemy/testing/warnings.py
+++ b/lib/sqlalchemy/testing/warnings.py
@@ -60,10 +60,6 @@ def setup_filters():
         #        r".*DefaultGenerator.execute\(\)",
         #
         #
-        # result sets
-        #
-        r"The Row.keys\(\) method",
-        r"Using non-integer/slice indices on Row ",
         #
         # Core SQL constructs
         #