From ff21c99b3c9c70ec5fefb8ac8bf3be87dcf64c0f Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 11 Jul 2023 12:01:26 -0700 Subject: [PATCH 001/159] Impelement get_homogeneous_pages. Unit tests forthcoming once we're directionally aligned. --- sqlakeyset/paging.py | 85 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index c1ed1d33..6850f883 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -3,6 +3,7 @@ from __future__ import annotations from functools import partial +from dataclasses import dataclass from typing import ( Any, List, @@ -438,3 +439,87 @@ def get_page( place, backwards = process_args(after, before, page) return orm_get_page(query, per_page, place, backwards) + + +# Do we need to support python 3.6? +@dataclass +class PageRequest(Generic[_TP]): + """See ``get_page()`` documentation for parameter explanations.""" + query: Query[_TP] + per_page: int = PER_PAGE_DEFAULT + after: OptionalKeyset = None + before: OptionalKeyset = None + page: Optional[Union[MarkerLike, str]] = None + + +def get_homogeneous_page(queries: list[PageRequest[_TP]]) -> list[Page[Row[_TP]]]: + """Get multiple pages of results for homogeneous legacy ORM queries. + + This only involves a single round trip to the database. To do that, under the + hood it generates a UNION ALL. That means each query must select exactly the + same columns. They may have different filters or ordering, but must result in + selecting the same columns with the same names. + + Resulting pages are returned in the same order as the original page requests. + """ + if not queries: + return [] + + paging_queries = [_prepare_homogeneous_page(query, i) for i, query in enumerate(queries)] + + query = paging_queries[0].query.query + query = query.union_all(*[q.query.query for q in paging_queries[1:]]) + + results = query.all() + + # We need to make sure there's an entry for every page in case some return + # empty. + page_to_rows = {i: list() for i in range(len(queries))} + for row in results: + page_to_rows[row._page_identifier].append(row) + + pages = [] + for i in range(len(queries)): + rows = page_to_rows[i] + pages.append(paging_queries[i].page_from_rows(rows)) + + +@dataclass +class _HomogeneousPagingQuery: + query: _PagingQuery + page_from_rows: Callable[[list[Row[_TP]]], Page[Row[_TP]]] + + +def _prepare_homogeneous_page( + query: PageRequest[_TP], page_identifier: int +) -> _HomogeneousPagingQuery: + """page_identifier MUST be a trusted int, as it is not escaped.""" + # Should have no effect if called correctly, but let's be extra safe. + page_identifier = int(page_identifier) + place, backwards = process_args(query.after, query.before, query.page) + + # Grab result_type and keys before adding the _page_identifier so that + # it isn't included in the results. + result_type = orm_result_type(query.query) + keys = orm_query_keys(query.query) + query.query = query.query.add_columns( + sa.sql.expression.literal_column(str(page_identifier)).label("_page_identifier") + ) + + # Could we order by page identifier to do the page collation in the DB? + + paging_query = prepare_paging( + q=query.query, + per_page=query.per_page, + place=place, + backwards=backwards, + orm=True, + dialect=query.query.session.get_bind().dialect, + ) + + def page_from_rows(rows): + return orm_page_from_rows( + paging_query, rows, keys, result_type, per_page, backwards, current_place=place + ) + + return _HomogeneousPagingQuery(query=paging_query, page_from_rows=page_from_rows) From eff5d52b7a5c36477c7a9cda30a9776371757d7d Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 08:30:38 -0700 Subject: [PATCH 002/159] Check in unit test for multi-page lookup. --- sqlakeyset/paging.py | 47 +++++++++--------- tests/test_paging.py | 111 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 112 insertions(+), 46 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 6850f883..5b38e3e2 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -22,7 +22,7 @@ from sqlalchemy.engine.interfaces import Dialect from sqlalchemy.orm import Session from sqlalchemy.orm.query import Query -from sqlalchemy.sql.expression import ColumnElement +from sqlalchemy.sql.expression import ColumnElement, literal_column from sqlalchemy.sql.selectable import Select from .columns import OC, MappedOrderColumn, find_order_key, parse_ob_clause @@ -441,7 +441,6 @@ def get_page( return orm_get_page(query, per_page, place, backwards) -# Do we need to support python 3.6? @dataclass class PageRequest(Generic[_TP]): """See ``get_page()`` documentation for parameter explanations.""" @@ -452,7 +451,7 @@ class PageRequest(Generic[_TP]): page: Optional[Union[MarkerLike, str]] = None -def get_homogeneous_page(queries: list[PageRequest[_TP]]) -> list[Page[Row[_TP]]]: +def get_homogeneous_pages(requests: list[PageRequest[_TP]]) -> list[Page[Row[_TP]]]: """Get multiple pages of results for homogeneous legacy ORM queries. This only involves a single round trip to the database. To do that, under the @@ -462,59 +461,61 @@ def get_homogeneous_page(queries: list[PageRequest[_TP]]) -> list[Page[Row[_TP]] Resulting pages are returned in the same order as the original page requests. """ - if not queries: + if not requests: return [] - paging_queries = [_prepare_homogeneous_page(query, i) for i, query in enumerate(queries)] + prepared_queries = [_prepare_homogeneous_page(request, i) for i, query in enumerate(requests)] - query = paging_queries[0].query.query - query = query.union_all(*[q.query.query for q in paging_queries[1:]]) + query = prepared_queries[0].paging_query.query + query = query.union_all(*[p.prepared_query.query for p in prepared_queries[1:]]) results = query.all() # We need to make sure there's an entry for every page in case some return # empty. - page_to_rows = {i: list() for i in range(len(queries))} + page_to_rows = {i: list() for i in range(len(requests))} for row in results: page_to_rows[row._page_identifier].append(row) pages = [] - for i in range(len(queries)): + for i in range(len(requests)): rows = page_to_rows[i] - pages.append(paging_queries[i].page_from_rows(rows)) + pages.append(prepared_queries[i].page_from_rows(rows)) + return pages @dataclass -class _HomogeneousPagingQuery: - query: _PagingQuery +class _PreparedQuery: + paging_query: _PagingQuery page_from_rows: Callable[[list[Row[_TP]]], Page[Row[_TP]]] def _prepare_homogeneous_page( - query: PageRequest[_TP], page_identifier: int -) -> _HomogeneousPagingQuery: + request: PageRequest[_TP], page_identifier: int +) -> _PreparedQuery: """page_identifier MUST be a trusted int, as it is not escaped.""" # Should have no effect if called correctly, but let's be extra safe. page_identifier = int(page_identifier) - place, backwards = process_args(query.after, query.before, query.page) + place, backwards = process_args(request.after, request.before, request.page) # Grab result_type and keys before adding the _page_identifier so that # it isn't included in the results. - result_type = orm_result_type(query.query) - keys = orm_query_keys(query.query) - query.query = query.query.add_columns( - sa.sql.expression.literal_column(str(page_identifier)).label("_page_identifier") + query = request.query + result_type = orm_result_type(query) + keys = orm_query_keys(query) + query = query.add_columns( + literal_column(str(page_identifier)).label("_page_identifier") ) # Could we order by page identifier to do the page collation in the DB? paging_query = prepare_paging( - q=query.query, - per_page=query.per_page, + q=query, + per_page=request.per_page, place=place, backwards=backwards, orm=True, - dialect=query.query.session.get_bind().dialect, + dialect=query.session.get_bind().dialect, ) def page_from_rows(rows): @@ -522,4 +523,4 @@ def page_from_rows(rows): paging_query, rows, keys, result_type, per_page, backwards, current_place=place ) - return _HomogeneousPagingQuery(query=paging_query, page_from_rows=page_from_rows) + return _PreparedQuery(paging_query=paging_query, page_from_rows=page_from_rows) diff --git a/tests/test_paging.py b/tests/test_paging.py index 721224f5..f21dd350 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -14,10 +14,11 @@ valid sqlalchemy versions are 1.3.0, 1.4.0, 2.0.0.)""" import warnings from packaging import version +from typing import Any import pytest import sqlalchemy -from sqlalchemy.orm import sessionmaker, aliased, Bundle +from sqlalchemy.orm import sessionmaker, aliased, Bundle, Query from sqlalchemy import ( desc, func, @@ -25,12 +26,16 @@ from sqlakeyset import ( get_page, + get_homogeneous_pages, select_page, serialize_bookmark, unserialize_bookmark, InvalidPage, + PageRequest, ) from sqlakeyset.paging import process_args +from sqlakeyset.results import Page +from sqlakeyset.types import MarkerLike from conftest import ( Book, Author, @@ -49,6 +54,76 @@ warnings.simplefilter("error") +@dataclass +class _PageTracker: + query: Query + unpaged: list + gathered: list + backwards: bool + page: tuple[MarkerLike | str, bool] + page_with_paging: Page | None = None + + +def assert_paging_orm(page_with_paging, gathered, backwards, unpaged): + """Returns the next page, or None if no further pages.""" + paging = page_with_paging.paging + + assert paging.current == page + + if backwards: + gathered = page_with_paging + gathered + else: + gathered = gathered + page_with_paging + + if len(gathered) < len(unpaged): + # Ensure each page is the correct size + assert paging.has_further + assert len(page_with_paging) == per_page + else: + assert not paging.has_further + + if not page_with_paging: + assert not paging.has_further + assert paging.further == paging.current + assert paging.current_opposite == (None, not paging.backwards) + # Is this return None necessary or will paging.further just be None? + return None + + return paging.further + + +def check_multiple_paging_orm(qs): + page_trackers = [ + _PageTracker( + query=q, + gathered=[], + backwards=(i % 2 == 0), + page=(None, i % 2 == 0), + unpaged=q.all(), + ) + for q in qs + ] + while True: + for t in page_tackers: + t.page = unserialize_bookmark(serialize_bookmark(t.page)) + + page_requests = [ + PageRequest(query=t.query, per_page=i, page=t.page) for i, t in enumerate(page_trackers) + ] + pages_with_paging = get_homogeneous_pages(page_requests) + for p, t in zip(pages_with_paging, page_trackers): + page_trackers.page_with_paging = p + + for t in list(page_tackers): + page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged) + if page is None: + # Ensure union of pages is original q.all() + assert t.gathered == t.unpaged + page_trackers.remove(t) + + t.page = page + + def check_paging_orm(q): item_counts = range(1, 12) @@ -65,28 +140,8 @@ def check_paging_orm(q): page = unserialize_bookmark(serialized_page) page_with_paging = get_page(q, per_page=per_page, page=serialized_page) - paging = page_with_paging.paging - - assert paging.current == page - - if backwards: - gathered = page_with_paging + gathered - else: - gathered = gathered + page_with_paging - - page = paging.further - - if len(gathered) < len(unpaged): - # Ensure each page is the correct size - assert paging.has_further - assert len(page_with_paging) == per_page - else: - assert not paging.has_further - - if not page_with_paging: - assert not paging.has_further - assert paging.further == paging.current - assert paging.current_opposite == (None, not paging.backwards) + page = assert_paging_orm(page_with_paging, gathered, backwards, unpaged) + if page is None: break # Ensure union of pages is original q.all() @@ -361,6 +416,16 @@ def test_orm_joined_inheritance(joined_inheritance_dburl): check_paging_orm(q=q) +def test_orm_multiple_pages(dburl): + with S(dburl, echo=ECHO) as s: + qs = [ + s.query(Animal).order_by(Animal.leg_count, Animal.id), + s.query(Animal).filter(Animal.leg_count == 4).order_by(Animal.id), + s.query(Animal).order_by(Animal.leg_count, Animal.id.desc()), + ] + check_multiple_paging_orm(qs=qs) + + def test_core(dburl): selectable = ( select(Book.b, Book.d, Book.id, Book.c) From d37132bdc8ddc2e51d4d47cc44b9c52eed3e5da2 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 08:34:44 -0700 Subject: [PATCH 003/159] Fix missing symbols. --- sqlakeyset/paging.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 5b38e3e2..14f71ba2 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -6,6 +6,8 @@ from dataclasses import dataclass from typing import ( Any, + Callable, + Generic, List, NamedTuple, Optional, @@ -449,7 +451,7 @@ class PageRequest(Generic[_TP]): after: OptionalKeyset = None before: OptionalKeyset = None page: Optional[Union[MarkerLike, str]] = None - + def get_homogeneous_pages(requests: list[PageRequest[_TP]]) -> list[Page[Row[_TP]]]: """Get multiple pages of results for homogeneous legacy ORM queries. @@ -464,7 +466,7 @@ def get_homogeneous_pages(requests: list[PageRequest[_TP]]) -> list[Page[Row[_TP if not requests: return [] - prepared_queries = [_prepare_homogeneous_page(request, i) for i, query in enumerate(requests)] + prepared_queries = [_prepare_homogeneous_page(request, i) for i, request in enumerate(requests)] query = prepared_queries[0].paging_query.query query = query.union_all(*[p.prepared_query.query for p in prepared_queries[1:]]) @@ -520,7 +522,7 @@ def _prepare_homogeneous_page( def page_from_rows(rows): return orm_page_from_rows( - paging_query, rows, keys, result_type, per_page, backwards, current_place=place + paging_query, rows, keys, result_type, request.per_page, backwards, current_place=place ) return _PreparedQuery(paging_query=paging_query, page_from_rows=page_from_rows) From df7466cdb3019526af8d7f48990118a1183d7353 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 09:39:22 -0700 Subject: [PATCH 004/159] Fix flake8 test_paging.py --- tests/test_paging.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index f21dd350..b6295586 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -13,8 +13,8 @@ docker containers. (Available python versions are 3.7, 3.8, 3.9, 3.10, 3.11 and valid sqlalchemy versions are 1.3.0, 1.4.0, 2.0.0.)""" import warnings +from dataclasses import dataclass from packaging import version -from typing import Any import pytest import sqlalchemy @@ -64,11 +64,11 @@ class _PageTracker: page_with_paging: Page | None = None -def assert_paging_orm(page_with_paging, gathered, backwards, unpaged): +def assert_paging_orm(page_with_paging, gathered, backwards, unpaged, per_page): """Returns the next page, or None if no further pages.""" paging = page_with_paging.paging - assert paging.current == page + assert paging.current == page_with_paging.page if backwards: gathered = page_with_paging + gathered @@ -101,10 +101,10 @@ def check_multiple_paging_orm(qs): page=(None, i % 2 == 0), unpaged=q.all(), ) - for q in qs + for i, q in enumerate(qs) ] while True: - for t in page_tackers: + for t in page_trackers: t.page = unserialize_bookmark(serialize_bookmark(t.page)) page_requests = [ @@ -114,8 +114,8 @@ def check_multiple_paging_orm(qs): for p, t in zip(pages_with_paging, page_trackers): page_trackers.page_with_paging = p - for t in list(page_tackers): - page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged) + for i, t in enumerate(list(page_trackers)): + page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, i) if page is None: # Ensure union of pages is original q.all() assert t.gathered == t.unpaged @@ -140,7 +140,7 @@ def check_paging_orm(q): page = unserialize_bookmark(serialized_page) page_with_paging = get_page(q, per_page=per_page, page=serialized_page) - page = assert_paging_orm(page_with_paging, gathered, backwards, unpaged) + page = assert_paging_orm(page_with_paging, gathered, backwards, unpaged, per_page) if page is None: break From 4843e68e8e623f594758d6c279ee344c84f05db8 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:05:35 -0700 Subject: [PATCH 005/159] Export get_homogeneous_pages. --- sqlakeyset/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sqlakeyset/__init__.py b/sqlakeyset/__init__.py index 322327c0..7d054cf2 100644 --- a/sqlakeyset/__init__.py +++ b/sqlakeyset/__init__.py @@ -16,6 +16,7 @@ __all__ = [ "get_page", + "get_homogeneous_pages", "select_page", "serialize_bookmark", "unserialize_bookmark", From 9985e4ed6423cdf18573c095cd4c91665b90d277 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:11:52 -0700 Subject: [PATCH 006/159] Actually import get_homogeneous_pages. --- sqlakeyset/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sqlakeyset/__init__.py b/sqlakeyset/__init__.py index 7d054cf2..63f55a6d 100644 --- a/sqlakeyset/__init__.py +++ b/sqlakeyset/__init__.py @@ -1,4 +1,4 @@ -from .paging import get_page, select_page, InvalidPage +from .paging import get_homogeneous_pages, get_page, select_page, InvalidPage from .results import ( Page, Paging, @@ -15,8 +15,8 @@ from .types import Keyset, Marker __all__ = [ - "get_page", "get_homogeneous_pages", + "get_page", "select_page", "serialize_bookmark", "unserialize_bookmark", From bd486e10a823f06b5a428b3f774cf5d9e29e8d81 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:14:05 -0700 Subject: [PATCH 007/159] Add PageRequest. --- sqlakeyset/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/__init__.py b/sqlakeyset/__init__.py index 63f55a6d..6b286d8d 100644 --- a/sqlakeyset/__init__.py +++ b/sqlakeyset/__init__.py @@ -1,4 +1,4 @@ -from .paging import get_homogeneous_pages, get_page, select_page, InvalidPage +from .paging import get_homogeneous_pages, get_page, select_page, InvalidPage, PageRequest from .results import ( Page, Paging, @@ -21,6 +21,7 @@ "serialize_bookmark", "unserialize_bookmark", "Page", + "PageRequest", "Paging", "Keyset", "Marker", From e53424376c6cecf00fb333c51f78b39ab0975583 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:17:32 -0700 Subject: [PATCH 008/159] Fix bad symbol. --- tests/test_paging.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index b6295586..6230042c 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -64,11 +64,11 @@ class _PageTracker: page_with_paging: Page | None = None -def assert_paging_orm(page_with_paging, gathered, backwards, unpaged, per_page): +def assert_paging_orm(page_with_paging, gathered, backwards, unpaged, page, per_page): """Returns the next page, or None if no further pages.""" paging = page_with_paging.paging - assert paging.current == page_with_paging.page + assert paging.current == page if backwards: gathered = page_with_paging + gathered @@ -115,7 +115,7 @@ def check_multiple_paging_orm(qs): page_trackers.page_with_paging = p for i, t in enumerate(list(page_trackers)): - page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, i) + page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, t.page, i) if page is None: # Ensure union of pages is original q.all() assert t.gathered == t.unpaged @@ -140,7 +140,7 @@ def check_paging_orm(q): page = unserialize_bookmark(serialized_page) page_with_paging = get_page(q, per_page=per_page, page=serialized_page) - page = assert_paging_orm(page_with_paging, gathered, backwards, unpaged, per_page) + page = assert_paging_orm(page_with_paging, gathered, backwards, unpaged, page, per_page) if page is None: break From 5e6fd6a34d55c5144a89fd812e690cff49e8bec6 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:22:09 -0700 Subject: [PATCH 009/159] Extend gathered. --- tests/test_paging.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 6230042c..9ecf0bf0 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -65,15 +65,18 @@ class _PageTracker: def assert_paging_orm(page_with_paging, gathered, backwards, unpaged, page, per_page): - """Returns the next page, or None if no further pages.""" + """Returns the next page, or None if no further pages. + + Modifies gathered in place. + """ paging = page_with_paging.paging assert paging.current == page if backwards: - gathered = page_with_paging + gathered + gathered[:0] = page_with_paging + gathered else: - gathered = gathered + page_with_paging + gathered.extend(page_with_paging) if len(gathered) < len(unpaged): # Ensure each page is the correct size From f5075c3418bb3c6f26d70d0304e55799224ea21a Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:24:55 -0700 Subject: [PATCH 010/159] Use Union instead of |. --- tests/test_paging.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 9ecf0bf0..f3495010 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -15,6 +15,7 @@ import warnings from dataclasses import dataclass from packaging import version +from typing import Union import pytest import sqlalchemy @@ -60,7 +61,7 @@ class _PageTracker: unpaged: list gathered: list backwards: bool - page: tuple[MarkerLike | str, bool] + page: tuple[Union[MarkerLike, str], bool] page_with_paging: Page | None = None @@ -74,7 +75,7 @@ def assert_paging_orm(page_with_paging, gathered, backwards, unpaged, page, per_ assert paging.current == page if backwards: - gathered[:0] = page_with_paging + gathered + gathered[:0] = page_with_paging else: gathered.extend(page_with_paging) From 9aae97fcc14503a8832e82fa0e07a22da98cf4ba Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:26:41 -0700 Subject: [PATCH 011/159] Use Tuple instead of tuple. --- tests/test_paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index f3495010..9aebd226 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -15,7 +15,7 @@ import warnings from dataclasses import dataclass from packaging import version -from typing import Union +from typing import Tuple, Union import pytest import sqlalchemy @@ -61,7 +61,7 @@ class _PageTracker: unpaged: list gathered: list backwards: bool - page: tuple[Union[MarkerLike, str], bool] + page: Tuple[Union[MarkerLike, str], bool] page_with_paging: Page | None = None From cbefa301e09ada21dd97516f7ed50b1c9b4b6c76 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:31:45 -0700 Subject: [PATCH 012/159] Use Book instead of Animal. --- tests/test_paging.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 9aebd226..4fad4629 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -423,9 +423,9 @@ def test_orm_joined_inheritance(joined_inheritance_dburl): def test_orm_multiple_pages(dburl): with S(dburl, echo=ECHO) as s: qs = [ - s.query(Animal).order_by(Animal.leg_count, Animal.id), - s.query(Animal).filter(Animal.leg_count == 4).order_by(Animal.id), - s.query(Animal).order_by(Animal.leg_count, Animal.id.desc()), + s.query(Book).order_by(Book.author_id, Book.id), + s.query(Book).filter(Book.author_id < 4).order_by(Book.id), + s.query(Book).order_by(Book.author_id, Book.id.desc()), ] check_multiple_paging_orm(qs=qs) From bb86b7e5c85567890ca7f1e734a8b40263e84a7b Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:34:20 -0700 Subject: [PATCH 013/159] More python 3.7 and fix wrong symbol name. --- sqlakeyset/paging.py | 2 +- tests/test_paging.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 14f71ba2..952f407b 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -469,7 +469,7 @@ def get_homogeneous_pages(requests: list[PageRequest[_TP]]) -> list[Page[Row[_TP prepared_queries = [_prepare_homogeneous_page(request, i) for i, request in enumerate(requests)] query = prepared_queries[0].paging_query.query - query = query.union_all(*[p.prepared_query.query for p in prepared_queries[1:]]) + query = query.union_all(*[p.paging_query.query for p in prepared_queries[1:]]) results = query.all() diff --git a/tests/test_paging.py b/tests/test_paging.py index 4fad4629..6399f46c 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -15,7 +15,7 @@ import warnings from dataclasses import dataclass from packaging import version -from typing import Tuple, Union +from typing import Optional, Tuple, Union import pytest import sqlalchemy @@ -62,7 +62,7 @@ class _PageTracker: gathered: list backwards: bool page: Tuple[Union[MarkerLike, str], bool] - page_with_paging: Page | None = None + page_with_paging: Optional[Page] = None def assert_paging_orm(page_with_paging, gathered, backwards, unpaged, page, per_page): From cdd0fc23c4231b904492432d8eec82d6a651232b Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:38:20 -0700 Subject: [PATCH 014/159] Improve tests. --- tests/test_paging.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 6399f46c..350fff95 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -424,10 +424,23 @@ def test_orm_multiple_pages(dburl): with S(dburl, echo=ECHO) as s: qs = [ s.query(Book).order_by(Book.author_id, Book.id), - s.query(Book).filter(Book.author_id < 4).order_by(Book.id), + s.query(Book).filter(Book.author_id == 1).order_by(Book.id), s.query(Book).order_by(Book.author_id, Book.id.desc()), ] - check_multiple_paging_orm(qs=qs) + check_multiple_paging_orm(qs=qs) + + +def test_orm_multiple_pages_one_query(dburl): + with S(dburl, echo=ECHO) as s: + qs = [ + s.query(Book).order_by(Book.id), + ] + check_multiple_paging_orm(qs=qs) + + +def test_orm_multiple_pages_empty_queries(dburl): + with S(dburl, echo=ECHO) as s: + assert get_homogeneous_pages([]) == [] def test_core(dburl): From 2af1ca2a6181e446c5bfe5e4c37b12d250476eb1 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:40:45 -0700 Subject: [PATCH 015/159] Remove unused db session. --- tests/test_paging.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 350fff95..f68991ff 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -439,8 +439,7 @@ def test_orm_multiple_pages_one_query(dburl): def test_orm_multiple_pages_empty_queries(dburl): - with S(dburl, echo=ECHO) as s: - assert get_homogeneous_pages([]) == [] + assert get_homogeneous_pages([]) == [] def test_core(dburl): From 76c2939d9312ec4498741c52ccd5ac0a3bd2823e Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:52:36 -0700 Subject: [PATCH 016/159] Exclude sqlite. --- sqlakeyset/paging.py | 3 +++ tests/conftest.py | 1 + tests/test_paging.py | 6 +++--- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 952f407b..eb158cb7 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -461,6 +461,9 @@ def get_homogeneous_pages(requests: list[PageRequest[_TP]]) -> list[Page[Row[_TP same columns. They may have different filters or ordering, but must result in selecting the same columns with the same names. + Note: This requires the underlying database to support ORDER BY and LIMIT + statements in components of a compound select, which SQLite does not. + Resulting pages are returned in the same order as the original page requests. """ if not requests: diff --git a/tests/conftest.py b/tests/conftest.py index 37421080..4d264386 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -246,6 +246,7 @@ def _dburl(request): dburl = pytest.fixture(params=SUPPORTED_ENGINES)(_dburl) no_mysql_dburl = pytest.fixture(params=["sqlite", "postgresql"])(_dburl) +no_sqlite_dburl = pytest.fixture(params=["mysql", "postgresql"])(_dburl) pg_only_dburl = pytest.fixture(params=["postgresql"])(_dburl) diff --git a/tests/test_paging.py b/tests/test_paging.py index f68991ff..8da4e5e6 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -420,7 +420,7 @@ def test_orm_joined_inheritance(joined_inheritance_dburl): check_paging_orm(q=q) -def test_orm_multiple_pages(dburl): +def test_orm_multiple_pages(no_sqlite_url): with S(dburl, echo=ECHO) as s: qs = [ s.query(Book).order_by(Book.author_id, Book.id), @@ -430,7 +430,7 @@ def test_orm_multiple_pages(dburl): check_multiple_paging_orm(qs=qs) -def test_orm_multiple_pages_one_query(dburl): +def test_orm_multiple_pages_one_query(no_sqlite_url): with S(dburl, echo=ECHO) as s: qs = [ s.query(Book).order_by(Book.id), @@ -438,7 +438,7 @@ def test_orm_multiple_pages_one_query(dburl): check_multiple_paging_orm(qs=qs) -def test_orm_multiple_pages_empty_queries(dburl): +def test_orm_multiple_pages_empty_queries(): assert get_homogeneous_pages([]) == [] From 6867d838d8b9311b9a47a63873377e6552137c9e Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:54:52 -0700 Subject: [PATCH 017/159] s/dburl/no_sqlite_url --- tests/test_paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 8da4e5e6..0764cbcd 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -421,7 +421,7 @@ def test_orm_joined_inheritance(joined_inheritance_dburl): def test_orm_multiple_pages(no_sqlite_url): - with S(dburl, echo=ECHO) as s: + with S(no_sqlite_url, echo=ECHO) as s: qs = [ s.query(Book).order_by(Book.author_id, Book.id), s.query(Book).filter(Book.author_id == 1).order_by(Book.id), @@ -431,7 +431,7 @@ def test_orm_multiple_pages(no_sqlite_url): def test_orm_multiple_pages_one_query(no_sqlite_url): - with S(dburl, echo=ECHO) as s: + with S(no_sqlite_url, echo=ECHO) as s: qs = [ s.query(Book).order_by(Book.id), ] From 19ff0099048f5cf066d3c87e47bbd99efd54f4b7 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 15:57:43 -0700 Subject: [PATCH 018/159] s/url/dburl --- tests/test_paging.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 0764cbcd..5432c95a 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -420,8 +420,8 @@ def test_orm_joined_inheritance(joined_inheritance_dburl): check_paging_orm(q=q) -def test_orm_multiple_pages(no_sqlite_url): - with S(no_sqlite_url, echo=ECHO) as s: +def test_orm_multiple_pages(no_sqlite_dburl): + with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ s.query(Book).order_by(Book.author_id, Book.id), s.query(Book).filter(Book.author_id == 1).order_by(Book.id), @@ -430,8 +430,8 @@ def test_orm_multiple_pages(no_sqlite_url): check_multiple_paging_orm(qs=qs) -def test_orm_multiple_pages_one_query(no_sqlite_url): - with S(no_sqlite_url, echo=ECHO) as s: +def test_orm_multiple_pages_one_query(no_sqlite_dburl): + with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ s.query(Book).order_by(Book.id), ] From abdef8af139c050d8d7212b63a800878edcb6029 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 13 Jul 2023 16:01:12 -0700 Subject: [PATCH 019/159] Fix for loop variable. --- tests/test_paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 5432c95a..6bb80f8f 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -116,7 +116,7 @@ def check_multiple_paging_orm(qs): ] pages_with_paging = get_homogeneous_pages(page_requests) for p, t in zip(pages_with_paging, page_trackers): - page_trackers.page_with_paging = p + t.page_with_paging = p for i, t in enumerate(list(page_trackers)): page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, t.page, i) From a5514cfd8195fcd6eb2ef03c44e1256fcdf4f13e Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 06:32:33 -0700 Subject: [PATCH 020/159] Per page should be at least 1. --- tests/test_paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 6bb80f8f..ef582e4e 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -112,14 +112,14 @@ def check_multiple_paging_orm(qs): t.page = unserialize_bookmark(serialize_bookmark(t.page)) page_requests = [ - PageRequest(query=t.query, per_page=i, page=t.page) for i, t in enumerate(page_trackers) + PageRequest(query=t.query, per_page=i + 1, page=t.page) for i, t in enumerate(page_trackers) ] pages_with_paging = get_homogeneous_pages(page_requests) for p, t in zip(pages_with_paging, page_trackers): t.page_with_paging = p for i, t in enumerate(list(page_trackers)): - page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, t.page, i) + page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, t.page, i + 1) if page is None: # Ensure union of pages is original q.all() assert t.gathered == t.unpaged From b7a34f5d764cc7ada3d2c78393baf012dc76c4be Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 06:57:09 -0700 Subject: [PATCH 021/159] Fix UNION ALL ordering which isn't guaranteed. --- sqlakeyset/paging.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index eb158cb7..6a4481c0 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -19,7 +19,7 @@ ) from typing_extensions import Literal # to keep python 3.7 support -from sqlalchemy import tuple_, and_, or_ +from sqlalchemy import tuple_, and_, or_, func, text from sqlalchemy.engine import Connection from sqlalchemy.engine.interfaces import Dialect from sqlalchemy.orm import Session @@ -472,8 +472,9 @@ def get_homogeneous_pages(requests: list[PageRequest[_TP]]) -> list[Page[Row[_TP prepared_queries = [_prepare_homogeneous_page(request, i) for i, request in enumerate(requests)] query = prepared_queries[0].paging_query.query - query = query.union_all(*[p.paging_query.query for p in prepared_queries[1:]]) + query = query.union_all(*[p.paging_query.query for p in prepared_queries[1:]]).order_by(text("_page_identifier"), text("_row_number")) + print(query) results = query.all() # We need to make sure there's an entry for every page in case some return @@ -508,9 +509,6 @@ def _prepare_homogeneous_page( query = request.query result_type = orm_result_type(query) keys = orm_query_keys(query) - query = query.add_columns( - literal_column(str(page_identifier)).label("_page_identifier") - ) # Could we order by page identifier to do the page collation in the DB? @@ -522,6 +520,13 @@ def _prepare_homogeneous_page( orm=True, dialect=query.session.get_bind().dialect, ) + query = paging_query.query + paging_query.query = paging_query.query.add_columns( + literal_column(str(page_identifier)).label("_page_identifier"), + func.ROW_NUMBER().over( + order_by=[c.element for c in paging_query.order_columns] + ).label("_row_number") + ) def page_from_rows(rows): return orm_page_from_rows( From 2f4f4bd92bacac07a1dc82c08728d428dbf3fa3f Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 07:02:28 -0700 Subject: [PATCH 022/159] NamedTuple is immutable. --- sqlakeyset/paging.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 6a4481c0..f08d7570 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -510,6 +510,14 @@ def _prepare_homogeneous_page( result_type = orm_result_type(query) keys = orm_query_keys(query) + # This is unfortunately duplicated with prepare_paging, but we need + """ + selectable = orm_to_selectable(query) + order_cols = parse_ob_clause(selectable) + if backwards: + order_cols = [c.reversed for c in order_cols] + """ + # Could we order by page identifier to do the page collation in the DB? paging_query = prepare_paging( @@ -521,12 +529,13 @@ def _prepare_homogeneous_page( dialect=query.session.get_bind().dialect, ) query = paging_query.query - paging_query.query = paging_query.query.add_columns( + query = query.add_columns( literal_column(str(page_identifier)).label("_page_identifier"), func.ROW_NUMBER().over( order_by=[c.element for c in paging_query.order_columns] ).label("_row_number") ) + paging_query = _PagingQuery(query, *paging_query[1:]) def page_from_rows(rows): return orm_page_from_rows( From 3909554c4671e580818342a18c8a22688edb4bca Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 07:05:26 -0700 Subject: [PATCH 023/159] Trailing whitespace. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index f08d7570..38a7df24 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -510,7 +510,7 @@ def _prepare_homogeneous_page( result_type = orm_result_type(query) keys = orm_query_keys(query) - # This is unfortunately duplicated with prepare_paging, but we need + # This is unfortunately duplicated with prepare_paging, but we need """ selectable = orm_to_selectable(query) order_cols = parse_ob_clause(selectable) From 3393641d12aca68542d8202cd2c1bb92c1e0a54f Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 07:16:03 -0700 Subject: [PATCH 024/159] Use uo instead of element to get ordering right for ROW_NUMBER. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 38a7df24..f3f08c35 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -532,7 +532,7 @@ def _prepare_homogeneous_page( query = query.add_columns( literal_column(str(page_identifier)).label("_page_identifier"), func.ROW_NUMBER().over( - order_by=[c.element for c in paging_query.order_columns] + order_by=[c.uo for c in paging_query.order_columns] ).label("_row_number") ) paging_query = _PagingQuery(query, *paging_query[1:]) From f985063a662878547322f5375c4e5578be3da41c Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 07:24:48 -0700 Subject: [PATCH 025/159] Use deque for extendleft functionality. --- tests/test_paging.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index ef582e4e..27702734 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -13,6 +13,7 @@ docker containers. (Available python versions are 3.7, 3.8, 3.9, 3.10, 3.11 and valid sqlalchemy versions are 1.3.0, 1.4.0, 2.0.0.)""" import warnings +from collections import deque from dataclasses import dataclass from packaging import version from typing import Optional, Tuple, Union @@ -59,7 +60,7 @@ class _PageTracker: query: Query unpaged: list - gathered: list + gathered: deque backwards: bool page: Tuple[Union[MarkerLike, str], bool] page_with_paging: Optional[Page] = None @@ -75,7 +76,7 @@ def assert_paging_orm(page_with_paging, gathered, backwards, unpaged, page, per_ assert paging.current == page if backwards: - gathered[:0] = page_with_paging + gathered.extendleft(reversed(page_with_paging)) else: gathered.extend(page_with_paging) @@ -100,7 +101,7 @@ def check_multiple_paging_orm(qs): page_trackers = [ _PageTracker( query=q, - gathered=[], + gathered=deque(), backwards=(i % 2 == 0), page=(None, i % 2 == 0), unpaged=q.all(), @@ -122,7 +123,7 @@ def check_multiple_paging_orm(qs): page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, t.page, i + 1) if page is None: # Ensure union of pages is original q.all() - assert t.gathered == t.unpaged + assert list(t.gathered) == t.unpaged page_trackers.remove(t) t.page = page @@ -135,7 +136,7 @@ def check_paging_orm(q): for backwards in [False, True]: for per_page in item_counts: - gathered = [] + gathered = deque() page = None, backwards @@ -149,7 +150,7 @@ def check_paging_orm(q): break # Ensure union of pages is original q.all() - assert gathered == unpaged + assert list(gathered) == unpaged def check_paging_core(selectable, s): From e5feac2c5f9e014b4adcb58b1e607e34513de0ff Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 07:30:07 -0700 Subject: [PATCH 026/159] Debugging info. --- sqlakeyset/paging.py | 1 - tests/test_paging.py | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index f3f08c35..0f211f42 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -474,7 +474,6 @@ def get_homogeneous_pages(requests: list[PageRequest[_TP]]) -> list[Page[Row[_TP query = prepared_queries[0].paging_query.query query = query.union_all(*[p.paging_query.query for p in prepared_queries[1:]]).order_by(text("_page_identifier"), text("_row_number")) - print(query) results = query.all() # We need to make sure there's an entry for every page in case some return diff --git a/tests/test_paging.py b/tests/test_paging.py index 27702734..a0e440f3 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -81,9 +81,10 @@ def assert_paging_orm(page_with_paging, gathered, backwards, unpaged, page, per_ gathered.extend(page_with_paging) if len(gathered) < len(unpaged): + print(f"{len(gathered)} < {len(unpaged)}") # Ensure each page is the correct size - assert paging.has_further assert len(page_with_paging) == per_page + assert paging.has_further else: assert not paging.has_further From 46248d252f7a1da5d0c9438dfad11aab2ffede20 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 07:43:04 -0700 Subject: [PATCH 027/159] Testing. --- tests/test_paging.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index a0e440f3..c76e7080 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -114,14 +114,15 @@ def check_multiple_paging_orm(qs): t.page = unserialize_bookmark(serialize_bookmark(t.page)) page_requests = [ - PageRequest(query=t.query, per_page=i + 1, page=t.page) for i, t in enumerate(page_trackers) + PageRequest(query=t.query, per_page=i + 2, page=t.page) for i, t in enumerate(page_trackers) ] pages_with_paging = get_homogeneous_pages(page_requests) for p, t in zip(pages_with_paging, page_trackers): t.page_with_paging = p + print(p) for i, t in enumerate(list(page_trackers)): - page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, t.page, i + 1) + page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, t.page, i + 2) if page is None: # Ensure union of pages is original q.all() assert list(t.gathered) == t.unpaged From c5a606e065a7b1cb6952a9cbd81e8a410f904692 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 07:51:12 -0700 Subject: [PATCH 028/159] Add before preparing paging. --- sqlakeyset/paging.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 0f211f42..17ccf83f 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -510,13 +510,17 @@ def _prepare_homogeneous_page( keys = orm_query_keys(query) # This is unfortunately duplicated with prepare_paging, but we need - """ selectable = orm_to_selectable(query) order_cols = parse_ob_clause(selectable) if backwards: order_cols = [c.reversed for c in order_cols] - """ + query = query.add_columns( + literal_column(str(page_identifier)).label("_page_identifier"), + func.ROW_NUMBER().over( + order_by=[c.uo for c in order_cols] + ).label("_row_number") + ) # Could we order by page identifier to do the page collation in the DB? paging_query = prepare_paging( @@ -528,12 +532,6 @@ def _prepare_homogeneous_page( dialect=query.session.get_bind().dialect, ) query = paging_query.query - query = query.add_columns( - literal_column(str(page_identifier)).label("_page_identifier"), - func.ROW_NUMBER().over( - order_by=[c.uo for c in paging_query.order_columns] - ).label("_row_number") - ) paging_query = _PagingQuery(query, *paging_query[1:]) def page_from_rows(rows): From 3ccdd3e1563984faed0a68e0d984843482496d24 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 07:54:33 -0700 Subject: [PATCH 029/159] author_id is nullable which doesn't work with paging. --- tests/test_paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index c76e7080..fea4bd87 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -426,9 +426,9 @@ def test_orm_joined_inheritance(joined_inheritance_dburl): def test_orm_multiple_pages(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ - s.query(Book).order_by(Book.author_id, Book.id), + s.query(Book).order_by(Book.name, Book.id), s.query(Book).filter(Book.author_id == 1).order_by(Book.id), - s.query(Book).order_by(Book.author_id, Book.id.desc()), + s.query(Book).order_by(Book.name, Book.id.desc()), ] check_multiple_paging_orm(qs=qs) From 0e7cc0bcfce403720017e93aef7d3f7e8271f71d Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 08:10:28 -0700 Subject: [PATCH 030/159] Actually end the test on success. --- tests/test_paging.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_paging.py b/tests/test_paging.py index fea4bd87..fbaca2d6 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -127,9 +127,13 @@ def check_multiple_paging_orm(qs): # Ensure union of pages is original q.all() assert list(t.gathered) == t.unpaged page_trackers.remove(t) + continue t.page = page + if not page_trackers: + break + def check_paging_orm(q): item_counts = range(1, 12) From 13789365d2a591c01b6c414f138d10f003726c1c Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 09:13:29 -0700 Subject: [PATCH 031/159] Add a test for fetching columns. --- sqlakeyset/paging.py | 6 ++++-- tests/test_paging.py | 16 ++++++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 17ccf83f..3cbbac5c 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -504,12 +504,15 @@ def _prepare_homogeneous_page( place, backwards = process_args(request.after, request.before, request.page) # Grab result_type and keys before adding the _page_identifier so that - # it isn't included in the results. + # it isn't included in the results. page_from_rows will slice the resulting + # row based on the length of keys query = request.query result_type = orm_result_type(query) keys = orm_query_keys(query) # This is unfortunately duplicated with prepare_paging, but we need + # to get it to add the columns *before* we do the other query wrangling + # inside prepare_paging. selectable = orm_to_selectable(query) order_cols = parse_ob_clause(selectable) if backwards: @@ -521,7 +524,6 @@ def _prepare_homogeneous_page( order_by=[c.uo for c in order_cols] ).label("_row_number") ) - # Could we order by page identifier to do the page collation in the DB? paging_query = prepare_paging( q=query, diff --git a/tests/test_paging.py b/tests/test_paging.py index fbaca2d6..1021adf6 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -81,7 +81,6 @@ def assert_paging_orm(page_with_paging, gathered, backwards, unpaged, page, per_ gathered.extend(page_with_paging) if len(gathered) < len(unpaged): - print(f"{len(gathered)} < {len(unpaged)}") # Ensure each page is the correct size assert len(page_with_paging) == per_page assert paging.has_further @@ -114,15 +113,14 @@ def check_multiple_paging_orm(qs): t.page = unserialize_bookmark(serialize_bookmark(t.page)) page_requests = [ - PageRequest(query=t.query, per_page=i + 2, page=t.page) for i, t in enumerate(page_trackers) + PageRequest(query=t.query, per_page=i + 1, page=t.page) for i, t in enumerate(page_trackers) ] pages_with_paging = get_homogeneous_pages(page_requests) for p, t in zip(pages_with_paging, page_trackers): t.page_with_paging = p - print(p) for i, t in enumerate(list(page_trackers)): - page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, t.page, i + 2) + page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, t.page, i + 1) if page is None: # Ensure union of pages is original q.all() assert list(t.gathered) == t.unpaged @@ -437,6 +435,16 @@ def test_orm_multiple_pages(no_sqlite_dburl): check_multiple_paging_orm(qs=qs) +def test_orm_multiple_pages_select_columns(no_sqlite_dburl): + with S(no_sqlite_dburl, echo=ECHO) as s: + qs = [ + s.query(Book).order_by(Book.name, Book.id), + s.query(Book.name, Book.id).filter(Book.author_id == 1).order_by(Book.id), + s.query(Book.name, Book.author_id, Book.id).order_by(Book.name, Book.id.desc()), + ] + check_multiple_paging_orm(qs=qs) + + def test_orm_multiple_pages_one_query(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ From b610c69435fafb7604fbb3d71834a47064409fbe Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 09:18:06 -0700 Subject: [PATCH 032/159] Make test homogeneous. --- tests/test_paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 1021adf6..5124d267 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -438,8 +438,8 @@ def test_orm_multiple_pages(no_sqlite_dburl): def test_orm_multiple_pages_select_columns(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ - s.query(Book).order_by(Book.name, Book.id), - s.query(Book.name, Book.id).filter(Book.author_id == 1).order_by(Book.id), + s.query(Book.name, Book.author_id, Book.id).order_by(Book.name, Book.id), + s.query(Book.name, Book.author_id, Book.id).filter(Book.author_id == 1).order_by(Book.id), s.query(Book.name, Book.author_id, Book.id).order_by(Book.name, Book.id.desc()), ] check_multiple_paging_orm(qs=qs) From d46719f8e754479d3885f7516dac938f7190c6eb Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 09:30:08 -0700 Subject: [PATCH 033/159] Move page_identifier inside prepare_paging. --- sqlakeyset/paging.py | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 3cbbac5c..4bebeecc 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -24,7 +24,7 @@ from sqlalchemy.engine.interfaces import Dialect from sqlalchemy.orm import Session from sqlalchemy.orm.query import Query -from sqlalchemy.sql.expression import ColumnElement, literal_column +from sqlalchemy.sql.expression import ColumnElement, literal from sqlalchemy.sql.selectable import Select from .columns import OC, MappedOrderColumn, find_order_key, parse_ob_clause @@ -155,6 +155,7 @@ def prepare_paging( backwards: bool, orm: Literal[True], dialect: Dialect, + page_identifier: Optional[int] = None, ) -> _PagingQuery: ... @@ -167,6 +168,7 @@ def prepare_paging( backwards: bool, orm: Literal[False], dialect: Dialect, + page_identifier: Optional[int] = None, ) -> _PagingSelect: ... @@ -178,6 +180,7 @@ def prepare_paging( backwards: bool, orm: bool, dialect: Dialect, + page_identifier: Optional[int] = None, ) -> Union[_PagingQuery, _PagingSelect]: if orm: if not isinstance(q, Query): @@ -206,6 +209,15 @@ def prepare_paging( extra_columns = [ col.extra_column for col in mapped_ocols if col.extra_column is not None ] + + # page_identifier is used for fetching multiple pages. + if page_identifier is not None: + extra_columns += [ + literal(page_identifier).label("_page_identifier"), + func.ROW_NUMBER().over( + order_by=[c.uo for c in order_cols] + ).label("_row_number"), + ] if hasattr(q, "add_columns"): # ORM or SQLAlchemy 1.4+ q = q.add_columns(*extra_columns) else: @@ -498,33 +510,12 @@ class _PreparedQuery: def _prepare_homogeneous_page( request: PageRequest[_TP], page_identifier: int ) -> _PreparedQuery: - """page_identifier MUST be a trusted int, as it is not escaped.""" - # Should have no effect if called correctly, but let's be extra safe. - page_identifier = int(page_identifier) place, backwards = process_args(request.after, request.before, request.page) - # Grab result_type and keys before adding the _page_identifier so that - # it isn't included in the results. page_from_rows will slice the resulting - # row based on the length of keys query = request.query result_type = orm_result_type(query) keys = orm_query_keys(query) - # This is unfortunately duplicated with prepare_paging, but we need - # to get it to add the columns *before* we do the other query wrangling - # inside prepare_paging. - selectable = orm_to_selectable(query) - order_cols = parse_ob_clause(selectable) - if backwards: - order_cols = [c.reversed for c in order_cols] - - query = query.add_columns( - literal_column(str(page_identifier)).label("_page_identifier"), - func.ROW_NUMBER().over( - order_by=[c.uo for c in order_cols] - ).label("_row_number") - ) - paging_query = prepare_paging( q=query, per_page=request.per_page, @@ -532,9 +523,8 @@ def _prepare_homogeneous_page( backwards=backwards, orm=True, dialect=query.session.get_bind().dialect, + page_identifier=page_identifier, ) - query = paging_query.query - paging_query = _PagingQuery(query, *paging_query[1:]) def page_from_rows(rows): return orm_page_from_rows( From bdac930c82ea4c4c09843b6160eb7ff448c0609d Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 11:09:11 -0700 Subject: [PATCH 034/159] Add select_homogeneous_pages and refactor tests. --- sqlakeyset/__init__.py | 2 +- sqlakeyset/paging.py | 107 +++++++++++++++++++++++++++++++++++++---- tests/test_paging.py | 43 ++++++++++------- 3 files changed, 123 insertions(+), 29 deletions(-) diff --git a/sqlakeyset/__init__.py b/sqlakeyset/__init__.py index 6b286d8d..4996f35e 100644 --- a/sqlakeyset/__init__.py +++ b/sqlakeyset/__init__.py @@ -1,4 +1,4 @@ -from .paging import get_homogeneous_pages, get_page, select_page, InvalidPage, PageRequest +from .paging import get_homogeneous_pages, get_page, select_homogeneous_pages, select_page, InvalidPage, OrmPageRequest, PageRequest from .results import ( Page, Paging, diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 4bebeecc..48da5c32 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -291,11 +291,13 @@ def core_get_page( :param backwards: If ``True``, reverse pagination direction. :returns: :class:`Page` """ - # We need the result schema for the *original* query in order to properly - # trim off our extra_columns. As far as I can tell, this is the only + # In SQLAlchemy 1.3, we need the result schema for the *original* query in order + # to properly trim off our extra_columns. As far as I can tell, this is the only # way to get it without copy-pasting chunks of the sqlalchemy internals. # LIMIT 0 to minimize database load (though the fact that a round trip to # the DB has to happen at all is regrettable). + # + # Thankfully this is obsolete in 1.4+ result_type = core_result_type(selectable, s) sel = prepare_paging( q=selectable, @@ -456,7 +458,7 @@ def get_page( @dataclass -class PageRequest(Generic[_TP]): +class OrmPageRequest(Generic[_TP]): """See ``get_page()`` documentation for parameter explanations.""" query: Query[_TP] per_page: int = PER_PAGE_DEFAULT @@ -465,7 +467,17 @@ class PageRequest(Generic[_TP]): page: Optional[Union[MarkerLike, str]] = None -def get_homogeneous_pages(requests: list[PageRequest[_TP]]) -> list[Page[Row[_TP]]]: +@dataclass +class PageRequest(Generic[_TP]): + """See ``select_page()`` documentation for parameter explanations.""" + selectable: Select[_TP] + per_page: int = PER_PAGE_DEFAULT + after: OptionalKeyset = None + before: OptionalKeyset = None + page: Optional[Union[MarkerLike, str]] = None + + +def get_homogeneous_pages(requests: list[OrmPageRequest[_TP]]) -> list[Page[Row[_TP]]]: """Get multiple pages of results for homogeneous legacy ORM queries. This only involves a single round trip to the database. To do that, under the @@ -481,10 +493,14 @@ def get_homogeneous_pages(requests: list[PageRequest[_TP]]) -> list[Page[Row[_TP if not requests: return [] - prepared_queries = [_prepare_homogeneous_page(request, i) for i, request in enumerate(requests)] + prepared_queries = [ + _orm_prepare_homogeneous_page(request, i) for i, request in enumerate(requests) + ] query = prepared_queries[0].paging_query.query - query = query.union_all(*[p.paging_query.query for p in prepared_queries[1:]]).order_by(text("_page_identifier"), text("_row_number")) + query = query.union_all( + *[p.paging_query.query for p in prepared_queries[1:]] + ).order_by(text("_page_identifier"), text("_row_number")) results = query.all() @@ -501,14 +517,85 @@ def get_homogeneous_pages(requests: list[PageRequest[_TP]]) -> list[Page[Row[_TP return pages +def select_homogeneous_pages( + requests: list[PageRequest[_TP]], s: Union[Session, Connection] +) -> list[Page[Row[_TP]]]: + """Get multiple pages of results for homogeneous legacy ORM queries. + + This only involves a single round trip to the database. To do that, under the + hood it generates a UNION ALL. That means each query must select exactly the + same columns. They may have different filters or ordering, but must result in + selecting the same columns with the same names. + + Note: This requires the underlying database to support ORDER BY and LIMIT + statements in components of a compound select, which SQLite does not. + + Resulting pages are returned in the same order as the original page requests. + """ + if not requests: + return [] + + prepared_queries = [_core_prepare_homogeneous_page(request, s, i) for i, request in enumerate(requests)] + + select = union_all(*[p.paging_query.select for p in prepared_queries]) + selected = s.execute(sel.select) + + results = query.fetchall() + + # We need to make sure there's an entry for every page in case some return + # empty. + page_to_rows = {i: list() for i in range(len(requests))} + for row in results: + page_to_rows[row._page_identifier].append(row) + + pages = [] + for i in range(len(requests)): + rows = page_to_rows[i] + pages.append(prepared_queries[i].page_from_rows(rows, selected)) + return pages + + @dataclass class _PreparedQuery: - paging_query: _PagingQuery - page_from_rows: Callable[[list[Row[_TP]]], Page[Row[_TP]]] + paging_query: Union[_PagingQuery, _PagingSelect] + page_from_rows: Callable[..., Page[Row[_TP]]] + + +def _core_prepare_homogeneous_page( + request: PageRequest[_TP], s: Union[Session, Connection], page_identifier: int +) -> _PreparedQuery: + place, backwards = process_args(request.after, request.before, request.page) + + selectable = request.selectable + result_type = core_result_type(selectable, s) + sel = prepare_paging( + q=selectable, + per_page=request.per_page, + place=place, + backwards=backwards, + orm=False, + dialect=get_bind(q=selectable, s=s).dialect, + ) + def page_from_rows(rows, selected): + keys = list(selected.keys()) + N = len(keys) - len(sel.extra_columns) + keys = keys[:N] + page = core_page_from_rows( + sel, + rows, + keys, + result_type, + request.per_page, + backwards, + current_place=place, + ) + return page + + return _PreparedQuery(paging_query=sel, page_from_rows=page_from_rows) -def _prepare_homogeneous_page( - request: PageRequest[_TP], page_identifier: int +def _orm_prepare_homogeneous_page( + request: OrmPageRequest[_TP], page_identifier: int ) -> _PreparedQuery: place, backwards = process_args(request.after, request.before, request.page) diff --git a/tests/test_paging.py b/tests/test_paging.py index 5124d267..3ca1ece3 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -33,6 +33,7 @@ serialize_bookmark, unserialize_bookmark, InvalidPage, + OrmPageRequest, PageRequest, ) from sqlakeyset.paging import process_args @@ -113,7 +114,7 @@ def check_multiple_paging_orm(qs): t.page = unserialize_bookmark(serialize_bookmark(t.page)) page_requests = [ - PageRequest(query=t.query, per_page=i + 1, page=t.page) for i, t in enumerate(page_trackers) + OrmPageRequest(query=t.query, per_page=i + 1, page=t.page) for i, t in enumerate(page_trackers) ] pages_with_paging = get_homogeneous_pages(page_requests) for p, t in zip(pages_with_paging, page_trackers): @@ -157,6 +158,26 @@ def check_paging_orm(q): assert list(gathered) == unpaged +def assert_paging_core(page_with_paging, gathered, backwards, unpaged, page, per_page): + paging = page_with_paging.paging + + assert paging.current == page + assert page_with_paging.keys() == result.keys() + + if backwards: + gathered.extendleft(reversed(page_with_paging)) + else: + gathered.extend(page_with_paging) + + if not page_with_paging: + assert not paging.has_further + assert paging.further == paging.current + assert paging.current_opposite == (None, not paging.backwards) + return None + + return paging.further + + def check_paging_core(selectable, s): item_counts = range(1, 12) @@ -165,7 +186,7 @@ def check_paging_core(selectable, s): for backwards in [False, True]: for per_page in item_counts: - gathered = [] + gathered = deque() page = None, backwards @@ -176,22 +197,8 @@ def check_paging_core(selectable, s): page_with_paging = select_page( s, selectable, per_page=per_page, page=serialized_page ) - paging = page_with_paging.paging - - assert paging.current == page - assert page_with_paging.keys() == result.keys() - - if backwards: - gathered = page_with_paging + gathered - else: - gathered = gathered + page_with_paging - - page = paging.further - - if not page_with_paging: - assert not paging.has_further - assert paging.further == paging.current - assert paging.current_opposite == (None, not paging.backwards) + page = assert_paging_core(page_with_paging, gathered, backwards, unpaged, page, per_page) + if page is None: break assert gathered == unpaged From 1127c0f187dd8e75bd8c50c8301b3c44c710a996 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 11:17:33 -0700 Subject: [PATCH 035/159] flake8 fixes. --- sqlakeyset/__init__.py | 12 +++++++++++- sqlakeyset/paging.py | 11 +++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/sqlakeyset/__init__.py b/sqlakeyset/__init__.py index 4996f35e..47737356 100644 --- a/sqlakeyset/__init__.py +++ b/sqlakeyset/__init__.py @@ -1,4 +1,12 @@ -from .paging import get_homogeneous_pages, get_page, select_homogeneous_pages, select_page, InvalidPage, OrmPageRequest, PageRequest +from .paging import ( + get_homogeneous_pages, + get_page, + select_homogeneous_pages, + select_page, + InvalidPage, + OrmPageRequest, + PageRequest, +) from .results import ( Page, Paging, @@ -17,11 +25,13 @@ __all__ = [ "get_homogeneous_pages", "get_page", + "select_homogeneous_pages", "select_page", "serialize_bookmark", "unserialize_bookmark", "Page", "PageRequest", + "OrmPageRequest", "Paging", "Keyset", "Marker", diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 48da5c32..4e61e854 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -24,7 +24,7 @@ from sqlalchemy.engine.interfaces import Dialect from sqlalchemy.orm import Session from sqlalchemy.orm.query import Query -from sqlalchemy.sql.expression import ColumnElement, literal +from sqlalchemy.sql.expression import ColumnElement, literal, union_all from sqlalchemy.sql.selectable import Select from .columns import OC, MappedOrderColumn, find_order_key, parse_ob_clause @@ -537,10 +537,12 @@ def select_homogeneous_pages( prepared_queries = [_core_prepare_homogeneous_page(request, s, i) for i, request in enumerate(requests)] - select = union_all(*[p.paging_query.select for p in prepared_queries]) - selected = s.execute(sel.select) + select = union_all( + *[p.paging_query.select for p in prepared_queries] + ).order_by(text("_page_identifier"), text("_row_number")) + selected = s.execute(select) - results = query.fetchall() + results = selected.fetchall() # We need to make sure there's an entry for every page in case some return # empty. @@ -576,6 +578,7 @@ def _core_prepare_homogeneous_page( orm=False, dialect=get_bind(q=selectable, s=s).dialect, ) + def page_from_rows(rows, selected): keys = list(selected.keys()) N = len(keys) - len(sel.extra_columns) From 24943ea7789b304bc99059b68815c29841bd4926 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 11:20:27 -0700 Subject: [PATCH 036/159] More flake8. --- tests/test_paging.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 3ca1ece3..86667a0a 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -34,7 +34,6 @@ unserialize_bookmark, InvalidPage, OrmPageRequest, - PageRequest, ) from sqlakeyset.paging import process_args from sqlakeyset.results import Page @@ -158,7 +157,7 @@ def check_paging_orm(q): assert list(gathered) == unpaged -def assert_paging_core(page_with_paging, gathered, backwards, unpaged, page, per_page): +def assert_paging_core(page_with_paging, gathered, backwards, result, page, per_page): paging = page_with_paging.paging assert paging.current == page @@ -197,7 +196,7 @@ def check_paging_core(selectable, s): page_with_paging = select_page( s, selectable, per_page=per_page, page=serialized_page ) - page = assert_paging_core(page_with_paging, gathered, backwards, unpaged, page, per_page) + page = assert_paging_core(page_with_paging, gathered, backwards, result, page, per_page) if page is None: break From 5bb4c5d7d34d046fee36bbaca70d6ba305404363 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 11:23:12 -0700 Subject: [PATCH 037/159] Convert deque to list. --- tests/test_paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 86667a0a..340cc654 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -200,7 +200,7 @@ def check_paging_core(selectable, s): if page is None: break - assert gathered == unpaged + assert list(gathered) == unpaged def test_orm_query1(dburl): From 29985c410eaaeeabb2abf375f9bbe71c0f8171ed Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 11:34:48 -0700 Subject: [PATCH 038/159] Test select_homogeneous_pages. --- tests/test_paging.py | 123 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 97 insertions(+), 26 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 340cc654..8472381a 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -16,7 +16,7 @@ from collections import deque from dataclasses import dataclass from packaging import version -from typing import Optional, Tuple, Union +from typing import Any, Optional, Tuple, Union import pytest import sqlalchemy @@ -25,6 +25,7 @@ desc, func, ) +from sqlalchemy.sql.selectable import Select from sqlakeyset import ( get_page, @@ -58,8 +59,9 @@ @dataclass class _PageTracker: - query: Query + query: Union[Query, Select] unpaged: list + selected: Any gathered: deque backwards: bool page: Tuple[Union[MarkerLike, str], bool] @@ -97,6 +99,30 @@ def assert_paging_orm(page_with_paging, gathered, backwards, unpaged, page, per_ return paging.further +def check_paging_orm(q): + item_counts = range(1, 12) + + unpaged = q.all() + + for backwards in [False, True]: + for per_page in item_counts: + gathered = deque() + + page = None, backwards + + while True: + serialized_page = serialize_bookmark(page) + page = unserialize_bookmark(serialized_page) + + page_with_paging = get_page(q, per_page=per_page, page=serialized_page) + page = assert_paging_orm(page_with_paging, gathered, backwards, unpaged, page, per_page) + if page is None: + break + + # Ensure union of pages is original q.all() + assert list(gathered) == unpaged + + def check_multiple_paging_orm(qs): page_trackers = [ _PageTracker( @@ -133,30 +159,6 @@ def check_multiple_paging_orm(qs): break -def check_paging_orm(q): - item_counts = range(1, 12) - - unpaged = q.all() - - for backwards in [False, True]: - for per_page in item_counts: - gathered = deque() - - page = None, backwards - - while True: - serialized_page = serialize_bookmark(page) - page = unserialize_bookmark(serialized_page) - - page_with_paging = get_page(q, per_page=per_page, page=serialized_page) - page = assert_paging_orm(page_with_paging, gathered, backwards, unpaged, page, per_page) - if page is None: - break - - # Ensure union of pages is original q.all() - assert list(gathered) == unpaged - - def assert_paging_core(page_with_paging, gathered, backwards, result, page, per_page): paging = page_with_paging.paging @@ -203,6 +205,43 @@ def check_paging_core(selectable, s): assert list(gathered) == unpaged +def check_multiple_paging_core(qs, s): + page_trackers = [ + _PageTracker( + query=q, + gathered=deque(), + backwards=(i % 2 == 0), + page=(None, i % 2 == 0), + selected=s.execute(q), + unpaged=s.execute(q).fetchall(), + ) + for i, q in enumerate(qs) + ] + while True: + for t in page_trackers: + t.page = unserialize_bookmark(serialize_bookmark(t.page)) + + page_requests = [ + PageRequest(selectable=t.query, per_page=i + 1, page=t.page) for i, t in enumerate(page_trackers) + ] + pages_with_paging = select_homogeneous_pages(page_requests, s) + for p, t in zip(pages_with_paging, page_trackers): + t.page_with_paging = p + + for i, t in enumerate(list(page_trackers)): + page = assert_paging_core(t.page_with_paging, t.gathered, t.backwards, t.selected, t.page, i + 1) + if page is None: + # Ensure union of pages is original q.all() + assert list(t.gathered) == t.unpaged + page_trackers.remove(t) + continue + + t.page = page + + if not page_trackers: + break + + def test_orm_query1(dburl): spec = [desc(Book.b), Book.d, Book.id] @@ -463,6 +502,38 @@ def test_orm_multiple_pages_empty_queries(): assert get_homogeneous_pages([]) == [] +def test_core_multiple_pages(no_sqlite_dburl): + with S(no_sqlite_dburl, echo=ECHO) as s: + qs = [ + select(Book).order_by(Book.name, Book.id), + select(Book).filter(Book.author_id == 1).order_by(Book.id), + select(Book).order_by(Book.name, Book.id.desc()), + ] + check_multiple_paging_core(qs=qs, s=s) + + +def test_core_multiple_pages_select_columns(no_sqlite_dburl): + with S(no_sqlite_dburl, echo=ECHO) as s: + qs = [ + select(Book.name, Book.author_id, Book.id).order_by(Book.name, Book.id), + select(Book.name, Book.author_id, Book.id).filter(Book.author_id == 1).order_by(Book.id), + select(Book.name, Book.author_id, Book.id).order_by(Book.name, Book.id.desc()), + ] + check_multiple_paging_core(qs=qs, s=s) + + +def test_core_multiple_pages_one_query(no_sqlite_dburl): + with S(no_sqlite_dburl, echo=ECHO) as s: + qs = [ + select(Book).order_by(Book.id), + ] + check_multiple_paging_core(qs=qs, s=s) + + +def test_core_multiple_pages_empty_queries(): + assert select_homogeneous_pages([]) == [] + + def test_core(dburl): selectable = ( select(Book.b, Book.d, Book.id, Book.c) From 87a6a08dfcf57564522aa4243be5486c3d192959 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 11:41:55 -0700 Subject: [PATCH 039/159] Flake 8 violations. --- tests/test_paging.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 8472381a..3b0393e4 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -28,13 +28,15 @@ from sqlalchemy.sql.selectable import Select from sqlakeyset import ( - get_page, get_homogeneous_pages, + get_page, + select_homogeneous_pages, select_page, serialize_bookmark, unserialize_bookmark, InvalidPage, OrmPageRequest, + PageRequest, ) from sqlakeyset.paging import process_args from sqlakeyset.results import Page From 993e45fd0b21e3ed790e7dadac9d171e8a73a42b Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 11:50:02 -0700 Subject: [PATCH 040/159] Fix default. --- tests/test_paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 3b0393e4..f921cf30 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -63,7 +63,7 @@ class _PageTracker: query: Union[Query, Select] unpaged: list - selected: Any + selected: Any = None gathered: deque backwards: bool page: Tuple[Union[MarkerLike, str], bool] From 23ec6da81d96f23a51761f219d48246cf9e1b510 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 11:51:11 -0700 Subject: [PATCH 041/159] non-default before default. --- tests/test_paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index f921cf30..44201593 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -63,10 +63,10 @@ class _PageTracker: query: Union[Query, Select] unpaged: list - selected: Any = None gathered: deque backwards: bool page: Tuple[Union[MarkerLike, str], bool] + selected: Any = None page_with_paging: Optional[Page] = None From aff47d87f58ca44a8d06955a1798f912db7e68e9 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 11:53:44 -0700 Subject: [PATCH 042/159] Add page_identifier --- sqlakeyset/paging.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 4e61e854..2f61b493 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -577,6 +577,7 @@ def _core_prepare_homogeneous_page( backwards=backwards, orm=False, dialect=get_bind(q=selectable, s=s).dialect, + page_identifier=page_identifier, ) def page_from_rows(rows, selected): From dfee9ddee901fa13fdc326c2bdf3e7cec4527c5b Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 12:11:01 -0700 Subject: [PATCH 043/159] filter -> where. --- sqlakeyset/paging.py | 2 ++ tests/test_paging.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 2f61b493..975741f9 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -218,6 +218,7 @@ def prepare_paging( order_by=[c.uo for c in order_cols] ).label("_row_number"), ] + if hasattr(q, "add_columns"): # ORM or SQLAlchemy 1.4+ q = q.add_columns(*extra_columns) else: @@ -582,6 +583,7 @@ def _core_prepare_homogeneous_page( def page_from_rows(rows, selected): keys = list(selected.keys()) + print(keys) N = len(keys) - len(sel.extra_columns) keys = keys[:N] page = core_page_from_rows( diff --git a/tests/test_paging.py b/tests/test_paging.py index 44201593..bbb4fc11 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -508,7 +508,7 @@ def test_core_multiple_pages(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ select(Book).order_by(Book.name, Book.id), - select(Book).filter(Book.author_id == 1).order_by(Book.id), + select(Book).where(Book.author_id == 1).order_by(Book.id), select(Book).order_by(Book.name, Book.id.desc()), ] check_multiple_paging_core(qs=qs, s=s) @@ -518,7 +518,7 @@ def test_core_multiple_pages_select_columns(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ select(Book.name, Book.author_id, Book.id).order_by(Book.name, Book.id), - select(Book.name, Book.author_id, Book.id).filter(Book.author_id == 1).order_by(Book.id), + select(Book.name, Book.author_id, Book.id).where(Book.author_id == 1).order_by(Book.id), select(Book.name, Book.author_id, Book.id).order_by(Book.name, Book.id.desc()), ] check_multiple_paging_core(qs=qs, s=s) From 403a3977399be7cfc1ab693fd897ad699fbbe7fc Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 12:14:10 -0700 Subject: [PATCH 044/159] Add debugging. --- sqlakeyset/paging.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 975741f9..0138e212 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -583,9 +583,10 @@ def _core_prepare_homogeneous_page( def page_from_rows(rows, selected): keys = list(selected.keys()) - print(keys) N = len(keys) - len(sel.extra_columns) keys = keys[:N] + print(keys) + print(rows[0]) page = core_page_from_rows( sel, rows, From ead71a30c1e0200448f11bccd7ac2368ce2115f0 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 12:17:09 -0700 Subject: [PATCH 045/159] Fix select statements. --- tests/test_paging.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index bbb4fc11..24c1db43 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -507,9 +507,9 @@ def test_orm_multiple_pages_empty_queries(): def test_core_multiple_pages(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ - select(Book).order_by(Book.name, Book.id), - select(Book).where(Book.author_id == 1).order_by(Book.id), - select(Book).order_by(Book.name, Book.id.desc()), + select(Book.name).order_by(Book.name, Book.id), + select(Book.name).where(Book.author_id == 1).order_by(Book.id), + select(Book.name).order_by(Book.name, Book.id.desc()), ] check_multiple_paging_core(qs=qs, s=s) @@ -527,7 +527,7 @@ def test_core_multiple_pages_select_columns(no_sqlite_dburl): def test_core_multiple_pages_one_query(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ - select(Book).order_by(Book.id), + select(Book.a, Book.b).order_by(Book.id), ] check_multiple_paging_core(qs=qs, s=s) From 685fca4bc2d3f08b0b7ce6136f6da8418dbe81f8 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Fri, 14 Jul 2023 12:25:09 -0700 Subject: [PATCH 046/159] Go back to Book orm. --- tests/test_paging.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 24c1db43..2701b741 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -507,9 +507,9 @@ def test_orm_multiple_pages_empty_queries(): def test_core_multiple_pages(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ - select(Book.name).order_by(Book.name, Book.id), - select(Book.name).where(Book.author_id == 1).order_by(Book.id), - select(Book.name).order_by(Book.name, Book.id.desc()), + select(Book).order_by(Book.name, Book.id), + select(Book).where(Book.author_id == 1).order_by(Book.id), + select(Book).order_by(Book.name, Book.id.desc()), ] check_multiple_paging_core(qs=qs, s=s) From ccb3e915ac214827fbe7124c650de5a3f3652266 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 11:20:39 -0700 Subject: [PATCH 047/159] add a print statement. --- sqlakeyset/paging.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 0138e212..41ebf1a9 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -583,9 +583,10 @@ def _core_prepare_homogeneous_page( def page_from_rows(rows, selected): keys = list(selected.keys()) + print(f"pre-shrunk keys: {keys}") N = len(keys) - len(sel.extra_columns) keys = keys[:N] - print(keys) + print(f"post-shrunk keys: {keys}") print(rows[0]) page = core_page_from_rows( sel, From 4f62fba2bbc0c8ef3933e04569bd8bec8a3b1053 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 11:34:07 -0700 Subject: [PATCH 048/159] Start with the simplest test first. --- tests/test_paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 2701b741..a17045ea 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -503,7 +503,7 @@ def test_orm_multiple_pages_one_query(no_sqlite_dburl): def test_orm_multiple_pages_empty_queries(): assert get_homogeneous_pages([]) == [] - +""" def test_core_multiple_pages(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ @@ -522,7 +522,7 @@ def test_core_multiple_pages_select_columns(no_sqlite_dburl): select(Book.name, Book.author_id, Book.id).order_by(Book.name, Book.id.desc()), ] check_multiple_paging_core(qs=qs, s=s) - +""" def test_core_multiple_pages_one_query(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: From 43694f09ab775a33f2de3c2c901cb0d67dc13c30 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 11:37:15 -0700 Subject: [PATCH 049/159] flake8 --- tests/test_paging.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_paging.py b/tests/test_paging.py index a17045ea..4202d873 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -503,6 +503,7 @@ def test_orm_multiple_pages_one_query(no_sqlite_dburl): def test_orm_multiple_pages_empty_queries(): assert get_homogeneous_pages([]) == [] + """ def test_core_multiple_pages(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: @@ -524,6 +525,7 @@ def test_core_multiple_pages_select_columns(no_sqlite_dburl): check_multiple_paging_core(qs=qs, s=s) """ + def test_core_multiple_pages_one_query(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ From 34c41b23dde8fa9a94c9fe2862f0ea6511c6a465 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 11:41:28 -0700 Subject: [PATCH 050/159] Print statement caused exception. --- sqlakeyset/paging.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 41ebf1a9..b32d66de 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -587,7 +587,8 @@ def page_from_rows(rows, selected): N = len(keys) - len(sel.extra_columns) keys = keys[:N] print(f"post-shrunk keys: {keys}") - print(rows[0]) + if rows: + print(rows[0]) page = core_page_from_rows( sel, rows, From db47749535190581bc6886d10a10f7aa5d4e1ded Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 12:55:45 -0700 Subject: [PATCH 051/159] Don't double order single selects. --- sqlakeyset/paging.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index b32d66de..2ab8f824 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -540,7 +540,10 @@ def select_homogeneous_pages( select = union_all( *[p.paging_query.select for p in prepared_queries] - ).order_by(text("_page_identifier"), text("_row_number")) + ) + if len(prepared_queries) > 1: + select = select.order_by(text("_page_identifier"), text("_row_number")) + selected = s.execute(select) results = selected.fetchall() From 90e9ae760eb03b3944de40fda681f958e93ae6c7 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 13:00:36 -0700 Subject: [PATCH 052/159] Fix test_core..._empty_queries. --- tests/test_paging.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 4202d873..6a054ba3 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -535,7 +535,8 @@ def test_core_multiple_pages_one_query(no_sqlite_dburl): def test_core_multiple_pages_empty_queries(): - assert select_homogeneous_pages([]) == [] + with S(no_sqlite_dburl, echo=ECHO) as s: + assert select_homogeneous_pages([], s) == [] def test_core(dburl): From 8ecb334a6aba9b525f6a0a4b6f379b8ebadc751f Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 13:02:12 -0700 Subject: [PATCH 053/159] flake8. --- tests/test_paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 6a054ba3..dd0e6d11 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -534,7 +534,7 @@ def test_core_multiple_pages_one_query(no_sqlite_dburl): check_multiple_paging_core(qs=qs, s=s) -def test_core_multiple_pages_empty_queries(): +def test_core_multiple_pages_empty_queries(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: assert select_homogeneous_pages([], s) == [] From 6c33a0f3e3258092dbcfe869ebc0adb198458c2e Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 13:05:14 -0700 Subject: [PATCH 054/159] One more test. --- sqlakeyset/paging.py | 1 + tests/test_paging.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 2ab8f824..edbea85d 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -544,6 +544,7 @@ def select_homogeneous_pages( if len(prepared_queries) > 1: select = select.order_by(text("_page_identifier"), text("_row_number")) + print(f"Select statement: {select}") selected = s.execute(select) results = selected.fetchall() diff --git a/tests/test_paging.py b/tests/test_paging.py index dd0e6d11..eab245a4 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -513,6 +513,7 @@ def test_core_multiple_pages(no_sqlite_dburl): select(Book).order_by(Book.name, Book.id.desc()), ] check_multiple_paging_core(qs=qs, s=s) +""" def test_core_multiple_pages_select_columns(no_sqlite_dburl): @@ -523,7 +524,6 @@ def test_core_multiple_pages_select_columns(no_sqlite_dburl): select(Book.name, Book.author_id, Book.id).order_by(Book.name, Book.id.desc()), ] check_multiple_paging_core(qs=qs, s=s) -""" def test_core_multiple_pages_one_query(no_sqlite_dburl): From e230d1a421ea637de237551f4a2e5e036518dac4 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 13:19:08 -0700 Subject: [PATCH 055/159] Try using sub-select for key generation. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index edbea85d..575cba86 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -558,7 +558,7 @@ def select_homogeneous_pages( pages = [] for i in range(len(requests)): rows = page_to_rows[i] - pages.append(prepared_queries[i].page_from_rows(rows, selected)) + pages.append(prepared_queries[i].page_from_rows(rows, s.execute(prepared_queries[i].paging_query.select))) return pages From 0917ceb340999d0904cc28d1bd5a38be4c4da906 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 13:24:54 -0700 Subject: [PATCH 056/159] See if individualized selected works for select(Book) --- tests/test_paging.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index eab245a4..b3b93c2e 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -504,7 +504,6 @@ def test_orm_multiple_pages_empty_queries(): assert get_homogeneous_pages([]) == [] -""" def test_core_multiple_pages(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ @@ -513,7 +512,6 @@ def test_core_multiple_pages(no_sqlite_dburl): select(Book).order_by(Book.name, Book.id.desc()), ] check_multiple_paging_core(qs=qs, s=s) -""" def test_core_multiple_pages_select_columns(no_sqlite_dburl): From 719440c222787852d67a4119bf25244bf6d8674f Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 13:40:24 -0700 Subject: [PATCH 057/159] Try a different way of forming the union. --- sqlakeyset/paging.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 575cba86..d5318667 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -538,10 +538,14 @@ def select_homogeneous_pages( prepared_queries = [_core_prepare_homogeneous_page(request, s, i) for i, request in enumerate(requests)] + """ select = union_all( *[p.paging_query.select for p in prepared_queries] ) + """ + select = prepared_queries[0].paging_query.select if len(prepared_queries) > 1: + select = select.union_all(*[p.paging_query.select for p in prepared_queries[1:]]) select = select.order_by(text("_page_identifier"), text("_row_number")) print(f"Select statement: {select}") From e192516bfb68a08035439fc27d640c38fe89047e Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 13:41:38 -0700 Subject: [PATCH 058/159] flake8 --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index d5318667..a8a2330f 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -24,7 +24,7 @@ from sqlalchemy.engine.interfaces import Dialect from sqlalchemy.orm import Session from sqlalchemy.orm.query import Query -from sqlalchemy.sql.expression import ColumnElement, literal, union_all +from sqlalchemy.sql.expression import ColumnElement, literal from sqlalchemy.sql.selectable import Select from .columns import OC, MappedOrderColumn, find_order_key, parse_ob_clause From 6b596a5fe79f27745df2f0076c2fe41c57ef990f Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 13:44:32 -0700 Subject: [PATCH 059/159] Backtrack on testing, plus add a more fundamental test. --- tests/test_paging.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_paging.py b/tests/test_paging.py index b3b93c2e..f37a91ab 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -504,6 +504,7 @@ def test_orm_multiple_pages_empty_queries(): assert get_homogeneous_pages([]) == [] +""" def test_core_multiple_pages(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ @@ -512,6 +513,7 @@ def test_core_multiple_pages(no_sqlite_dburl): select(Book).order_by(Book.name, Book.id.desc()), ] check_multiple_paging_core(qs=qs, s=s) +""" def test_core_multiple_pages_select_columns(no_sqlite_dburl): @@ -552,6 +554,17 @@ def test_core(dburl): check_paging_core(selectable=selectable, s=s.connection()) +def test_core_whole_model(dburl): + selectable = ( + select(Book) + .where(Book.d == 99) + .order_by(Book.id) + ) + + with S(dburl, echo=ECHO) as s: + check_paging_core(selectable=selectable, s=s) + + def test_core2(dburl): with S(dburl, echo=ECHO) as s: sel = select(Book.score).order_by(Book.id) From 806ef9d2a6178db7562a9358a6e74bc5e2bc2bc0 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 13:47:46 -0700 Subject: [PATCH 060/159] Add a test. --- tests/test_paging.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_paging.py b/tests/test_paging.py index f37a91ab..e206a71e 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -534,6 +534,14 @@ def test_core_multiple_pages_one_query(no_sqlite_dburl): check_multiple_paging_core(qs=qs, s=s) +def test_core_multiple_pages_one_query_whole_model(no_sqlite_dburl): + with S(no_sqlite_dburl, echo=ECHO) as s: + qs = [ + select(Book).order_by(Book.id), + ] + check_multiple_paging_core(qs=qs, s=s) + + def test_core_multiple_pages_empty_queries(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: assert select_homogeneous_pages([], s) == [] From fa5fd494817f9dfed9a6ab5a936d7c599c09d142 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 13:55:14 -0700 Subject: [PATCH 061/159] Only execute one subselect as necessary. --- sqlakeyset/paging.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index a8a2330f..b3dddb65 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -538,14 +538,10 @@ def select_homogeneous_pages( prepared_queries = [_core_prepare_homogeneous_page(request, s, i) for i, request in enumerate(requests)] - """ select = union_all( *[p.paging_query.select for p in prepared_queries] ) - """ - select = prepared_queries[0].paging_query.select if len(prepared_queries) > 1: - select = select.union_all(*[p.paging_query.select for p in prepared_queries[1:]]) select = select.order_by(text("_page_identifier"), text("_row_number")) print(f"Select statement: {select}") @@ -560,9 +556,17 @@ def select_homogeneous_pages( page_to_rows[row._page_identifier].append(row) pages = [] + # This is an unfortunate side effect of union_all. It appears we union_all + # a bunch of selects, it changes the "keys" on us in cases where the column + # name and python attribute don't match. So we have to execute the first + # query standalone to ge the correct keys. + subselect_result = ( + s.execute(prepared_queries[0].paging_query.select) + if len(prepared_queries) > 1 else selected + ) for i in range(len(requests)): rows = page_to_rows[i] - pages.append(prepared_queries[i].page_from_rows(rows, s.execute(prepared_queries[i].paging_query.select))) + pages.append(prepared_queries[i].page_from_rows(rows, subselect_result)) return pages From 1b9133b767ecba64fb733d27e6669c756dd49597 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 14:02:03 -0700 Subject: [PATCH 062/159] flake8 --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index b3dddb65..4313ed07 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -24,7 +24,7 @@ from sqlalchemy.engine.interfaces import Dialect from sqlalchemy.orm import Session from sqlalchemy.orm.query import Query -from sqlalchemy.sql.expression import ColumnElement, literal +from sqlalchemy.sql.expression import ColumnElement, literal, union_all from sqlalchemy.sql.selectable import Select from .columns import OC, MappedOrderColumn, find_order_key, parse_ob_clause From 1909b8e9e29d25896f6742b242f8a384976d5f72 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 14:11:40 -0700 Subject: [PATCH 063/159] Clean up single query select. --- sqlakeyset/paging.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 4313ed07..d5d77043 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -538,10 +538,12 @@ def select_homogeneous_pages( prepared_queries = [_core_prepare_homogeneous_page(request, s, i) for i, request in enumerate(requests)] - select = union_all( - *[p.paging_query.select for p in prepared_queries] - ) - if len(prepared_queries) > 1: + if len(requests) == 1: + select = prepared_queries[0].paging_query.select + else: + select = union_all( + *[p.paging_query.select for p in prepared_queries] + ) select = select.order_by(text("_page_identifier"), text("_row_number")) print(f"Select statement: {select}") From 1af24f8d8c57121b6d0280ed4d22894098c20feb Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 14:38:34 -0700 Subject: [PATCH 064/159] Uncomment test to send Anthony error. --- sqlakeyset/paging.py | 2 +- tests/test_paging.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index d5d77043..85ba6ab8 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -558,7 +558,7 @@ def select_homogeneous_pages( page_to_rows[row._page_identifier].append(row) pages = [] - # This is an unfortunate side effect of union_all. It appears we union_all + # This is an unfortunate side effect of union_all. It appears when we union_all # a bunch of selects, it changes the "keys" on us in cases where the column # name and python attribute don't match. So we have to execute the first # query standalone to ge the correct keys. diff --git a/tests/test_paging.py b/tests/test_paging.py index e206a71e..85c3ecd9 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -504,7 +504,6 @@ def test_orm_multiple_pages_empty_queries(): assert get_homogeneous_pages([]) == [] -""" def test_core_multiple_pages(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ @@ -513,7 +512,6 @@ def test_core_multiple_pages(no_sqlite_dburl): select(Book).order_by(Book.name, Book.id.desc()), ] check_multiple_paging_core(qs=qs, s=s) -""" def test_core_multiple_pages_select_columns(no_sqlite_dburl): From a13e1b2991dfe18bef9b9a47ea13085ea3a296d2 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 15:19:52 -0700 Subject: [PATCH 065/159] Play around with corresponding_column --- sqlakeyset/paging.py | 18 ++++++++++++------ tests/test_paging.py | 2 ++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 85ba6ab8..c9c374fc 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -538,15 +538,19 @@ def select_homogeneous_pages( prepared_queries = [_core_prepare_homogeneous_page(request, s, i) for i, request in enumerate(requests)] - if len(requests) == 1: - select = prepared_queries[0].paging_query.select - else: - select = union_all( - *[p.paging_query.select for p in prepared_queries] - ) + select = union_all( + *[p.paging_query.select for p in prepared_queries] + ) + if len(requests) > 1: select = select.order_by(text("_page_identifier"), text("_row_number")) print(f"Select statement: {select}") + mapped_order_columns = [] + for p in prepared_queries: + paging_query = p.paging_query + for ocol in paging_query.order_columns: + corresponding_column = select.corresponding_column(ocol.element) + mapped_order_columns.append(find_order_key(OC(corresponding_column), select.column_descriptions)) selected = s.execute(select) results = selected.fetchall() @@ -568,6 +572,8 @@ def select_homogeneous_pages( ) for i in range(len(requests)): rows = page_to_rows[i] + key_rows = [tuple(col.get_from_row(row) for col in mapped_order_columns) for row in rows] + print(f"Key rows: {key_rows}") pages.append(prepared_queries[i].page_from_rows(rows, subselect_result)) return pages diff --git a/tests/test_paging.py b/tests/test_paging.py index 85c3ecd9..d41be10a 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -514,6 +514,7 @@ def test_core_multiple_pages(no_sqlite_dburl): check_multiple_paging_core(qs=qs, s=s) +""" def test_core_multiple_pages_select_columns(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ @@ -538,6 +539,7 @@ def test_core_multiple_pages_one_query_whole_model(no_sqlite_dburl): select(Book).order_by(Book.id), ] check_multiple_paging_core(qs=qs, s=s) +""" def test_core_multiple_pages_empty_queries(no_sqlite_dburl): From ca3e0d3aac42383d9f20dcc4837434e1c2774e3e Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 15:23:45 -0700 Subject: [PATCH 066/159] Use raw_cols instead --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index c9c374fc..abd97b45 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -550,7 +550,7 @@ def select_homogeneous_pages( paging_query = p.paging_query for ocol in paging_query.order_columns: corresponding_column = select.corresponding_column(ocol.element) - mapped_order_columns.append(find_order_key(OC(corresponding_column), select.column_descriptions)) + mapped_order_columns.append(find_order_key(OC(corresponding_column), select._raw_columns)) selected = s.execute(select) results = selected.fetchall() From 2a9baefd0aab5552050b0a0a1ad411220c789636 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 15:41:04 -0700 Subject: [PATCH 067/159] Try outer select. --- sqlakeyset/paging.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index abd97b45..045ca05f 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -24,7 +24,7 @@ from sqlalchemy.engine.interfaces import Dialect from sqlalchemy.orm import Session from sqlalchemy.orm.query import Query -from sqlalchemy.sql.expression import ColumnElement, literal, union_all +from sqlalchemy.sql.expression import ColumnElement, literal, select, union_all from sqlalchemy.sql.selectable import Select from .columns import OC, MappedOrderColumn, find_order_key, parse_ob_clause @@ -538,20 +538,16 @@ def select_homogeneous_pages( prepared_queries = [_core_prepare_homogeneous_page(request, s, i) for i, request in enumerate(requests)] - select = union_all( + selectable = union_all( *[p.paging_query.select for p in prepared_queries] ) if len(requests) > 1: - select = select.order_by(text("_page_identifier"), text("_row_number")) - - print(f"Select statement: {select}") - mapped_order_columns = [] - for p in prepared_queries: - paging_query = p.paging_query - for ocol in paging_query.order_columns: - corresponding_column = select.corresponding_column(ocol.element) - mapped_order_columns.append(find_order_key(OC(corresponding_column), select._raw_columns)) - selected = s.execute(select) + selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) + + print(f"Select statement: {selectable}") + selectable = select(selectable.selected_columns).select_from(selectable) + print(f"Select from statement: {selectable}") + selected = s.execute(selectable) results = selected.fetchall() From 2b8ceeb83b158dd58755cf2b5edfb4a25a56c132 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 15:42:26 -0700 Subject: [PATCH 068/159] flake8 --- sqlakeyset/paging.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 045ca05f..1a432a53 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -568,8 +568,6 @@ def select_homogeneous_pages( ) for i in range(len(requests)): rows = page_to_rows[i] - key_rows = [tuple(col.get_from_row(row) for col in mapped_order_columns) for row in rows] - print(f"Key rows: {key_rows}") pages.append(prepared_queries[i].page_from_rows(rows, subselect_result)) return pages From b94a6e4bf79b5663504c818ab752552f3a18f9be Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 15:47:33 -0700 Subject: [PATCH 069/159] Try outer select based on original. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 1a432a53..21866e21 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -545,7 +545,7 @@ def select_homogeneous_pages( selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) print(f"Select statement: {selectable}") - selectable = select(selectable.selected_columns).select_from(selectable) + selectable = select(prepared_queries[0].paging_query.select.selected_columns).select_from(selectable) print(f"Select from statement: {selectable}") selected = s.execute(selectable) From bebaea33f21ecb21fd1064321c6d2d095c83ce7e Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 15:50:51 -0700 Subject: [PATCH 070/159] This time use requests in the inner. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 21866e21..daf934fd 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -545,7 +545,7 @@ def select_homogeneous_pages( selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) print(f"Select statement: {selectable}") - selectable = select(prepared_queries[0].paging_query.select.selected_columns).select_from(selectable) + selectable = select(requests[0].selectable.selected_columns).select_from(selectable) print(f"Select from statement: {selectable}") selected = s.execute(selectable) From ad4548f0e8ebdf74125eb02621fdf4950a9000e1 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 16:07:48 -0700 Subject: [PATCH 071/159] Add extra columns. --- sqlakeyset/paging.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index daf934fd..0cf91415 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -546,6 +546,7 @@ def select_homogeneous_pages( print(f"Select statement: {selectable}") selectable = select(requests[0].selectable.selected_columns).select_from(selectable) + selectable = selectable.add_columns(*prepared_queries[0].paging_query.extra_columns) print(f"Select from statement: {selectable}") selected = s.execute(selectable) From cf67e5d8a13b2a8e74d019e2aff8687b2029a9bc Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 16:18:13 -0700 Subject: [PATCH 072/159] Try remapping corresponding columns. --- sqlakeyset/paging.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 0cf91415..e71472c1 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -567,8 +567,15 @@ def select_homogeneous_pages( s.execute(prepared_queries[0].paging_query.select) if len(prepared_queries) > 1 else selected ) + for i in range(len(requests)): rows = page_to_rows[i] + mapped_order_columns = [] + for ocol in prepared_queries[i].paging_query.order_columns: + corresponding_column = selectable.corresponding_column(ocol.element) + mapped_order_columns.append(find_order_key(OC(corresponding_column), selectable._raw_columns)) + key_rows = [tuple(col.get_from_row(row) for col in mapped_order_columns) for row in rows] + print(f"key_rows: {key_rows}") pages.append(prepared_queries[i].page_from_rows(rows, subselect_result)) return pages From fb1259c8ae9d7158c51451adb10fe24e5ecb06fd Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 16:20:42 -0700 Subject: [PATCH 073/159] Use column_descriptions. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index e71472c1..307ff4dd 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -573,7 +573,7 @@ def select_homogeneous_pages( mapped_order_columns = [] for ocol in prepared_queries[i].paging_query.order_columns: corresponding_column = selectable.corresponding_column(ocol.element) - mapped_order_columns.append(find_order_key(OC(corresponding_column), selectable._raw_columns)) + mapped_order_columns.append(find_order_key(OC(corresponding_column), selectable.column_descriptions)) key_rows = [tuple(col.get_from_row(row) for col in mapped_order_columns) for row in rows] print(f"key_rows: {key_rows}") pages.append(prepared_queries[i].page_from_rows(rows, subselect_result)) From e520f2e4e61775bb647927f06d7fead73c3b1a88 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 16:33:39 -0700 Subject: [PATCH 074/159] More flexible column mapping. --- sqlakeyset/columns.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sqlakeyset/columns.py b/sqlakeyset/columns.py index 4295ce01..ef45589e 100644 --- a/sqlakeyset/columns.py +++ b/sqlakeyset/columns.py @@ -366,9 +366,8 @@ def derive_order_key(ocol, desc, index): else: return None - entity = desc["entity"] expr = desc["expr"] - + entity = desc.get("entity") if isinstance(expr, Bundle): for key, col in dict(expr.columns).items(): if strip_labels(col).compare(ocol.comparable_value): From a00da44c21e46e605bea097b4ad311af10aa41ea Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 16:40:06 -0700 Subject: [PATCH 075/159] Add logging. --- sqlakeyset/columns.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sqlakeyset/columns.py b/sqlakeyset/columns.py index ef45589e..689018a5 100644 --- a/sqlakeyset/columns.py +++ b/sqlakeyset/columns.py @@ -408,6 +408,7 @@ def derive_order_key(ocol, desc, index): # is an attribute with label try: + print(f"Attempting to match label: {ocol.quoted_full_name} / {OC(expr).full_name}") if ocol.quoted_full_name == OC(expr).full_name: return DirectColumn(ocol, index) except sqlalchemy.exc.ArgumentError: From 106d0fa17c2bc477d84864970ef907b57c39559a Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 16:45:10 -0700 Subject: [PATCH 076/159] Compare quoted to quoted. --- sqlakeyset/columns.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sqlakeyset/columns.py b/sqlakeyset/columns.py index 689018a5..94126dd2 100644 --- a/sqlakeyset/columns.py +++ b/sqlakeyset/columns.py @@ -408,8 +408,8 @@ def derive_order_key(ocol, desc, index): # is an attribute with label try: - print(f"Attempting to match label: {ocol.quoted_full_name} / {OC(expr).full_name}") - if ocol.quoted_full_name == OC(expr).full_name: + print(f"Attempting to match label: {ocol.quoted_full_name} / {OC(expr).full_name} / {OC(expr).quoted_full_name}") + if ocol.quoted_full_name == OC(expr).quoted_full_name: return DirectColumn(ocol, index) except sqlalchemy.exc.ArgumentError: pass From 1af4901196c6612984dc82e7795abae4bebbb826 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 16:55:51 -0700 Subject: [PATCH 077/159] Try using the remapped columns. --- sqlakeyset/paging.py | 23 ++++++++++++----------- tests/test_paging.py | 1 + 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 307ff4dd..886e6ab2 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -567,16 +567,24 @@ def select_homogeneous_pages( s.execute(prepared_queries[0].paging_query.select) if len(prepared_queries) > 1 else selected ) + keys = list(subselect_result.keys()) + print(f"pre-shrunk keys: {keys}") + N = len(keys) - len(prepared_queries[0].paging_query.extra_columns) + keys = keys[:N] + print(f"post-shrunk keys: {keys}") for i in range(len(requests)): rows = page_to_rows[i] + prepared_query = prepared_queries[i] + old_sel = prepared_query.paging_query mapped_order_columns = [] - for ocol in prepared_queries[i].paging_query.order_columns: + for ocol in prepared_query.paging_query.order_columns: corresponding_column = selectable.corresponding_column(ocol.element) mapped_order_columns.append(find_order_key(OC(corresponding_column), selectable.column_descriptions)) key_rows = [tuple(col.get_from_row(row) for col in mapped_order_columns) for row in rows] print(f"key_rows: {key_rows}") - pages.append(prepared_queries[i].page_from_rows(rows, subselect_result)) + sel = _PagingSelect(old_sel.select, old_sel.order_columns, mapped_order_columns, old_sel.extra_columns) + pages.append(prepared_queries[i].page_from_rows(rows, sel, keys)) return pages @@ -603,16 +611,9 @@ def _core_prepare_homogeneous_page( page_identifier=page_identifier, ) - def page_from_rows(rows, selected): - keys = list(selected.keys()) - print(f"pre-shrunk keys: {keys}") - N = len(keys) - len(sel.extra_columns) - keys = keys[:N] - print(f"post-shrunk keys: {keys}") - if rows: - print(rows[0]) + def page_from_rows(rows, paging_select, keys): page = core_page_from_rows( - sel, + paging_select, rows, keys, result_type, diff --git a/tests/test_paging.py b/tests/test_paging.py index d41be10a..57fbdc3e 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -505,6 +505,7 @@ def test_orm_multiple_pages_empty_queries(): def test_core_multiple_pages(no_sqlite_dburl): + # TODO: Add a test with an order by that adds an extra column. with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ select(Book).order_by(Book.name, Book.id), From 720992594d0b7939d6e50cab8ed8f607e666f8ec Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 17:00:20 -0700 Subject: [PATCH 078/159] Print page_to_rows. --- sqlakeyset/paging.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 886e6ab2..d90db298 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -572,6 +572,7 @@ def select_homogeneous_pages( N = len(keys) - len(prepared_queries[0].paging_query.extra_columns) keys = keys[:N] print(f"post-shrunk keys: {keys}") + print(f"Page to rows: {page_to_rows}") for i in range(len(requests)): rows = page_to_rows[i] From 44eb2ee9f6c0cfbcbf524964d1d579f02da1b852 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 17:08:11 -0700 Subject: [PATCH 079/159] Order outer select. --- sqlakeyset/paging.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index d90db298..1af823d2 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -541,12 +541,15 @@ def select_homogeneous_pages( selectable = union_all( *[p.paging_query.select for p in prepared_queries] ) + """ if len(requests) > 1: selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) + """ print(f"Select statement: {selectable}") selectable = select(requests[0].selectable.selected_columns).select_from(selectable) selectable = selectable.add_columns(*prepared_queries[0].paging_query.extra_columns) + selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) print(f"Select from statement: {selectable}") selected = s.execute(selectable) From b98d3641014739ed620e21de28c1e882dda2da08 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 17:17:35 -0700 Subject: [PATCH 080/159] Fix extra columns kinda. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 1af823d2..ac17cffe 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -548,7 +548,7 @@ def select_homogeneous_pages( print(f"Select statement: {selectable}") selectable = select(requests[0].selectable.selected_columns).select_from(selectable) - selectable = selectable.add_columns(*prepared_queries[0].paging_query.extra_columns) + selectable = selectable.add_columns(text("_page_identifier"), text("_row_number")) selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) print(f"Select from statement: {selectable}") selected = s.execute(selectable) From 014e1c5b64686d89432e68a8e7d86ec41a03bd41 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 17:28:59 -0700 Subject: [PATCH 081/159] Print literal bound statement. --- sqlakeyset/paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index ac17cffe..0a79441e 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -546,11 +546,11 @@ def select_homogeneous_pages( selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) """ - print(f"Select statement: {selectable}") + print(f"Select statement: {selectable.compile(compile_kwargs={"literal_binds": True})}") selectable = select(requests[0].selectable.selected_columns).select_from(selectable) selectable = selectable.add_columns(text("_page_identifier"), text("_row_number")) selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) - print(f"Select from statement: {selectable}") + print(f"Select from statement: {selectable.compile(compile_kwargs={"literal_binds": True})}") selected = s.execute(selectable) results = selected.fetchall() From f080b0dcddd0c95af3436caea22eede253045a65 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 17:31:59 -0700 Subject: [PATCH 082/159] Try to fix f string. --- sqlakeyset/paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 0a79441e..2d3e41f4 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -546,11 +546,11 @@ def select_homogeneous_pages( selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) """ - print(f"Select statement: {selectable.compile(compile_kwargs={"literal_binds": True})}") + print(f"Select statement: { selectable.compile(compile_kwargs={"literal_binds": True}) }") selectable = select(requests[0].selectable.selected_columns).select_from(selectable) selectable = selectable.add_columns(text("_page_identifier"), text("_row_number")) selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) - print(f"Select from statement: {selectable.compile(compile_kwargs={"literal_binds": True})}") + print(f"Select from statement: { selectable.compile(compile_kwargs={"literal_binds": True}) }") selected = s.execute(selectable) results = selected.fetchall() From 6de382d859a000f22a4d84c2d960a59c48a2f949 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 17:33:15 -0700 Subject: [PATCH 083/159] Bail on f string. --- sqlakeyset/paging.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 2d3e41f4..180ad299 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -545,12 +545,13 @@ def select_homogeneous_pages( if len(requests) > 1: selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) """ - - print(f"Select statement: { selectable.compile(compile_kwargs={"literal_binds": True}) }") + compiled = selectable.compile(compile_kwargs={"literal_binds": True}) + print(f"Select statement: {compiled}") selectable = select(requests[0].selectable.selected_columns).select_from(selectable) selectable = selectable.add_columns(text("_page_identifier"), text("_row_number")) selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) - print(f"Select from statement: { selectable.compile(compile_kwargs={"literal_binds": True}) }") + compiled = selectable.compile(compile_kwargs={"literal_binds": True}) + print(f"Select from statement: {compiled}") selected = s.execute(selectable) results = selected.fetchall() From 33280252a82c7753455e53f86d4a83bebc5982d2 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 17:38:21 -0700 Subject: [PATCH 084/159] Try using .c --- sqlakeyset/paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 180ad299..84c4a987 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -548,8 +548,8 @@ def select_homogeneous_pages( compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select statement: {compiled}") selectable = select(requests[0].selectable.selected_columns).select_from(selectable) - selectable = selectable.add_columns(text("_page_identifier"), text("_row_number")) - selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) + selectable = selectable.add_columns(selectable.c._page_identifier, selectable.c._row_number) + selectable = selectable.order_by(selectable.c._page_identifier, selectable.c._row_number) compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select from statement: {compiled}") selected = s.execute(selectable) From 5f22343cb4107632b9b5d365c24a6f93c7e8d031 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 17:43:14 -0700 Subject: [PATCH 085/159] Select * --- sqlakeyset/paging.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 84c4a987..0147d0dd 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -547,9 +547,8 @@ def select_homogeneous_pages( """ compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select statement: {compiled}") - selectable = select(requests[0].selectable.selected_columns).select_from(selectable) - selectable = selectable.add_columns(selectable.c._page_identifier, selectable.c._row_number) - selectable = selectable.order_by(selectable.c._page_identifier, selectable.c._row_number) + selectable = select(text("*")).select_from(selectable) + selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select from statement: {compiled}") selected = s.execute(selectable) From e4f4c8bc30e484f03d7cebc8c0beb61ff2325810 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 17:46:18 -0700 Subject: [PATCH 086/159] Let's try .columns? --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 0147d0dd..d0bd8d82 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -547,7 +547,7 @@ def select_homogeneous_pages( """ compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select statement: {compiled}") - selectable = select(text("*")).select_from(selectable) + selectable = select([col for col in selectable.columns]).select_from(selectable) selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select from statement: {compiled}") From 948ab10c3445a588599ae291100de5642caa13d8 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Tue, 18 Jul 2023 17:48:53 -0700 Subject: [PATCH 087/159] Try subquery. --- sqlakeyset/paging.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index d0bd8d82..2210826e 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -547,8 +547,11 @@ def select_homogeneous_pages( """ compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select statement: {compiled}") - selectable = select([col for col in selectable.columns]).select_from(selectable) + """ + selectable = select(*[col for col in selectable.columns]).select_from(selectable) selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) + """ + selectable = select(selectable.subquery()).order_by(text("_page_identifier"), text("_row_number")) compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select from statement: {compiled}") selected = s.execute(selectable) From fbdc607f3f4992c9575ace641f453bb2c86fa5a3 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 09:12:48 -0700 Subject: [PATCH 088/159] Add prints to check row equivalence. --- tests/test_paging.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_paging.py b/tests/test_paging.py index 57fbdc3e..ecc75c8b 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -151,6 +151,10 @@ def check_multiple_paging_orm(qs): page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, t.page, i + 1) if page is None: # Ensure union of pages is original q.all() + for actual, expected in zip(t.gathered, t.paged): + print(f"actual: {actual.__dict__}") + print(f"expected: {expected.__dict__}") + assert False assert list(t.gathered) == t.unpaged page_trackers.remove(t) continue From 6d33e3de5c0cb267dda74cf92189c462373dac73 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 09:19:04 -0700 Subject: [PATCH 089/159] Fix spelling mistake. --- tests/test_paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index ecc75c8b..ebb45f2d 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -151,7 +151,7 @@ def check_multiple_paging_orm(qs): page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, t.page, i + 1) if page is None: # Ensure union of pages is original q.all() - for actual, expected in zip(t.gathered, t.paged): + for actual, expected in zip(t.gathered, t.unpaged): print(f"actual: {actual.__dict__}") print(f"expected: {expected.__dict__}") assert False From ac95966515c02b8905292188dc7262d404a4a3da Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 09:23:08 -0700 Subject: [PATCH 090/159] Compare __dict__ --- tests/test_paging.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_paging.py b/tests/test_paging.py index ebb45f2d..97673e18 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -154,6 +154,7 @@ def check_multiple_paging_orm(qs): for actual, expected in zip(t.gathered, t.unpaged): print(f"actual: {actual.__dict__}") print(f"expected: {expected.__dict__}") + assert actual.__dict__ == expected.__dict__ assert False assert list(t.gathered) == t.unpaged page_trackers.remove(t) From ddc3fdcc44b69cfc244e03c3927f4e8b3c4e5dc0 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 09:23:33 -0700 Subject: [PATCH 091/159] Remove known bad assertions. --- tests/test_paging.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 97673e18..66009546 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -155,8 +155,6 @@ def check_multiple_paging_orm(qs): print(f"actual: {actual.__dict__}") print(f"expected: {expected.__dict__}") assert actual.__dict__ == expected.__dict__ - assert False - assert list(t.gathered) == t.unpaged page_trackers.remove(t) continue From 8dcfe3f68cd3f8065ca21acd9ad9818f75d658a1 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 09:34:04 -0700 Subject: [PATCH 092/159] Fix non-orm assertions. --- tests/test_paging.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 66009546..acbe7dfa 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -152,9 +152,16 @@ def check_multiple_paging_orm(qs): if page is None: # Ensure union of pages is original q.all() for actual, expected in zip(t.gathered, t.unpaged): + if hasattr(expected, __dict__): + # ORM objects are tough. We don't return exactly the same result + # type, but the results are functionally equivalent as tested + # here. + assert actual.__dict__ == expected.__dict__ + else: + assert actual == expected print(f"actual: {actual.__dict__}") print(f"expected: {expected.__dict__}") - assert actual.__dict__ == expected.__dict__ + page_trackers.remove(t) continue From 9aea2d6dc7a9f9ef1ba3e725b82ceab46adc5b27 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 09:35:16 -0700 Subject: [PATCH 093/159] Fix hasattr --- tests/test_paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index acbe7dfa..47ee2ee7 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -152,7 +152,7 @@ def check_multiple_paging_orm(qs): if page is None: # Ensure union of pages is original q.all() for actual, expected in zip(t.gathered, t.unpaged): - if hasattr(expected, __dict__): + if hasattr(expected, "__dict__"): # ORM objects are tough. We don't return exactly the same result # type, but the results are functionally equivalent as tested # here. @@ -161,7 +161,7 @@ def check_multiple_paging_orm(qs): assert actual == expected print(f"actual: {actual.__dict__}") print(f"expected: {expected.__dict__}") - + page_trackers.remove(t) continue From ac61edde4d2495ca430908d0e1ffbcd6af2924e8 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 09:38:23 -0700 Subject: [PATCH 094/159] hasattr didn't work. --- tests/test_paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 47ee2ee7..37e6df1a 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -152,7 +152,7 @@ def check_multiple_paging_orm(qs): if page is None: # Ensure union of pages is original q.all() for actual, expected in zip(t.gathered, t.unpaged): - if hasattr(expected, "__dict__"): + if isinstance(expected, Base): # ORM objects are tough. We don't return exactly the same result # type, but the results are functionally equivalent as tested # here. From 1ce0889c60e13348e3470add1204b9b99d1827a5 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 09:39:59 -0700 Subject: [PATCH 095/159] Remove bad print. --- tests/test_paging.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 37e6df1a..a6015c9e 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -159,8 +159,6 @@ def check_multiple_paging_orm(qs): assert actual.__dict__ == expected.__dict__ else: assert actual == expected - print(f"actual: {actual.__dict__}") - print(f"expected: {expected.__dict__}") page_trackers.remove(t) continue From 38d6e684f18d87c78b00f4f4d5b2c9bd0cee0e46 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 09:47:44 -0700 Subject: [PATCH 096/159] Try unpacking tuple in test just to see. --- tests/test_paging.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index a6015c9e..95a5e16f 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -151,15 +151,7 @@ def check_multiple_paging_orm(qs): page = assert_paging_orm(t.page_with_paging, t.gathered, t.backwards, t.unpaged, t.page, i + 1) if page is None: # Ensure union of pages is original q.all() - for actual, expected in zip(t.gathered, t.unpaged): - if isinstance(expected, Base): - # ORM objects are tough. We don't return exactly the same result - # type, but the results are functionally equivalent as tested - # here. - assert actual.__dict__ == expected.__dict__ - else: - assert actual == expected - + assert list(t.gathered) == t.unpaged page_trackers.remove(t) continue @@ -242,7 +234,11 @@ def check_multiple_paging_core(qs, s): page = assert_paging_core(t.page_with_paging, t.gathered, t.backwards, t.selected, t.page, i + 1) if page is None: # Ensure union of pages is original q.all() - assert list(t.gathered) == t.unpaged + for actual, expected in zip(t.gathered, t.unpaged): + if isinstance(expected, tuple) and not isinstance(actual, tuple): + assert expected[0] == actual + else: + assert actual == expected page_trackers.remove(t) continue From 61d1db6371a9f339f7e9b0eb76276d67754cc77b Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 09:53:16 -0700 Subject: [PATCH 097/159] More print info. --- tests/test_paging.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 95a5e16f..a80400af 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -235,7 +235,8 @@ def check_multiple_paging_core(qs, s): if page is None: # Ensure union of pages is original q.all() for actual, expected in zip(t.gathered, t.unpaged): - if isinstance(expected, tuple) and not isinstance(actual, tuple): + if len(expected) != len(actual): + print(f"Expected type: {type(expected)} / Actual type: {type(actual)} / Expected[0] type: {type(expected[0])}") assert expected[0] == actual else: assert actual == expected From 9989b09b7929c539470ce1111010fc0c1780dcb7 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 10:10:52 -0700 Subject: [PATCH 098/159] Test multiple ORM. --- sqlakeyset/sqla20.py | 4 ++++ tests/test_paging.py | 9 +++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/sqlakeyset/sqla20.py b/sqlakeyset/sqla20.py index 9e17514e..6fdf2be8 100644 --- a/sqlakeyset/sqla20.py +++ b/sqlakeyset/sqla20.py @@ -61,6 +61,10 @@ 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 + """ + if any(col._annotations for col in selectable._raw_columns): + return selectable._raw_columns[0] + """ return None diff --git a/tests/test_paging.py b/tests/test_paging.py index a80400af..007f055c 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -237,7 +237,7 @@ def check_multiple_paging_core(qs, s): for actual, expected in zip(t.gathered, t.unpaged): if len(expected) != len(actual): print(f"Expected type: {type(expected)} / Actual type: {type(actual)} / Expected[0] type: {type(expected[0])}") - assert expected[0] == actual + assert expected[0].__dict__ == actual.__dict__ else: assert actual == expected page_trackers.remove(t) @@ -568,10 +568,11 @@ def test_core(dburl): check_paging_core(selectable=selectable, s=s.connection()) -def test_core_whole_model(dburl): +def test_core_whole_models(dburl): selectable = ( - select(Book) - .where(Book.d == 99) + select(Book, Author) + .where(Book.id < 10) + .join(Author, Book.author_id == Author.id) .order_by(Book.id) ) From 02417827021577b9ed18529caf791f1d58e43ffb Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 10:13:20 -0700 Subject: [PATCH 099/159] Test multi-orm core. --- sqlakeyset/sqla20.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sqlakeyset/sqla20.py b/sqlakeyset/sqla20.py index 6fdf2be8..68ba325a 100644 --- a/sqlakeyset/sqla20.py +++ b/sqlakeyset/sqla20.py @@ -65,6 +65,8 @@ def core_result_type(selectable, s): if any(col._annotations for col in selectable._raw_columns): return selectable._raw_columns[0] """ + if hasattr(selectable, "_raw_columns"): + print(selectable._raw_columns[0]) return None From 36147a926aef8b9e0ad87ca908fb94baf9bb820f Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 10:29:11 -0700 Subject: [PATCH 100/159] Fix dict comparison. --- tests/test_paging.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 007f055c..bdf40219 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -237,7 +237,8 @@ def check_multiple_paging_core(qs, s): for actual, expected in zip(t.gathered, t.unpaged): if len(expected) != len(actual): print(f"Expected type: {type(expected)} / Actual type: {type(actual)} / Expected[0] type: {type(expected[0])}") - assert expected[0].__dict__ == actual.__dict__ + for k, v in expected[0].__dict__.items(): + assert v == getattr(actual, k) else: assert actual == expected page_trackers.remove(t) From 821e1387e6d3fcf2cb60d9449864b1b8de562f52 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 10:31:03 -0700 Subject: [PATCH 101/159] skip _sa_instance_state --- tests/test_paging.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_paging.py b/tests/test_paging.py index bdf40219..f022e0dd 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -238,6 +238,8 @@ def check_multiple_paging_core(qs, s): if len(expected) != len(actual): print(f"Expected type: {type(expected)} / Actual type: {type(actual)} / Expected[0] type: {type(expected[0])}") for k, v in expected[0].__dict__.items(): + if k == "_sa_instance_state": + continue assert v == getattr(actual, k) else: assert actual == expected From 87af81b738f25ae6033b6136fc037b193fe23f9c Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 10:34:39 -0700 Subject: [PATCH 102/159] Skip id too --- tests/test_paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index f022e0dd..e7ca1024 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -238,7 +238,7 @@ def check_multiple_paging_core(qs, s): if len(expected) != len(actual): print(f"Expected type: {type(expected)} / Actual type: {type(actual)} / Expected[0] type: {type(expected[0])}") for k, v in expected[0].__dict__.items(): - if k == "_sa_instance_state": + if k == "_sa_instance_state" or k == "id": continue assert v == getattr(actual, k) else: From bfd7746844bef02e64188111ed151b751737636e Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 10:42:15 -0700 Subject: [PATCH 103/159] Skip popularity too. --- tests/test_paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index e7ca1024..b9641367 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -238,7 +238,7 @@ def check_multiple_paging_core(qs, s): if len(expected) != len(actual): print(f"Expected type: {type(expected)} / Actual type: {type(actual)} / Expected[0] type: {type(expected[0])}") for k, v in expected[0].__dict__.items(): - if k == "_sa_instance_state" or k == "id": + if k == "_sa_instance_state" or k in {"id", "popularity"}: continue assert v == getattr(actual, k) else: From f912cfc1d80b2f4b7be6e8e8aabbcb4f134c6754 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 10:43:59 -0700 Subject: [PATCH 104/159] raw_columns print. --- sqlakeyset/sqla20.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/sqla20.py b/sqlakeyset/sqla20.py index 68ba325a..7a06987c 100644 --- a/sqlakeyset/sqla20.py +++ b/sqlakeyset/sqla20.py @@ -66,7 +66,7 @@ def core_result_type(selectable, s): return selectable._raw_columns[0] """ if hasattr(selectable, "_raw_columns"): - print(selectable._raw_columns[0]) + print(f"Raw columns: {selectable._raw_columns[0]}") return None From e16e162145bfb34a965370346333cda99e10bfac Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 10:48:32 -0700 Subject: [PATCH 105/159] Print better --- sqlakeyset/sqla20.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/sqla20.py b/sqlakeyset/sqla20.py index 7a06987c..41739561 100644 --- a/sqlakeyset/sqla20.py +++ b/sqlakeyset/sqla20.py @@ -66,7 +66,7 @@ def core_result_type(selectable, s): return selectable._raw_columns[0] """ if hasattr(selectable, "_raw_columns"): - print(f"Raw columns: {selectable._raw_columns[0]}") + print(f"Raw columns: {selectable._raw_columns}") return None From 3e23a8462f88a14deeaeae44d7a2bc15a317b11f Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 11:16:25 -0700 Subject: [PATCH 106/159] Use annotations. --- sqlakeyset/sqla20.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/sqla20.py b/sqlakeyset/sqla20.py index 41739561..d53e8b94 100644 --- a/sqlakeyset/sqla20.py +++ b/sqlakeyset/sqla20.py @@ -66,7 +66,11 @@ def core_result_type(selectable, s): return selectable._raw_columns[0] """ if hasattr(selectable, "_raw_columns"): - print(f"Raw columns: {selectable._raw_columns}") + for col in selectable._raw_columns: + print(f"Annotations: {col._annotations}") + if "parententity" in col._annotations: + return col._annotations["parententity"].entity + #print(f"Raw columns: {selectable._raw_columns}") return None @@ -106,6 +110,8 @@ def core_coerce_row(row: Row, extra_columns, result_type) -> Row: parent = row._parent processors = None # Processors are applied immediately in sqla1.4+ data = row._data[:N] + if result_type is not None: + print(f"result: {result_type(data)}") if hasattr(row, "_key_to_index"): # 2.0.11+ From 09e62b77ee0053c40eabd0e1f3094472360d4fb5 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 11:17:25 -0700 Subject: [PATCH 107/159] flake8 --- sqlakeyset/sqla20.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/sqla20.py b/sqlakeyset/sqla20.py index d53e8b94..7fbaf21d 100644 --- a/sqlakeyset/sqla20.py +++ b/sqlakeyset/sqla20.py @@ -70,7 +70,7 @@ def core_result_type(selectable, s): print(f"Annotations: {col._annotations}") if "parententity" in col._annotations: return col._annotations["parententity"].entity - #print(f"Raw columns: {selectable._raw_columns}") + # print(f"Raw columns: {selectable._raw_columns}") return None From b3f3dd0060989e8e8c83639e01a37dff565b91e5 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 11:23:00 -0700 Subject: [PATCH 108/159] Skip the .entity for mapper. --- sqlakeyset/sqla20.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/sqla20.py b/sqlakeyset/sqla20.py index 7fbaf21d..599549bf 100644 --- a/sqlakeyset/sqla20.py +++ b/sqlakeyset/sqla20.py @@ -69,7 +69,7 @@ def core_result_type(selectable, s): for col in selectable._raw_columns: print(f"Annotations: {col._annotations}") if "parententity" in col._annotations: - return col._annotations["parententity"].entity + return col._annotations["parententity"] # print(f"Raw columns: {selectable._raw_columns}") return None From 82b04e369a4d948861e2cbe34d82385f90e50e17 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 11:30:54 -0700 Subject: [PATCH 109/159] Only use mapper for ORM. --- sqlakeyset/sqla20.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sqlakeyset/sqla20.py b/sqlakeyset/sqla20.py index 599549bf..5cce95ad 100644 --- a/sqlakeyset/sqla20.py +++ b/sqlakeyset/sqla20.py @@ -3,6 +3,7 @@ from sqlalchemy.engine.result import result_tuple from sqlalchemy.engine.row import Row +from sqlalchemy.sql.schema import Table from .constants import ORDER_COL_PREFIX @@ -67,9 +68,11 @@ def core_result_type(selectable, s): """ if hasattr(selectable, "_raw_columns"): for col in selectable._raw_columns: + if not isinstance(col, Table): + continue print(f"Annotations: {col._annotations}") if "parententity" in col._annotations: - return col._annotations["parententity"] + return col._annotations["parententity"].entity # print(f"Raw columns: {selectable._raw_columns}") return None @@ -111,7 +114,7 @@ def core_coerce_row(row: Row, extra_columns, result_type) -> Row: processors = None # Processors are applied immediately in sqla1.4+ data = row._data[:N] if result_type is not None: - print(f"result: {result_type(data)}") + print(f"result: {result_type(*data)}") if hasattr(row, "_key_to_index"): # 2.0.11+ From bd19b1992d0f9eea6180ea08ffd95215ce636412 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 11:37:51 -0700 Subject: [PATCH 110/159] Skip annotations. --- sqlakeyset/sqla20.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sqlakeyset/sqla20.py b/sqlakeyset/sqla20.py index 5cce95ad..47d344ce 100644 --- a/sqlakeyset/sqla20.py +++ b/sqlakeyset/sqla20.py @@ -65,7 +65,6 @@ def core_result_type(selectable, s): """ if any(col._annotations for col in selectable._raw_columns): return selectable._raw_columns[0] - """ if hasattr(selectable, "_raw_columns"): for col in selectable._raw_columns: if not isinstance(col, Table): @@ -74,6 +73,7 @@ def core_result_type(selectable, s): if "parententity" in col._annotations: return col._annotations["parententity"].entity # print(f"Raw columns: {selectable._raw_columns}") + """ return None @@ -113,8 +113,6 @@ def core_coerce_row(row: Row, extra_columns, result_type) -> Row: parent = row._parent processors = None # Processors are applied immediately in sqla1.4+ data = row._data[:N] - if result_type is not None: - print(f"result: {result_type(*data)}") if hasattr(row, "_key_to_index"): # 2.0.11+ From e4710dc7c0ff3fa70316d4f272698b2707534361 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 11:39:47 -0700 Subject: [PATCH 111/159] flake8 --- sqlakeyset/sqla20.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sqlakeyset/sqla20.py b/sqlakeyset/sqla20.py index 47d344ce..09473225 100644 --- a/sqlakeyset/sqla20.py +++ b/sqlakeyset/sqla20.py @@ -3,7 +3,6 @@ from sqlalchemy.engine.result import result_tuple from sqlalchemy.engine.row import Row -from sqlalchemy.sql.schema import Table from .constants import ORDER_COL_PREFIX From a551b3a2ab47fa613c367bee95ac54895c1e6228 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 11:41:49 -0700 Subject: [PATCH 112/159] Skip test. --- tests/test_paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index b9641367..233780e4 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -512,6 +512,7 @@ def test_orm_multiple_pages_empty_queries(): assert get_homogeneous_pages([]) == [] +""" def test_core_multiple_pages(no_sqlite_dburl): # TODO: Add a test with an order by that adds an extra column. with S(no_sqlite_dburl, echo=ECHO) as s: @@ -523,7 +524,6 @@ def test_core_multiple_pages(no_sqlite_dburl): check_multiple_paging_core(qs=qs, s=s) -""" def test_core_multiple_pages_select_columns(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ From 4fe5643ed3b7d8dfa606204f26a3d0ab0f5f5caa Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:06:25 -0700 Subject: [PATCH 113/159] Try using select.from_statement --- sqlakeyset/paging.py | 18 +++++++++++++++--- tests/test_paging.py | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 2210826e..0195e7c1 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -26,6 +26,7 @@ from sqlalchemy.orm.query import Query from sqlalchemy.sql.expression import ColumnElement, literal, select, union_all from sqlalchemy.sql.selectable import Select +from sqlalchemy.sql.schema import Table from .columns import OC, MappedOrderColumn, find_order_key, parse_ob_clause from .results import Page, Paging, unserialize_bookmark @@ -540,18 +541,27 @@ def select_homogeneous_pages( selectable = union_all( *[p.paging_query.select for p in prepared_queries] - ) + ).order_by(text("_page_identifier"), text("_row_number")) + def get_columns(selectable): + for col in selectable._raw_columns: + if isinstance(col, Table): + yield col._annotations["parententity"].entity + else: + yield col """ if len(requests) > 1: selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) """ compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select statement: {compiled}") + columns = list(get_columns(prepared_queries[0].paging_query.select)) + [text("_page_identifier")] + selectable = select(*columns).from_statement(selectable) """ selectable = select(*[col for col in selectable.columns]).select_from(selectable) selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) - """ + selectable = select(selectable.subquery()).order_by(text("_page_identifier"), text("_row_number")) + """ compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select from statement: {compiled}") selected = s.execute(selectable) @@ -569,11 +579,13 @@ def select_homogeneous_pages( # a bunch of selects, it changes the "keys" on us in cases where the column # name and python attribute don't match. So we have to execute the first # query standalone to ge the correct keys. + """ subselect_result = ( s.execute(prepared_queries[0].paging_query.select) if len(prepared_queries) > 1 else selected ) - keys = list(subselect_result.keys()) + """ + keys = list(selected.keys()) print(f"pre-shrunk keys: {keys}") N = len(keys) - len(prepared_queries[0].paging_query.extra_columns) keys = keys[:N] diff --git a/tests/test_paging.py b/tests/test_paging.py index 233780e4..b9641367 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -512,7 +512,6 @@ def test_orm_multiple_pages_empty_queries(): assert get_homogeneous_pages([]) == [] -""" def test_core_multiple_pages(no_sqlite_dburl): # TODO: Add a test with an order by that adds an extra column. with S(no_sqlite_dburl, echo=ECHO) as s: @@ -524,6 +523,7 @@ def test_core_multiple_pages(no_sqlite_dburl): check_multiple_paging_core(qs=qs, s=s) +""" def test_core_multiple_pages_select_columns(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ From a30ffb11c4aaf86aa9bdf01b73b64bb111ba8899 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:13:01 -0700 Subject: [PATCH 114/159] flake8 --- sqlakeyset/paging.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 0195e7c1..48ae9ddb 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -542,6 +542,7 @@ def select_homogeneous_pages( selectable = union_all( *[p.paging_query.select for p in prepared_queries] ).order_by(text("_page_identifier"), text("_row_number")) + def get_columns(selectable): for col in selectable._raw_columns: if isinstance(col, Table): @@ -559,7 +560,7 @@ def get_columns(selectable): """ selectable = select(*[col for col in selectable.columns]).select_from(selectable) selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) - + selectable = select(selectable.subquery()).order_by(text("_page_identifier"), text("_row_number")) """ compiled = selectable.compile(compile_kwargs={"literal_binds": True}) From 507f4985c45bf89a0c0b0587b8897f428754d795 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:23:03 -0700 Subject: [PATCH 115/159] Skip order by? --- sqlakeyset/paging.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 48ae9ddb..85f0600e 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -541,7 +541,8 @@ def select_homogeneous_pages( selectable = union_all( *[p.paging_query.select for p in prepared_queries] - ).order_by(text("_page_identifier"), text("_row_number")) + ) + # .order_by(text("_page_identifier"), text("_row_number")) def get_columns(selectable): for col in selectable._raw_columns: @@ -556,6 +557,7 @@ def get_columns(selectable): compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select statement: {compiled}") columns = list(get_columns(prepared_queries[0].paging_query.select)) + [text("_page_identifier")] + print(columns) selectable = select(*columns).from_statement(selectable) """ selectable = select(*[col for col in selectable.columns]).select_from(selectable) From 8bd589f14ebca63a53446fe0f885e18ecd44d44b Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:30:13 -0700 Subject: [PATCH 116/159] Use the correct selectable. --- sqlakeyset/paging.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 85f0600e..25c0ba3b 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -541,8 +541,7 @@ def select_homogeneous_pages( selectable = union_all( *[p.paging_query.select for p in prepared_queries] - ) - # .order_by(text("_page_identifier"), text("_row_number")) + ).order_by(text("_page_identifier"), text("_row_number")) def get_columns(selectable): for col in selectable._raw_columns: @@ -556,7 +555,7 @@ def get_columns(selectable): """ compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select statement: {compiled}") - columns = list(get_columns(prepared_queries[0].paging_query.select)) + [text("_page_identifier")] + columns = list(get_columns(requests[0].selectable)) + [text("_page_identifier")] print(columns) selectable = select(*columns).from_statement(selectable) """ From 2d54d086fa3dd55ea2a8f366bc5b19950d750b21 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:33:03 -0700 Subject: [PATCH 117/159] Try not selecting _page_identifier. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 25c0ba3b..eb497632 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -555,7 +555,7 @@ def get_columns(selectable): """ compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select statement: {compiled}") - columns = list(get_columns(requests[0].selectable)) + [text("_page_identifier")] + columns = list(get_columns(requests[0].selectable)) print(columns) selectable = select(*columns).from_statement(selectable) """ From 1842d3cd5b108786a123655a9f6985b57d5da5b7 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:35:42 -0700 Subject: [PATCH 118/159] Try using prepared_queries instead. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index eb497632..d3741fe1 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -555,7 +555,7 @@ def get_columns(selectable): """ compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select statement: {compiled}") - columns = list(get_columns(requests[0].selectable)) + columns = list(get_columns(prepared_queries[0].paging_query.select)) print(columns) selectable = select(*columns).from_statement(selectable) """ From a56ed32a42c678611df12f1d806aa8b796f52d4a Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:38:11 -0700 Subject: [PATCH 119/159] Get rid of corresponding_column nonsense. --- sqlakeyset/paging.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index d3741fe1..4c5eb54f 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -598,6 +598,7 @@ def get_columns(selectable): rows = page_to_rows[i] prepared_query = prepared_queries[i] old_sel = prepared_query.paging_query + """ mapped_order_columns = [] for ocol in prepared_query.paging_query.order_columns: corresponding_column = selectable.corresponding_column(ocol.element) @@ -606,6 +607,8 @@ def get_columns(selectable): print(f"key_rows: {key_rows}") sel = _PagingSelect(old_sel.select, old_sel.order_columns, mapped_order_columns, old_sel.extra_columns) pages.append(prepared_queries[i].page_from_rows(rows, sel, keys)) + """ + pages.append(prepared_queries[i].page_from_rows(rows, old_sel, keys)) return pages From e4fd214bbd066a579c8c98b74179e99d6672c5bd Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:48:07 -0700 Subject: [PATCH 120/159] Special case len(requests) == 1. --- sqlakeyset/paging.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 4c5eb54f..dc40f23d 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -536,6 +536,22 @@ def select_homogeneous_pages( """ if not requests: return [] + + if len(requests) == 1: + # Handling 1 request is annoying because of its effect on union_all, + # so it's easier to just farm it out. + request = requests[0] + return [ + select_page( + request.s, + request.selectable, + per_page=request.per_page, + after=request.after, + before=request.before, + page=request.page + ) + ] + prepared_queries = [_core_prepare_homogeneous_page(request, s, i) for i, request in enumerate(requests)] From 8f39aef09844d562d0b9114e75e491ab0d09a155 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:49:08 -0700 Subject: [PATCH 121/159] flake8 --- sqlakeyset/paging.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index dc40f23d..644c69e5 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -536,7 +536,7 @@ def select_homogeneous_pages( """ if not requests: return [] - + if len(requests) == 1: # Handling 1 request is annoying because of its effect on union_all, # so it's easier to just farm it out. @@ -552,7 +552,6 @@ def select_homogeneous_pages( ) ] - prepared_queries = [_core_prepare_homogeneous_page(request, s, i) for i, request in enumerate(requests)] selectable = union_all( From 24f21a3924d399b78d3e6f8ef000ac06224113aa Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:50:34 -0700 Subject: [PATCH 122/159] fix symbol. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 644c69e5..eb06dbee 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -543,7 +543,7 @@ def select_homogeneous_pages( request = requests[0] return [ select_page( - request.s, + s, request.selectable, per_page=request.per_page, after=request.after, From aad8bc958b45e6c89ec6cccbb54fc21343d73116 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:53:03 -0700 Subject: [PATCH 123/159] Do we even need _annotations? --- sqlakeyset/paging.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index eb06dbee..3ae9559f 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -559,11 +559,14 @@ def select_homogeneous_pages( ).order_by(text("_page_identifier"), text("_row_number")) def get_columns(selectable): + """ for col in selectable._raw_columns: if isinstance(col, Table): yield col._annotations["parententity"].entity else: yield col + """ + return selectable._raw_columns """ if len(requests) > 1: selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) From 1b81cbedcacb7d541d45f3eb6a10badd4af25143 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:54:19 -0700 Subject: [PATCH 124/159] Use Table. --- sqlakeyset/paging.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 3ae9559f..a2bf0002 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -566,6 +566,7 @@ def get_columns(selectable): else: yield col """ + _ = Table return selectable._raw_columns """ if len(requests) > 1: From 3760d88dc26bbfbffadcd3a2081c710fc251cc66 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 16:59:08 -0700 Subject: [PATCH 125/159] Comment out tests to use test_core_whole_models. --- sqlakeyset/paging.py | 12 +----------- tests/test_paging.py | 7 ++++--- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index a2bf0002..77d2371a 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -558,23 +558,13 @@ def select_homogeneous_pages( *[p.paging_query.select for p in prepared_queries] ).order_by(text("_page_identifier"), text("_row_number")) - def get_columns(selectable): - """ - for col in selectable._raw_columns: - if isinstance(col, Table): - yield col._annotations["parententity"].entity - else: - yield col - """ - _ = Table - return selectable._raw_columns """ if len(requests) > 1: selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) """ compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select statement: {compiled}") - columns = list(get_columns(prepared_queries[0].paging_query.select)) + columns = prepared_queries[0].paging_query.select._raw_columns print(columns) selectable = select(*columns).from_statement(selectable) """ diff --git a/tests/test_paging.py b/tests/test_paging.py index b9641367..b81d1769 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -25,6 +25,7 @@ desc, func, ) +from sqlalchemy.sql.expression import literal from sqlalchemy.sql.selectable import Select from sqlakeyset import ( @@ -512,6 +513,7 @@ def test_orm_multiple_pages_empty_queries(): assert get_homogeneous_pages([]) == [] +""" def test_core_multiple_pages(no_sqlite_dburl): # TODO: Add a test with an order by that adds an extra column. with S(no_sqlite_dburl, echo=ECHO) as s: @@ -523,7 +525,6 @@ def test_core_multiple_pages(no_sqlite_dburl): check_multiple_paging_core(qs=qs, s=s) -""" def test_core_multiple_pages_select_columns(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ @@ -573,9 +574,9 @@ def test_core(dburl): def test_core_whole_models(dburl): selectable = ( - select(Book, Author) + select(Book, Author, literal(0)) .where(Book.id < 10) - .join(Author, Book.author_id == Author.id) + .join(Author, Book.author_id == Author.id, isouter=True) .order_by(Book.id) ) From d7a2f5c93f1db817ff57d03fc20a11f66269e339 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 17:00:09 -0700 Subject: [PATCH 126/159] flake8 --- sqlakeyset/paging.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 77d2371a..7e10bf67 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -26,7 +26,6 @@ from sqlalchemy.orm.query import Query from sqlalchemy.sql.expression import ColumnElement, literal, select, union_all from sqlalchemy.sql.selectable import Select -from sqlalchemy.sql.schema import Table from .columns import OC, MappedOrderColumn, find_order_key, parse_ob_clause from .results import Page, Paging, unserialize_bookmark From b82baed58a5ea1f5673210a5cb1ad6eb6725d7ce Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 17:04:37 -0700 Subject: [PATCH 127/159] Remove join in test. --- tests/test_paging.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index b81d1769..d7be4727 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -574,9 +574,10 @@ def test_core(dburl): def test_core_whole_models(dburl): selectable = ( - select(Book, Author, literal(0)) + # Consider joining against another table. Need to figure out + # 1.3 compatible syntax though. + select(Book, literal(0)) .where(Book.id < 10) - .join(Author, Book.author_id == Author.id, isouter=True) .order_by(Book.id) ) From bc1cd2a8e3984701bc279b775802f682b63aee7c Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 17:10:44 -0700 Subject: [PATCH 128/159] Run full test suite. --- tests/test_paging.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index d7be4727..4d097fe8 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -41,6 +41,7 @@ ) from sqlakeyset.paging import process_args from sqlakeyset.results import Page +from sqlakeyset.sqla import SQLA_VERSION from sqlakeyset.types import MarkerLike from conftest import ( Book, @@ -513,7 +514,10 @@ def test_orm_multiple_pages_empty_queries(): assert get_homogeneous_pages([]) == [] -""" +@pytest.mark.skipif( + SQLA_VERSION < version.parse("1.4.0b1"), + reason="Broken in 1.3." +) def test_core_multiple_pages(no_sqlite_dburl): # TODO: Add a test with an order by that adds an extra column. with S(no_sqlite_dburl, echo=ECHO) as s: @@ -549,7 +553,6 @@ def test_core_multiple_pages_one_query_whole_model(no_sqlite_dburl): select(Book).order_by(Book.id), ] check_multiple_paging_core(qs=qs, s=s) -""" def test_core_multiple_pages_empty_queries(no_sqlite_dburl): @@ -572,10 +575,12 @@ def test_core(dburl): check_paging_core(selectable=selectable, s=s.connection()) -def test_core_whole_models(dburl): +@pytest.mark.skipif( + SQLA_VERSION < version.parse("1.4.0b1"), + reason="Broken in 1.3." +) +def test_core_whole_model_plus_other_columns(dburl): selectable = ( - # Consider joining against another table. Need to figure out - # 1.3 compatible syntax though. select(Book, literal(0)) .where(Book.id < 10) .order_by(Book.id) From 8028a14df3088980a217610cd4b3d8e8c67433c6 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Wed, 19 Jul 2023 17:16:00 -0700 Subject: [PATCH 129/159] Finish skipping the rest of the tests. --- tests/test_paging.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 4d097fe8..c12316ee 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -516,7 +516,7 @@ def test_orm_multiple_pages_empty_queries(): @pytest.mark.skipif( SQLA_VERSION < version.parse("1.4.0b1"), - reason="Broken in 1.3." + reason="Not supported in 1.3." ) def test_core_multiple_pages(no_sqlite_dburl): # TODO: Add a test with an order by that adds an extra column. @@ -529,6 +529,10 @@ def test_core_multiple_pages(no_sqlite_dburl): check_multiple_paging_core(qs=qs, s=s) +@pytest.mark.skipif( + SQLA_VERSION < version.parse("1.4.0b1"), + reason="Not supported in 1.3." +) def test_core_multiple_pages_select_columns(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ @@ -539,6 +543,10 @@ def test_core_multiple_pages_select_columns(no_sqlite_dburl): check_multiple_paging_core(qs=qs, s=s) +@pytest.mark.skipif( + SQLA_VERSION < version.parse("1.4.0b1"), + reason="Not supported in 1.3." +) def test_core_multiple_pages_one_query(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ @@ -547,6 +555,10 @@ def test_core_multiple_pages_one_query(no_sqlite_dburl): check_multiple_paging_core(qs=qs, s=s) +@pytest.mark.skipif( + SQLA_VERSION < version.parse("1.4.0b1"), + reason="Not supported in 1.3." +) def test_core_multiple_pages_one_query_whole_model(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ From cf069420f5904db4db80b11e56129b393f5ff361 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 10:09:25 -0700 Subject: [PATCH 130/159] Add a test that tests different extra columns. --- tests/test_paging.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_paging.py b/tests/test_paging.py index c12316ee..4a804fce 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -543,6 +543,20 @@ def test_core_multiple_pages_select_columns(no_sqlite_dburl): check_multiple_paging_core(qs=qs, s=s) +@pytest.mark.skipif( + SQLA_VERSION < version.parse("1.4.0b1"), + reason="Not supported in 1.3." +) +def test_core_multiple_pages_different_extra_columns(no_sqlite_dburl): + with S(no_sqlite_dburl, echo=ECHO) as s: + qs = [ + select(Book.name).order_by(Book.b), + select(Book.name).order_by(Book.id), + select(Book.name).order_by(Book.c), + ] + check_multiple_paging_core(qs=qs, s=s) + + @pytest.mark.skipif( SQLA_VERSION < version.parse("1.4.0b1"), reason="Not supported in 1.3." From bd600854e0ca8dfddc3d55f1ed55b92a728dbe71 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 11:00:11 -0700 Subject: [PATCH 131/159] Try using same extra columns everywhere. --- sqlakeyset/paging.py | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 7e10bf67..f357146c 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -180,6 +180,7 @@ def prepare_paging( backwards: bool, orm: bool, dialect: Dialect, + extra_columns: Optional[List[ColumnElement]] = None, page_identifier: Optional[int] = None, ) -> Union[_PagingQuery, _PagingSelect]: if orm: @@ -206,9 +207,10 @@ def prepare_paging( if orm: q = q.only_return_tuples(True) # type: ignore - extra_columns = [ - col.extra_column for col in mapped_ocols if col.extra_column is not None - ] + if extra_columns is None: + extra_columns = [ + col.extra_column for col in mapped_ocols if col.extra_column is not None + ] # page_identifier is used for fetching multiple pages. if page_identifier is not None: @@ -551,7 +553,22 @@ def select_homogeneous_pages( ) ] - prepared_queries = [_core_prepare_homogeneous_page(request, s, i) for i, request in enumerate(requests)] + extra_columns: Set[OC] = set() + for request in requests: + order_cols = parse_ob_clause(request.selectable) + place, backwards = process_args(request.after, request.before, request.page) + if request.backwards: + order_cols = [c.reversed for c in order_cols] + mapped_ocols = [find_order_key(ocol, column_descriptions) for ocol in order_cols] + for col in mapped_cols: + if col.extra_column is None: + continue + oc = OC(col.extra_column) + if not any(oc.quoted_full_name == c.quoted_full_name for c in extra_columns): + extra_columns.add(oc) + extra_columns = list(col.element for col in extra_columns) + + prepared_queries = [_core_prepare_homogeneous_page(request, s, extra_columns, i) for i, request in enumerate(requests)] selectable = union_all( *[p.paging_query.select for p in prepared_queries] @@ -627,7 +644,10 @@ class _PreparedQuery: def _core_prepare_homogeneous_page( - request: PageRequest[_TP], s: Union[Session, Connection], page_identifier: int + request: PageRequest[_TP], + s: Union[Session, Connection], + extra_columns: list[ColumnElement], + page_identifier: int ) -> _PreparedQuery: place, backwards = process_args(request.after, request.before, request.page) @@ -640,6 +660,7 @@ def _core_prepare_homogeneous_page( backwards=backwards, orm=False, dialect=get_bind(q=selectable, s=s).dialect, + extra_columns=extra_columns, page_identifier=page_identifier, ) From c6673543c62ad32452fe94c576740157b40f14b0 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 11:04:20 -0700 Subject: [PATCH 132/159] flake8 --- sqlakeyset/paging.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index f357146c..052589f5 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -553,14 +553,24 @@ def select_homogeneous_pages( ) ] - extra_columns: Set[OC] = set() + # Because UNION ALL requires identical SELECT statements, but we allow different + # order_bys which could result in different extra columns for order keys, we need + # to first find the superset of extra columns and then add those to ever single + # selectable. + + + extra_columns: set[OC] = set() for request in requests: + try: + column_descriptions = request.selectable.column_descriptions + except Exception: + column_descriptions = request.selectable._raw_columns # type: ignore order_cols = parse_ob_clause(request.selectable) place, backwards = process_args(request.after, request.before, request.page) if request.backwards: order_cols = [c.reversed for c in order_cols] mapped_ocols = [find_order_key(ocol, column_descriptions) for ocol in order_cols] - for col in mapped_cols: + for col in mapped_ocols: if col.extra_column is None: continue oc = OC(col.extra_column) From 1ec908913322cdb23da6f9a51f89974596ec334f Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 11:21:49 -0700 Subject: [PATCH 133/159] flake8 --- sqlakeyset/paging.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 052589f5..2ef5bad0 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -558,7 +558,6 @@ def select_homogeneous_pages( # to first find the superset of extra columns and then add those to ever single # selectable. - extra_columns: set[OC] = set() for request in requests: try: From f0690f4f9167eaf11de15c79aacffa578e87844e Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 11:41:14 -0700 Subject: [PATCH 134/159] Fix backwards. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 2ef5bad0..86a0c7f3 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -566,7 +566,7 @@ def select_homogeneous_pages( column_descriptions = request.selectable._raw_columns # type: ignore order_cols = parse_ob_clause(request.selectable) place, backwards = process_args(request.after, request.before, request.page) - if request.backwards: + if backwards: order_cols = [c.reversed for c in order_cols] mapped_ocols = [find_order_key(ocol, column_descriptions) for ocol in order_cols] for col in mapped_ocols: From aac2e3ee4b33b64ccebeececadd47757fe7f1ed6 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 11:48:32 -0700 Subject: [PATCH 135/159] Switch from set to list and add prints. --- sqlakeyset/paging.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 86a0c7f3..9e1746fb 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -222,6 +222,7 @@ def prepare_paging( ] if hasattr(q, "add_columns"): # ORM or SQLAlchemy 1.4+ + print(f"Adding extra columns: {extra_columns}") q = q.add_columns(*extra_columns) else: for col in extra_columns: # SQLAlchemy Core <1.4 @@ -558,7 +559,7 @@ def select_homogeneous_pages( # to first find the superset of extra columns and then add those to ever single # selectable. - extra_columns: set[OC] = set() + extra_columns = [] for request in requests: try: column_descriptions = request.selectable.column_descriptions @@ -574,9 +575,10 @@ def select_homogeneous_pages( continue oc = OC(col.extra_column) if not any(oc.quoted_full_name == c.quoted_full_name for c in extra_columns): - extra_columns.add(oc) - extra_columns = list(col.element for col in extra_columns) + extra_columns.append(oc) + extra_columns = [col.element for col in extra_columns] + print(f"Extra columns: {extra_columns}") prepared_queries = [_core_prepare_homogeneous_page(request, s, extra_columns, i) for i, request in enumerate(requests)] selectable = union_all( From 6047c6470996f186af006f5eddd9d28f3a56af0e Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 11:53:06 -0700 Subject: [PATCH 136/159] Copy extra columns. --- sqlakeyset/paging.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 9e1746fb..0f1b9e92 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -180,7 +180,7 @@ def prepare_paging( backwards: bool, orm: bool, dialect: Dialect, - extra_columns: Optional[List[ColumnElement]] = None, + extra_columns_override: Optional[List[ColumnElement]] = None, page_identifier: Optional[int] = None, ) -> Union[_PagingQuery, _PagingSelect]: if orm: @@ -211,6 +211,9 @@ def prepare_paging( extra_columns = [ col.extra_column for col in mapped_ocols if col.extra_column is not None ] + else: + # Important to copy this list, as we modify later. + extra_columns = list(extra_columns_override) # page_identifier is used for fetching multiple pages. if page_identifier is not None: From 787436d9c829d0954f468c2777b62cb37633927a Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 12:29:57 -0700 Subject: [PATCH 137/159] Fix naming. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 0f1b9e92..4e8d4d2d 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -674,7 +674,7 @@ def _core_prepare_homogeneous_page( backwards=backwards, orm=False, dialect=get_bind(q=selectable, s=s).dialect, - extra_columns=extra_columns, + extra_columns_override=extra_columns, page_identifier=page_identifier, ) From 30f90e3a19ea4f052e3411ca3566980ba2b53f9c Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 13:06:46 -0700 Subject: [PATCH 138/159] fix name. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 4e8d4d2d..682bedbd 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -207,7 +207,7 @@ def prepare_paging( if orm: q = q.only_return_tuples(True) # type: ignore - if extra_columns is None: + if extra_columns_override is None: extra_columns = [ col.extra_column for col in mapped_ocols if col.extra_column is not None ] From 2b4da2924a0a2f5a01f5e8bba8a3e321781ffb68 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 13:45:39 -0700 Subject: [PATCH 139/159] Refactor. --- sqlakeyset/paging.py | 80 ++++++++++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 682bedbd..76b461b4 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -180,7 +180,6 @@ def prepare_paging( backwards: bool, orm: bool, dialect: Dialect, - extra_columns_override: Optional[List[ColumnElement]] = None, page_identifier: Optional[int] = None, ) -> Union[_PagingQuery, _PagingSelect]: if orm: @@ -207,13 +206,9 @@ def prepare_paging( if orm: q = q.only_return_tuples(True) # type: ignore - if extra_columns_override is None: - extra_columns = [ - col.extra_column for col in mapped_ocols if col.extra_column is not None - ] - else: - # Important to copy this list, as we modify later. - extra_columns = list(extra_columns_override) + extra_columns = [ + col.extra_column for col in mapped_ocols if col.extra_column is not None + ] # page_identifier is used for fetching multiple pages. if page_identifier is not None: @@ -231,6 +226,17 @@ def prepare_paging( for col in extra_columns: # SQLAlchemy Core <1.4 q = q.column(col) # type: ignore + q = _apply_where_and_limit(q, selectable, per_page, place, dialect, order_cols) + + if orm: + assert isinstance(q, Query) + return _PagingQuery(q, order_cols, mapped_ocols, extra_columns) + else: + assert not isinstance(q, Query) + return _PagingSelect(q, order_cols, mapped_ocols, extra_columns) + + +def _apply_where_and_limit(q, selectable, per_page, place, dialect, order_cols): if place: condition = where_condition_for_page(order_cols, place, dialect) # For aggregate queries, paging condition is applied *after* @@ -245,12 +251,7 @@ def prepare_paging( q = q.where(condition) q = q.limit(per_page + 1) # 1 extra to check if there's a further page - if orm: - assert isinstance(q, Query) - return _PagingQuery(q, order_cols, mapped_ocols, extra_columns) - else: - assert not isinstance(q, Query) - return _PagingSelect(q, order_cols, mapped_ocols, extra_columns) + return q def orm_get_page( @@ -562,7 +563,9 @@ def select_homogeneous_pages( # to first find the superset of extra columns and then add those to ever single # selectable. - extra_columns = [] + extra_column_mappers: dict[str, MappedOrderColumn] = {} + mapped_order_columns_per_request = [] + order_cols_per_request = [] for request in requests: try: column_descriptions = request.selectable.column_descriptions @@ -572,17 +575,24 @@ def select_homogeneous_pages( place, backwards = process_args(request.after, request.before, request.page) if backwards: order_cols = [c.reversed for c in order_cols] + order_cols_per_request.append(order_cols) mapped_ocols = [find_order_key(ocol, column_descriptions) for ocol in order_cols] + for i, col in enumerate(list(mapped_ocols)): + if col.quoted_full_name in extra_column_mappers: + mapped_ocols[i] = extra_column_mappers[col.quoted_full_name] + mapped_order_columns_per_request.append(mapped_ocols) + for col in mapped_ocols: if col.extra_column is None: continue - oc = OC(col.extra_column) - if not any(oc.quoted_full_name == c.quoted_full_name for c in extra_columns): - extra_columns.append(oc) - extra_columns = [col.element for col in extra_columns] + name = OC(col.extra_column).quoted_full_name + if name not in extra_column_mappers: + extra_column_mappers[name] = col + + extra_columns = [col.extra_column for col in extra_column_mappers.values()] print(f"Extra columns: {extra_columns}") - prepared_queries = [_core_prepare_homogeneous_page(request, s, extra_columns, i) for i, request in enumerate(requests)] + prepared_queries = [_core_prepare_homogeneous_page(request, s, order_cols_per_request[i], mapped_order_columns_per_request[i], extra_columns, i) for i, request in enumerate(requests)] selectable = union_all( *[p.paging_query.select for p in prepared_queries] @@ -660,6 +670,8 @@ class _PreparedQuery: def _core_prepare_homogeneous_page( request: PageRequest[_TP], s: Union[Session, Connection], + order_cols: list[OC], + mapped_ocols: list[MappedOrderColumn] extra_columns: list[ColumnElement], page_identifier: int ) -> _PreparedQuery: @@ -667,16 +679,26 @@ def _core_prepare_homogeneous_page( selectable = request.selectable result_type = core_result_type(selectable, s) - sel = prepare_paging( - q=selectable, - per_page=request.per_page, - place=place, - backwards=backwards, - orm=False, - dialect=get_bind(q=selectable, s=s).dialect, - extra_columns_override=extra_columns, - page_identifier=page_identifier, + + clauses = [col.ob_clause for col in mapped_ocols] + selectable = selectable.order_by(None).order_by(*clauses) + + extra_columns += [ + literal(page_identifier).label("_page_identifier"), + func.ROW_NUMBER().over( + order_by=[c.uo for c in order_cols] + ).label("_row_number"), + ] + selectable = selectable.add_columns(*extra_columns) + selectable = _apply_where_and_limit( + selectable, + selectable, + request.per_page, + place, + get_bind(q=selectable, s=s).dialect, + order_cols ) + sel = _PagingSelect(selectable, order_cols, mapped_ocols, extra_columns) def page_from_rows(rows, paging_select, keys): page = core_page_from_rows( From 238c442fc7ef8f728f8fd0971413cbaa115931ac Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 13:47:34 -0700 Subject: [PATCH 140/159] flake8 --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 76b461b4..c7d44d21 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -671,7 +671,7 @@ def _core_prepare_homogeneous_page( request: PageRequest[_TP], s: Union[Session, Connection], order_cols: list[OC], - mapped_ocols: list[MappedOrderColumn] + mapped_ocols: list[MappedOrderColumn], extra_columns: list[ColumnElement], page_identifier: int ) -> _PreparedQuery: From ce37b0849e1340385c833346a7c94215034d87d0 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 13:49:07 -0700 Subject: [PATCH 141/159] Undefined orm. --- sqlakeyset/paging.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index c7d44d21..6f5d1b4f 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -226,7 +226,7 @@ def prepare_paging( for col in extra_columns: # SQLAlchemy Core <1.4 q = q.column(col) # type: ignore - q = _apply_where_and_limit(q, selectable, per_page, place, dialect, order_cols) + q = _apply_where_and_limit(q, selectable, per_page, place, dialect, order_cols, orm) if orm: assert isinstance(q, Query) @@ -236,7 +236,7 @@ def prepare_paging( return _PagingSelect(q, order_cols, mapped_ocols, extra_columns) -def _apply_where_and_limit(q, selectable, per_page, place, dialect, order_cols): +def _apply_where_and_limit(q, selectable, per_page, place, dialect, order_cols, orm): if place: condition = where_condition_for_page(order_cols, place, dialect) # For aggregate queries, paging condition is applied *after* @@ -696,7 +696,8 @@ def _core_prepare_homogeneous_page( request.per_page, place, get_bind(q=selectable, s=s).dialect, - order_cols + order_cols, + orm=False ) sel = _PagingSelect(selectable, order_cols, mapped_ocols, extra_columns) From 52e7d4d70fd9130c2735b990b5204e8c86fd48e3 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 13:53:27 -0700 Subject: [PATCH 142/159] Fix typing. --- sqlakeyset/paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 6f5d1b4f..a8b0caba 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -578,8 +578,8 @@ def select_homogeneous_pages( order_cols_per_request.append(order_cols) mapped_ocols = [find_order_key(ocol, column_descriptions) for ocol in order_cols] for i, col in enumerate(list(mapped_ocols)): - if col.quoted_full_name in extra_column_mappers: - mapped_ocols[i] = extra_column_mappers[col.quoted_full_name] + if col.extra_column is not None and col.oc.quoted_full_name in extra_column_mappers: + mapped_ocols[i] = extra_column_mappers[col.oc.quoted_full_name] mapped_order_columns_per_request.append(mapped_ocols) for col in mapped_ocols: From 8c3ed82a488bc63d9e65f4f9fc8f1de65f703a6b Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 14:01:13 -0700 Subject: [PATCH 143/159] Same list copy mistake. --- sqlakeyset/paging.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index a8b0caba..972d1720 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -578,16 +578,14 @@ def select_homogeneous_pages( order_cols_per_request.append(order_cols) mapped_ocols = [find_order_key(ocol, column_descriptions) for ocol in order_cols] for i, col in enumerate(list(mapped_ocols)): - if col.extra_column is not None and col.oc.quoted_full_name in extra_column_mappers: - mapped_ocols[i] = extra_column_mappers[col.oc.quoted_full_name] - mapped_order_columns_per_request.append(mapped_ocols) - - for col in mapped_ocols: if col.extra_column is None: continue name = OC(col.extra_column).quoted_full_name - if name not in extra_column_mappers: + if name in extra_column_mappers: + mapped_ocols[i] = extra_column_mappers[name] + else: extra_column_mappers[name] = col + mapped_order_columns_per_request.append(mapped_ocols) extra_columns = [col.extra_column for col in extra_column_mappers.values()] @@ -683,7 +681,7 @@ def _core_prepare_homogeneous_page( clauses = [col.ob_clause for col in mapped_ocols] selectable = selectable.order_by(None).order_by(*clauses) - extra_columns += [ + extra_columns = list(extra_columns) + [ literal(page_identifier).label("_page_identifier"), func.ROW_NUMBER().over( order_by=[c.uo for c in order_cols] From 9dd17c37eb139f9e8c5f7fa8137841139b2bd6ec Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 14:07:38 -0700 Subject: [PATCH 144/159] Needs distinct ordering. --- tests/test_paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 4a804fce..fd0f3360 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -550,9 +550,9 @@ def test_core_multiple_pages_select_columns(no_sqlite_dburl): def test_core_multiple_pages_different_extra_columns(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ - select(Book.name).order_by(Book.b), + select(Book.name).order_by(Book.b, book.id), select(Book.name).order_by(Book.id), - select(Book.name).order_by(Book.c), + select(Book.name).order_by(Book.c, book.id), ] check_multiple_paging_core(qs=qs, s=s) From 91f86984b7a67cdeffdb6d702ad94030558aa19e Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 14:11:54 -0700 Subject: [PATCH 145/159] Flake8 --- tests/test_paging.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index fd0f3360..d1ff13aa 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -550,9 +550,9 @@ def test_core_multiple_pages_select_columns(no_sqlite_dburl): def test_core_multiple_pages_different_extra_columns(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ - select(Book.name).order_by(Book.b, book.id), + select(Book.name).order_by(Book.b, Book.id), select(Book.name).order_by(Book.id), - select(Book.name).order_by(Book.c, book.id), + select(Book.name).order_by(Book.c, Book.id), ] check_multiple_paging_core(qs=qs, s=s) From 473c6a7ba37e6930dceff70432cf79d6610f7d19 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 14:15:55 -0700 Subject: [PATCH 146/159] Fix assertion. --- tests/test_paging.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index d1ff13aa..778f07b3 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -236,15 +236,7 @@ def check_multiple_paging_core(qs, s): page = assert_paging_core(t.page_with_paging, t.gathered, t.backwards, t.selected, t.page, i + 1) if page is None: # Ensure union of pages is original q.all() - for actual, expected in zip(t.gathered, t.unpaged): - if len(expected) != len(actual): - print(f"Expected type: {type(expected)} / Actual type: {type(actual)} / Expected[0] type: {type(expected[0])}") - for k, v in expected[0].__dict__.items(): - if k == "_sa_instance_state" or k in {"id", "popularity"}: - continue - assert v == getattr(actual, k) - else: - assert actual == expected + assert list(t.gathered) == t.unpaged page_trackers.remove(t) continue From 57df5bc925891978699f9cdfc13f85d9482460ae Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 14:50:09 -0700 Subject: [PATCH 147/159] Preserve ordering in cached mappers. --- sqlakeyset/paging.py | 4 +++- tests/test_paging.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 972d1720..7cd3324a 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -582,7 +582,9 @@ def select_homogeneous_pages( continue name = OC(col.extra_column).quoted_full_name if name in extra_column_mappers: - mapped_ocols[i] = extra_column_mappers[name] + mapped_ocols[i] = copy(extra_column_mappers[name]) + # Fix up oc to make sure the ordering is correct. + mapped_ocols[i].oc = order_cols[i] else: extra_column_mappers[name] = col mapped_order_columns_per_request.append(mapped_ocols) diff --git a/tests/test_paging.py b/tests/test_paging.py index 778f07b3..9af2bacb 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -236,7 +236,7 @@ def check_multiple_paging_core(qs, s): page = assert_paging_core(t.page_with_paging, t.gathered, t.backwards, t.selected, t.page, i + 1) if page is None: # Ensure union of pages is original q.all() - assert list(t.gathered) == t.unpaged + assert list(t.gathered) == t.unpaged, f"Different elements for tracker {t}" page_trackers.remove(t) continue From 61477e167eaa107f09050f822e28670b804bec48 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 14:53:32 -0700 Subject: [PATCH 148/159] Fix ordering. --- sqlakeyset/paging.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 7cd3324a..9d21d024 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -582,9 +582,11 @@ def select_homogeneous_pages( continue name = OC(col.extra_column).quoted_full_name if name in extra_column_mappers: - mapped_ocols[i] = copy(extra_column_mappers[name]) - # Fix up oc to make sure the ordering is correct. - mapped_ocols[i].oc = order_cols[i] + mapped_ocols[i] = extra_column_mappers[name] + # Since we cache these mappers across different selects, we need + # to fix up any ordering here. + if mapped_ocols[i].oc.is_ascending() != order_cols[i].is_ascending(): + mapped_ocols[i] = mapped_ocols[i].reversed() else: extra_column_mappers[name] = col mapped_order_columns_per_request.append(mapped_ocols) From e72e27b9f354c5ec74a9b83d3adffaa9cde9d18a Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 14:56:35 -0700 Subject: [PATCH 149/159] Fix callable. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 9d21d024..adfc0555 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -585,7 +585,7 @@ def select_homogeneous_pages( mapped_ocols[i] = extra_column_mappers[name] # Since we cache these mappers across different selects, we need # to fix up any ordering here. - if mapped_ocols[i].oc.is_ascending() != order_cols[i].is_ascending(): + if mapped_ocols[i].oc.is_ascending != order_cols[i].is_ascending: mapped_ocols[i] = mapped_ocols[i].reversed() else: extra_column_mappers[name] = col From f0f9ed28971d1d7760e124e35cd423290d2455ad Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 14:59:14 -0700 Subject: [PATCH 150/159] Another property. --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index adfc0555..28a35252 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -586,7 +586,7 @@ def select_homogeneous_pages( # Since we cache these mappers across different selects, we need # to fix up any ordering here. if mapped_ocols[i].oc.is_ascending != order_cols[i].is_ascending: - mapped_ocols[i] = mapped_ocols[i].reversed() + mapped_ocols[i] = mapped_ocols[i].reversed else: extra_column_mappers[name] = col mapped_order_columns_per_request.append(mapped_ocols) From 61685b6b434bf3d7ffe15599e29b4785ef392c0c Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 15:14:37 -0700 Subject: [PATCH 151/159] Add one more unit test. --- tests/test_paging.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_paging.py b/tests/test_paging.py index 9af2bacb..bba48ad5 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -494,6 +494,20 @@ def test_orm_multiple_pages_select_columns(no_sqlite_dburl): check_multiple_paging_orm(qs=qs) +@pytest.mark.skipif( + SQLA_VERSION < version.parse("1.4.0b1"), + reason="Not supported in 1.3." +) +def test_orm_multiple_pages_different_extra_columns(no_sqlite_dburl): + with S(no_sqlite_dburl, echo=ECHO) as s: + qs = [ + s.query(Book.name).order_by(Book.b, Book.id), + s.query(Book.name).order_by(Book.id), + s.query(Book.name).order_by(Book.c, Book.id), + ] + check_multiple_paging_core(qs=qs, s=s) + + def test_orm_multiple_pages_one_query(no_sqlite_dburl): with S(no_sqlite_dburl, echo=ECHO) as s: qs = [ From 70379cd29f8909d343330676f9a9c21a813cd52f Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 15:16:35 -0700 Subject: [PATCH 152/159] fix test. --- tests/test_paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index bba48ad5..47440aec 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -505,7 +505,7 @@ def test_orm_multiple_pages_different_extra_columns(no_sqlite_dburl): s.query(Book.name).order_by(Book.id), s.query(Book.name).order_by(Book.c, Book.id), ] - check_multiple_paging_core(qs=qs, s=s) + check_multiple_paging_orm(qs=qs, s=s) def test_orm_multiple_pages_one_query(no_sqlite_dburl): From 793710c37bad5e8acfddd1460d004129845da147 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 15:26:01 -0700 Subject: [PATCH 153/159] Fix test again. --- tests/test_paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_paging.py b/tests/test_paging.py index 47440aec..fc1830e2 100644 --- a/tests/test_paging.py +++ b/tests/test_paging.py @@ -505,7 +505,7 @@ def test_orm_multiple_pages_different_extra_columns(no_sqlite_dburl): s.query(Book.name).order_by(Book.id), s.query(Book.name).order_by(Book.c, Book.id), ] - check_multiple_paging_orm(qs=qs, s=s) + check_multiple_paging_orm(qs=qs) def test_orm_multiple_pages_one_query(no_sqlite_dburl): From 039d4563077996b035f99853fe4576991b099606 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 15:57:14 -0700 Subject: [PATCH 154/159] Attempt to refactor to fix orm as well. --- sqlakeyset/paging.py | 207 +++++++++++++++++++++---------------------- 1 file changed, 102 insertions(+), 105 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 28a35252..70d7fe1d 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -3,7 +3,7 @@ from __future__ import annotations from functools import partial -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import ( Any, Callable, @@ -210,17 +210,7 @@ def prepare_paging( col.extra_column for col in mapped_ocols if col.extra_column is not None ] - # page_identifier is used for fetching multiple pages. - if page_identifier is not None: - extra_columns += [ - literal(page_identifier).label("_page_identifier"), - func.ROW_NUMBER().over( - order_by=[c.uo for c in order_cols] - ).label("_row_number"), - ] - if hasattr(q, "add_columns"): # ORM or SQLAlchemy 1.4+ - print(f"Adding extra columns: {extra_columns}") q = q.add_columns(*extra_columns) else: for col in extra_columns: # SQLAlchemy Core <1.4 @@ -501,8 +491,10 @@ def get_homogeneous_pages(requests: list[OrmPageRequest[_TP]]) -> list[Page[Row[ if not requests: return [] + ordering_infos = _get_ordering_infos(requests, orm=True) prepared_queries = [ - _orm_prepare_homogeneous_page(request, i) for i, request in enumerate(requests) + _orm_prepare_homogeneous_page(request, ordering_infos[i], i) + for i, request in enumerate(requests) ] query = prepared_queries[0].paging_query.query @@ -563,58 +555,20 @@ def select_homogeneous_pages( # to first find the superset of extra columns and then add those to ever single # selectable. - extra_column_mappers: dict[str, MappedOrderColumn] = {} - mapped_order_columns_per_request = [] - order_cols_per_request = [] - for request in requests: - try: - column_descriptions = request.selectable.column_descriptions - except Exception: - column_descriptions = request.selectable._raw_columns # type: ignore - order_cols = parse_ob_clause(request.selectable) - place, backwards = process_args(request.after, request.before, request.page) - if backwards: - order_cols = [c.reversed for c in order_cols] - order_cols_per_request.append(order_cols) - mapped_ocols = [find_order_key(ocol, column_descriptions) for ocol in order_cols] - for i, col in enumerate(list(mapped_ocols)): - if col.extra_column is None: - continue - name = OC(col.extra_column).quoted_full_name - if name in extra_column_mappers: - mapped_ocols[i] = extra_column_mappers[name] - # Since we cache these mappers across different selects, we need - # to fix up any ordering here. - if mapped_ocols[i].oc.is_ascending != order_cols[i].is_ascending: - mapped_ocols[i] = mapped_ocols[i].reversed - else: - extra_column_mappers[name] = col - mapped_order_columns_per_request.append(mapped_ocols) - - extra_columns = [col.extra_column for col in extra_column_mappers.values()] + ordering_infos = _get_ordering_infos(requests, orm=False) - print(f"Extra columns: {extra_columns}") - prepared_queries = [_core_prepare_homogeneous_page(request, s, order_cols_per_request[i], mapped_order_columns_per_request[i], extra_columns, i) for i, request in enumerate(requests)] + prepared_queries = [ + _core_prepare_homogeneous_page(request, s, ordering_infos[i], i) + for i, request in enumerate(requests) + ] selectable = union_all( *[p.paging_query.select for p in prepared_queries] ).order_by(text("_page_identifier"), text("_row_number")) - """ - if len(requests) > 1: - selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) - """ - compiled = selectable.compile(compile_kwargs={"literal_binds": True}) - print(f"Select statement: {compiled}") columns = prepared_queries[0].paging_query.select._raw_columns - print(columns) selectable = select(*columns).from_statement(selectable) - """ - selectable = select(*[col for col in selectable.columns]).select_from(selectable) - selectable = selectable.order_by(text("_page_identifier"), text("_row_number")) - selectable = select(selectable.subquery()).order_by(text("_page_identifier"), text("_row_number")) - """ compiled = selectable.compile(compile_kwargs={"literal_binds": True}) print(f"Select from statement: {compiled}") selected = s.execute(selectable) @@ -628,41 +582,80 @@ def select_homogeneous_pages( page_to_rows[row._page_identifier].append(row) pages = [] - # This is an unfortunate side effect of union_all. It appears when we union_all - # a bunch of selects, it changes the "keys" on us in cases where the column - # name and python attribute don't match. So we have to execute the first - # query standalone to ge the correct keys. - """ - subselect_result = ( - s.execute(prepared_queries[0].paging_query.select) - if len(prepared_queries) > 1 else selected - ) - """ + keys = list(selected.keys()) - print(f"pre-shrunk keys: {keys}") N = len(keys) - len(prepared_queries[0].paging_query.extra_columns) keys = keys[:N] - print(f"post-shrunk keys: {keys}") - print(f"Page to rows: {page_to_rows}") for i in range(len(requests)): rows = page_to_rows[i] - prepared_query = prepared_queries[i] - old_sel = prepared_query.paging_query - """ - mapped_order_columns = [] - for ocol in prepared_query.paging_query.order_columns: - corresponding_column = selectable.corresponding_column(ocol.element) - mapped_order_columns.append(find_order_key(OC(corresponding_column), selectable.column_descriptions)) - key_rows = [tuple(col.get_from_row(row) for col in mapped_order_columns) for row in rows] - print(f"key_rows: {key_rows}") - sel = _PagingSelect(old_sel.select, old_sel.order_columns, mapped_order_columns, old_sel.extra_columns) - pages.append(prepared_queries[i].page_from_rows(rows, sel, keys)) - """ - pages.append(prepared_queries[i].page_from_rows(rows, old_sel, keys)) + pages.append(prepared_queries[i].page_from_rows(rows, keys)) return pages +@dataclass +class _OrderingInfo: + order_cols: list[OC] = field(default_factory=list) + mapped_ocols: list[MappedOrderColumn] = field(default_factory=list) + extra_columns: list[ColumnElement] = field(default_factory=list) + + +def _get_ordering_infos(requests, orm) -> list[_OrderingInfo]: + infos = [] + extra_column_mappers: dict[str, MappedOrderColumn] = {} + + for request in requests: + info = _OrderingInfo() + infos.append(info) + if orm: + if not isinstance(request, OrmPageRequest): + raise ValueError("If orm=True then requests must be OrmPageRequests") + selectable = orm_to_selectable(q) + column_descriptions = q.column_descriptions + else: + if isinstance(request, OrmPageRequest): + raise ValueError("If orm=False then q cannot be a OrmPageRequest") + selectable = q + try: + column_descriptions = q.column_descriptions + except Exception: + column_descriptions = q._raw_columns # type: ignore + + order_cols = parse_ob_clause(selectable) + place, backwards = process_args(request.after, request.before, request.page) + if backwards: + order_cols = [c.reversed for c in order_cols] + info.order_cols = order_cols + + mapped_ocols = [find_order_key(ocol, column_descriptions) for ocol in order_cols] + for i, col in enumerate(list(mapped_ocols)): + if col.extra_column is None: + continue + name = OC(col.extra_column).quoted_full_name + if name in extra_column_mappers: + mapped_ocols[i] = extra_column_mappers[name] + # Since we cache these mappers across different selects, we need + # to fix up any ordering here. + if mapped_ocols[i].oc.is_ascending != order_cols[i].is_ascending: + mapped_ocols[i] = mapped_ocols[i].reversed + else: + extra_column_mappers[name] = col + + info.mapped_ocols = mapped_ocols + + extra_columns = [col.extra_column for col in extra_column_mappers.values()] + print(f"Extra columns: {extra_columns}") + for i, info in enumerate(infos): + info.extra_columns = list(extra_columns) + [ + literal(page_identifier).label("_page_identifier"), + func.ROW_NUMBER().over( + order_by=[c.uo for c in info.order_cols] + ).label("_row_number"), + ] + + return infos + + @dataclass class _PreparedQuery: paging_query: Union[_PagingQuery, _PagingSelect] @@ -672,9 +665,7 @@ class _PreparedQuery: def _core_prepare_homogeneous_page( request: PageRequest[_TP], s: Union[Session, Connection], - order_cols: list[OC], - mapped_ocols: list[MappedOrderColumn], - extra_columns: list[ColumnElement], + info: _OrderingInfo, page_identifier: int ) -> _PreparedQuery: place, backwards = process_args(request.after, request.before, request.page) @@ -682,30 +673,24 @@ def _core_prepare_homogeneous_page( selectable = request.selectable result_type = core_result_type(selectable, s) - clauses = [col.ob_clause for col in mapped_ocols] + clauses = [col.ob_clause for col in info.mapped_ocols] selectable = selectable.order_by(None).order_by(*clauses) - extra_columns = list(extra_columns) + [ - literal(page_identifier).label("_page_identifier"), - func.ROW_NUMBER().over( - order_by=[c.uo for c in order_cols] - ).label("_row_number"), - ] - selectable = selectable.add_columns(*extra_columns) + selectable = selectable.add_columns(*info.extra_columns) selectable = _apply_where_and_limit( selectable, selectable, request.per_page, place, get_bind(q=selectable, s=s).dialect, - order_cols, + info.order_cols, orm=False ) - sel = _PagingSelect(selectable, order_cols, mapped_ocols, extra_columns) + sel = _PagingSelect(selectable, info.order_cols, info.mapped_ocols, info.extra_columns) - def page_from_rows(rows, paging_select, keys): + def page_from_rows(rows, keys): page = core_page_from_rows( - paging_select, + sel, rows, keys, result_type, @@ -719,7 +704,7 @@ def page_from_rows(rows, paging_select, keys): def _orm_prepare_homogeneous_page( - request: OrmPageRequest[_TP], page_identifier: int + request: OrmPageRequest[_TP], info: _OrderingInfo, page_identifier: int ) -> _PreparedQuery: place, backwards = process_args(request.after, request.before, request.page) @@ -727,15 +712,27 @@ def _orm_prepare_homogeneous_page( result_type = orm_result_type(query) keys = orm_query_keys(query) - paging_query = prepare_paging( - q=query, - per_page=request.per_page, - place=place, - backwards=backwards, - orm=True, - dialect=query.session.get_bind().dialect, - page_identifier=page_identifier, + clauses = [col.ob_clause for col in info.mapped_ocols] + query = query.order_by(None).order_by(*clauses) + + if hasattr(q, "add_columns"): # ORM or SQLAlchemy 1.4+ + print(f"Adding extra columns: {info.extra_columns}") + query = query.add_columns(*info.extra_columns) + else: + for col in info.extra_columns: # SQLAlchemy Core <1.4 + query = query.column(col) # type: ignore + + selectable = orm_to_selectable(query) + query = _apply_where_and_limit( + query, + orm_to_selectable(query), + request.per_page, + place, + get_bind(q=selectable, s=s).dialect, + info.order_cols, + orm=False ) + paging_query = _PagingQuery(query, info.order_cols, info.mapped_ocols, info.extra_columns) def page_from_rows(rows): return orm_page_from_rows( From 037ab1c727861ea29bdc8fde9d7e71ecd0d756e6 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 16:00:44 -0700 Subject: [PATCH 155/159] Flake8 --- sqlakeyset/paging.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 70d7fe1d..d25b97f8 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -610,16 +610,16 @@ def _get_ordering_infos(requests, orm) -> list[_OrderingInfo]: if orm: if not isinstance(request, OrmPageRequest): raise ValueError("If orm=True then requests must be OrmPageRequests") - selectable = orm_to_selectable(q) - column_descriptions = q.column_descriptions + selectable = orm_to_selectable(request.query) + column_descriptions = request.query.column_descriptions else: if isinstance(request, OrmPageRequest): raise ValueError("If orm=False then q cannot be a OrmPageRequest") - selectable = q + selectable = request.selectable try: - column_descriptions = q.column_descriptions + column_descriptions = selectable.column_descriptions except Exception: - column_descriptions = q._raw_columns # type: ignore + column_descriptions = selectable._raw_columns # type: ignore order_cols = parse_ob_clause(selectable) place, backwards = process_args(request.after, request.before, request.page) @@ -647,12 +647,12 @@ def _get_ordering_infos(requests, orm) -> list[_OrderingInfo]: print(f"Extra columns: {extra_columns}") for i, info in enumerate(infos): info.extra_columns = list(extra_columns) + [ - literal(page_identifier).label("_page_identifier"), + literal(i).label("_page_identifier"), func.ROW_NUMBER().over( order_by=[c.uo for c in info.order_cols] ).label("_row_number"), ] - + return infos @@ -715,7 +715,7 @@ def _orm_prepare_homogeneous_page( clauses = [col.ob_clause for col in info.mapped_ocols] query = query.order_by(None).order_by(*clauses) - if hasattr(q, "add_columns"): # ORM or SQLAlchemy 1.4+ + if hasattr(query, "add_columns"): # ORM or SQLAlchemy 1.4+ print(f"Adding extra columns: {info.extra_columns}") query = query.add_columns(*info.extra_columns) else: @@ -728,7 +728,7 @@ def _orm_prepare_homogeneous_page( orm_to_selectable(query), request.per_page, place, - get_bind(q=selectable, s=s).dialect, + query.session.get_bind().dialect, info.order_cols, orm=False ) From 80e0f5e6653c0bde87d3e60e9ca1529304b56ef7 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 16:01:38 -0700 Subject: [PATCH 156/159] flake8 --- sqlakeyset/paging.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index d25b97f8..1c5de9f8 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -722,7 +722,6 @@ def _orm_prepare_homogeneous_page( for col in info.extra_columns: # SQLAlchemy Core <1.4 query = query.column(col) # type: ignore - selectable = orm_to_selectable(query) query = _apply_where_and_limit( query, orm_to_selectable(query), From 1c45508141972d3fb3e78fac95c0763fc0a854f9 Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 16:06:16 -0700 Subject: [PATCH 157/159] Fix orm=False --- sqlakeyset/paging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 1c5de9f8..4f9ff7cf 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -729,7 +729,7 @@ def _orm_prepare_homogeneous_page( place, query.session.get_bind().dialect, info.order_cols, - orm=False + orm=True ) paging_query = _PagingQuery(query, info.order_cols, info.mapped_ocols, info.extra_columns) From 5e2980bf35e1c9e1d15f03f56398bdb3f827feba Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 16:09:16 -0700 Subject: [PATCH 158/159] Cleanup. --- sqlakeyset/columns.py | 1 - sqlakeyset/paging.py | 12 +++++------- sqlakeyset/sqla20.py | 12 ------------ 3 files changed, 5 insertions(+), 20 deletions(-) diff --git a/sqlakeyset/columns.py b/sqlakeyset/columns.py index 94126dd2..fa621131 100644 --- a/sqlakeyset/columns.py +++ b/sqlakeyset/columns.py @@ -408,7 +408,6 @@ def derive_order_key(ocol, desc, index): # is an attribute with label try: - print(f"Attempting to match label: {ocol.quoted_full_name} / {OC(expr).full_name} / {OC(expr).quoted_full_name}") if ocol.quoted_full_name == OC(expr).quoted_full_name: return DirectColumn(ocol, index) except sqlalchemy.exc.ArgumentError: diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index 4f9ff7cf..b4f69fdc 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -491,6 +491,10 @@ def get_homogeneous_pages(requests: list[OrmPageRequest[_TP]]) -> list[Page[Row[ if not requests: return [] + # Because UNION ALL requires identical SELECT statements, but we allow different + # order_bys which could result in different extra columns for order keys, we need + # to first find the superset of extra columns and then add those to every single + # selectable. ordering_infos = _get_ordering_infos(requests, orm=True) prepared_queries = [ _orm_prepare_homogeneous_page(request, ordering_infos[i], i) @@ -552,9 +556,8 @@ def select_homogeneous_pages( # Because UNION ALL requires identical SELECT statements, but we allow different # order_bys which could result in different extra columns for order keys, we need - # to first find the superset of extra columns and then add those to ever single + # to first find the superset of extra columns and then add those to every single # selectable. - ordering_infos = _get_ordering_infos(requests, orm=False) prepared_queries = [ @@ -569,10 +572,7 @@ def select_homogeneous_pages( columns = prepared_queries[0].paging_query.select._raw_columns selectable = select(*columns).from_statement(selectable) - compiled = selectable.compile(compile_kwargs={"literal_binds": True}) - print(f"Select from statement: {compiled}") selected = s.execute(selectable) - results = selected.fetchall() # We need to make sure there's an entry for every page in case some return @@ -644,7 +644,6 @@ def _get_ordering_infos(requests, orm) -> list[_OrderingInfo]: info.mapped_ocols = mapped_ocols extra_columns = [col.extra_column for col in extra_column_mappers.values()] - print(f"Extra columns: {extra_columns}") for i, info in enumerate(infos): info.extra_columns = list(extra_columns) + [ literal(i).label("_page_identifier"), @@ -716,7 +715,6 @@ def _orm_prepare_homogeneous_page( query = query.order_by(None).order_by(*clauses) if hasattr(query, "add_columns"): # ORM or SQLAlchemy 1.4+ - print(f"Adding extra columns: {info.extra_columns}") query = query.add_columns(*info.extra_columns) else: for col in info.extra_columns: # SQLAlchemy Core <1.4 diff --git a/sqlakeyset/sqla20.py b/sqlakeyset/sqla20.py index 09473225..9e17514e 100644 --- a/sqlakeyset/sqla20.py +++ b/sqlakeyset/sqla20.py @@ -61,18 +61,6 @@ 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 - """ - if any(col._annotations for col in selectable._raw_columns): - return selectable._raw_columns[0] - if hasattr(selectable, "_raw_columns"): - for col in selectable._raw_columns: - if not isinstance(col, Table): - continue - print(f"Annotations: {col._annotations}") - if "parententity" in col._annotations: - return col._annotations["parententity"].entity - # print(f"Raw columns: {selectable._raw_columns}") - """ return None From be87aad34d8d0255aca2e502d96e7cca2e7c4bac Mon Sep 17 00:00:00 2001 From: Matthew Albrecht Date: Thu, 20 Jul 2023 16:12:03 -0700 Subject: [PATCH 159/159] Clarify version --- sqlakeyset/paging.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sqlakeyset/paging.py b/sqlakeyset/paging.py index b4f69fdc..3b3a1d5c 100644 --- a/sqlakeyset/paging.py +++ b/sqlakeyset/paging.py @@ -524,7 +524,7 @@ def get_homogeneous_pages(requests: list[OrmPageRequest[_TP]]) -> list[Page[Row[ def select_homogeneous_pages( requests: list[PageRequest[_TP]], s: Union[Session, Connection] ) -> list[Page[Row[_TP]]]: - """Get multiple pages of results for homogeneous legacy ORM queries. + """Get multiple pages of results for homogeneous 2.0 style queries. This only involves a single round trip to the database. To do that, under the hood it generates a UNION ALL. That means each query must select exactly the @@ -535,6 +535,8 @@ def select_homogeneous_pages( statements in components of a compound select, which SQLite does not. Resulting pages are returned in the same order as the original page requests. + + Only supported in SQLAlchemy version 1.4+ """ if not requests: return []