Repository: djrobstep/sqlakeyset (python)

Issue:
`row._mapping` is broken after pagination
Already found the culprit at https://github.com/djrobstep/sqlakeyset/blob/aa097fc055322ad0773d619115640f7b005152c8/sqlakeyset/sqla20.py#L105-L112

Instead of `key -> col value` mapping, `Row` wants `key -> col index` mapping.

I'm running SQLAlchemy 2.0.23 and I have no idea if something has changed after 2.0.11.

Pull request description:
Fix key mapping
Fixes #99.

Patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 48b2f2a..69551f4 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -105,7 +105,7 @@ workflows:
           matrix:
             parameters:
               python: ["3.7", "3.8", "3.9", "3.10", "3.11"]
-              sqlalchemy: ["1.3.24", "1.4.49", "2.0.0", "2.0.21"]
+              sqlalchemy: ["1.3.24", "1.4.49", "2.0.0", "2.0.23"]
       - publish:
           requires:
             - build
diff --git a/sqlakeyset/sqla14.py b/sqlakeyset/sqla14.py
index fc3d363..781e6c2 100644
--- a/sqlakeyset/sqla14.py
+++ b/sqlakeyset/sqla14.py
@@ -1,5 +1,6 @@
 """Methods for messing with the internals of SQLAlchemy >1.3 results."""
 from __future__ import annotations
+from sqlalchemy.engine.result import SimpleResultMetaData
 from sqlalchemy.engine.row import LegacyRow
 
 from .constants import ORDER_COL_PREFIX
@@ -37,8 +38,12 @@ def core_coerce_row(row, extra_columns, result_type):
     else:
         cls = TruncatedRow
 
+    parent = row._parent
+    if isinstance(parent, SimpleResultMetaData):
+        parent = parent._reduce(list(parent.keys)[:N])
+
     return cls(
-        row._parent,
+        parent,
         None,  # Processors are applied immediately in sqla1.4+
         {  # Strip out added OCs from the keymap:
             k: v
diff --git a/sqlakeyset/sqla20.py b/sqlakeyset/sqla20.py
index 9e17514..b602e92 100644
--- a/sqlakeyset/sqla20.py
+++ b/sqlakeyset/sqla20.py
@@ -1,7 +1,7 @@
 """Methods for messing with the internals of SQLAlchemy 1.4/2.0 results."""
 from __future__ import annotations
 
-from sqlalchemy.engine.result import result_tuple
+from sqlalchemy.engine.result import SimpleResultMetaData, result_tuple
 from sqlalchemy.engine.row import Row
 
 from .constants import ORDER_COL_PREFIX
@@ -60,7 +60,7 @@ def orm_coerce_row(row, extra_columns, result_type):
 def core_result_type(selectable, s):
     """Given a SQLAlchemy Core selectable and a connection/session, get the
     type constructor for the result row type."""
-    # Unused in sqlalchemy 1.4: see core_coerce_row implementation
+    # Unused in sqlalchemy 1.4+: see core_coerce_row implementation
     return None
 
 
@@ -98,6 +98,8 @@ def core_coerce_row(row: Row, extra_columns, result_type) -> Row:
     N = len(row) - len(extra_columns)
 
     parent = row._parent
+    if isinstance(parent, SimpleResultMetaData):
+        parent = parent._reduce(list(parent.keys)[:N])
     processors = None  # Processors are applied immediately in sqla1.4+
     data = row._data[:N]
 
@@ -105,7 +107,7 @@ def core_coerce_row(row: Row, extra_columns, result_type) -> Row:
         # 2.0.11+
         structure = (
             {  # Strip out added OCs from the keymap:
-                k: row[v]
+                k: v
                 for k, v in row._key_to_index.items()
                 if not (isinstance(k, str) and k.startswith(ORDER_COL_PREFIX))
             },


Test patch:
diff --git a/tests/test_paging.py b/tests/test_paging.py
index dd07018..1c1d30c 100644
--- a/tests/test_paging.py
+++ b/tests/test_paging.py
@@ -45,6 +45,7 @@ from conftest import (
     JoinedInheritanceBase,
     Base,
     SQLA2,
+    SQLA_VERSION,
 )
 
 warnings.simplefilter("error")
@@ -171,13 +172,41 @@ def test_orm_bad_page(dburl):
 
         # check that malformed page tuple fails
         with pytest.raises(InvalidPage):
-            get_page(q, per_page=10, page=((1,), False, "Potatoes"))
+            get_page(q, per_page=10, page=((1,), False, "Potatoes"))  # type: ignore
 
         # one order col, so check place with 2 elements fails
         with pytest.raises(InvalidPage):
             get_page(q, per_page=10, page=((1, 1), False))
 
 
+@pytest.mark.skipif(
+    SQLA_VERSION < version.parse("1.4.0b1"),
+    reason="._mapping doesn't exist in sqlalchemy<1.4",
+)
+def test_orm_row_mapping(dburl):
+    with S(dburl, echo=ECHO) as s:
+        q = s.query(Book.id, Book.author_id).order_by(Book.name, Book.id)
+        orig = q.first()
+        new = get_page(q, per_page=2)[0]
+        omap = orig._mapping
+        nmap = new._mapping
+        assert dict(omap) == dict(nmap)
+
+
+@pytest.mark.skipif(
+    SQLA_VERSION < version.parse("1.4.0b1"),
+    reason="._mapping doesn't exist in sqlalchemy<1.4",
+)
+def test_core_row_mapping(dburl):
+    with S(dburl, echo=ECHO) as s:
+        q = select(Book.id, Book.author_id).order_by(Book.name, Book.id)
+        orig = s.execute(q).first()
+        new = select_page(s, q, per_page=2)[0]
+        omap = orig._mapping
+        nmap = new._mapping
+        assert dict(omap) == dict(nmap)
+
+
 def test_orm_order_by_arrowtype(dburl):
     with S(dburl, echo=ECHO) as s:
         q = s.query(Book).order_by(Book.published_at, Book.id)
@@ -522,14 +551,14 @@ def test_orm_custom_session_bind(dburl):
         # implementation of get_bind() and no .bind attribute:
         s._custom_bind = s.bind
         delattr(s, "bind")
-        s.get_bind = lambda *a, **k: s._custom_bind
+        s.get_bind = lambda *_, **__: s._custom_bind
 
         q = s.query(Book, Author, Book.id).outerjoin(Author).order_by(*spec)
         check_paging_orm(q=q)
 
 
 def test_multiple_engines(dburl, joined_inheritance_dburl):
-    kw = {"future": True} if SQLA2 else {}
+    kw: dict = {"future": True} if SQLA2 else {}
     eng = sqlalchemy.create_engine(dburl, **kw)
     eng2 = sqlalchemy.create_engine(joined_inheritance_dburl, **kw)
     session_factory = sessionmaker(bind=eng, **kw)