diff --git a/CHANGELOG.md b/CHANGELOG.md index 98cafa4..bb77efe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -141,6 +141,13 @@ hardening standalone use; the highlights: including nested jsonb decoding. (#61–#87) - `$maxgte` and related operators work without requiring custom SQL functions. (#61–#87) +- An array value whose entries are integers of more than one type -- a list + built from a Sage `Integer` and a couple of Python `int` literals, say -- is + made homogeneous before it is bound, instead of failing with psycopg's + `DataError: cannot dump lists of mixed types`. Nested arrays count as one + array, as they do in Postgres. Values that psycopg handles on its own, + including its `Int2`/`Int4`/`Int8`/`IntNumeric` oid wrappers and anything + holding a `bool`, are untouched. - Statistics and counts no longer accumulate duplicate rows, and `refresh_stats` converges. (#97) - Sage-free statistics, fresh-database bootstrap (`create=True`), and jsonb diff --git a/psycodict/base.py b/psycodict/base.py index ae61f04..8b72420 100644 --- a/psycodict/base.py +++ b/psycodict/base.py @@ -29,7 +29,7 @@ ) from psycopg.sql import SQL, Identifier, Placeholder, Literal, Composable -from .encoding import Json +from .encoding import Json, homogenize_values from .utils import reraise, DelayCommit, QueryLogFilter from .validation import ( InvalidDefinitionError, @@ -339,6 +339,11 @@ def _execute( elif commit: raise ValueError("buffered and commit are incompatible") + # psycopg refuses an array whose entries are integers of more than one + # type, which a caller assembling one from several sources (Sage + # integers from one, Python ints from another) produces easily. + values = homogenize_values(values, values_list) + try: cur = self._db._cursor(buffered=buffered) diff --git a/psycodict/encoding.py b/psycodict/encoding.py index 5747624..5eedf2f 100644 --- a/psycodict/encoding.py +++ b/psycodict/encoding.py @@ -7,6 +7,8 @@ import json import datetime import math +import numbers +from collections.abc import Mapping from psycopg.adapt import Dumper try: try: @@ -235,6 +237,192 @@ def dump(self, obj): return _pg_array_literal(obj._seq).encode() +def _array_element_types(seq, types): + """ + Add the types of the non-``None`` entries of a possibly nested list to + the set ``types``, flattening exactly as psycopg does when it decides + which dumper an array's elements get. + """ + for item in seq: + if isinstance(item, list): + _array_element_types(item, types) + elif item is not None: + types.add(type(item)) + + +def _as_ints(seq): + """ + A copy of a possibly nested list with every entry replaced by the + equivalent Python ``int``, leaving ``None`` (Postgres NULL) alone. + """ + return [ + _as_ints(item) if isinstance(item, list) + else item if item is None + else int(item) + for item in seq + ] + + +def homogenize_int_arrays(value): + """ + Return ``value`` with the entries of an all-integer list made into + Python ints, if they are not all of one type already. + + psycopg dumps a list by picking one dumper for the whole array, so it + refuses a list whose entries have different types unless those types + dump to the same Postgres oid: ``[Integer(1), 2]``, mixing Sage's + integers with Python's, raises ``DataError: cannot dump lists of mixed + types``. Such a list means one thing only -- an array of integers -- + and callers assembling one out of several sources (a parsed search box + here, a literal there) have no reason to care which flavour of integer + each source produced, so psycodict makes them agree rather than making + the query fail. + + Only lists that mix an integer type psycopg does not know natively into + an otherwise integral list are touched, so this changes the behaviour of + no query that worked before: + + - a list of a single type dumps fine as it is, whatever that type is; + - a list holding anything non-integral is left for psycopg to judge; + - ``bool`` is integral to Python but not to Postgres, so a list holding + one is left alone; + - subclasses of ``int`` (which is how psycopg's own ``Int2``/``Int4``/ + ``Int8``/``IntNumeric`` wrappers ask for a particular oid) are left + alone, since replacing them would discard the oid they were chosen for. + + INPUT: + + - ``value`` -- anything that might be passed to Postgres as a value + + OUTPUT: + + Either ``value`` itself, or a list of ints with the same shape. + + EXAMPLES:: + + >>> import numbers + >>> from psycodict.encoding import homogenize_int_arrays + + Sage's ``Integer`` is the type this exists for. psycodict does not + require Sage, so the examples below use a stand-in with the one property + that matters here, being registered as an integer:: + + >>> class MyInt: + ... def __init__(self, n): + ... self.n = n + ... def __int__(self): + ... return self.n + >>> _ = numbers.Integral.register(MyInt) + + A list of Python ints is homogeneous already and comes back unchanged, + as does anything that is not a list at all:: + + >>> homogenize_int_arrays([1, -1, 1]) + [1, -1, 1] + >>> homogenize_int_arrays(3) + 3 + + Mixing in the other integer type would make psycopg refuse the array, so + the entries become ints:: + + >>> homogenize_int_arrays([MyInt(1), -1, 1]) + [1, -1, 1] + + A nested array is one array, as it is in Postgres, and its NULLs stay + NULL:: + + >>> homogenize_int_arrays([[MyInt(1), 2], [3, None]]) + [[1, 2], [3, None]] + + A list that is not all integers is psycopg's business, not ours:: + + >>> homogenize_int_arrays([0.5, 1]) + [0.5, 1] + """ + if not isinstance(value, list): + return value + types = set() + _array_element_types(value, types) + if len(types) < 2: + # One type (or an array of nothing but NULLs): psycopg is content. + return value + if not all(issubclass(typ, numbers.Integral) for typ in types): + return value + if any(issubclass(typ, bool) for typ in types): + return value + if all(issubclass(typ, int) for typ in types): + # psycopg understands all of these; whatever it makes of the + # mixture, it is not ours to second-guess. + return value + return _as_ints(value) + + +def _map_changed(params, func): + """ + ``func`` applied to every value of a parameter collection -- a sequence + for ``%s`` placeholders, a mapping for the ``%(name)s`` ones that + multi-row inserts use -- except that ``params`` itself comes back when + ``func`` changed nothing, so that the overwhelmingly common case of a + query with nothing to fix allocates nothing. + """ + out = None + if isinstance(params, Mapping): + for key, item in params.items(): + new = func(item) + if new is not item: + if out is None: + out = dict(params) + out[key] = new + else: + for i, item in enumerate(params): + new = func(item) + if new is not item: + if out is None: + out = list(params) + out[i] = new + return params if out is None else out + + +def homogenize_values(values, values_list=False): + """ + Return the values of a query with every mixed-type integer array in them + made homogeneous by :func:`homogenize_int_arrays`. + + INPUT: + + - ``values`` -- the values of a single statement: a sequence with one + entry per ``%s`` placeholder, or a mapping keyed by the names of + ``%(name)s`` ones. When ``values_list`` is set, a sequence of such + collections instead, one per row of a multi-row insert. + - ``values_list`` -- boolean (default ``False``); whether ``values`` is a + sequence of rows rather than the values of one statement + + OUTPUT: + + ``values`` itself when nothing needed changing, which is the usual case, + so that ordinary queries copy nothing. + + EXAMPLES:: + + >>> from psycodict.encoding import homogenize_values + + A query whose values hold no mixed array -- almost every query -- gets + its own values back, not a copy of them:: + + >>> values = [[1, 2], 'a', 3] + >>> homogenize_values(values) is values + True + >>> rows = [[[1, 2], 'a'], [[3, 4], 'b']] + >>> homogenize_values(rows, values_list=True) is rows + True + """ + if not values: + return values + if values_list: + return _map_changed(values, homogenize_values) + return _map_changed(values, homogenize_int_arrays) + + class Json(): """ A wrapper marking a value for storage as json/jsonb, encoded with @@ -424,6 +612,13 @@ def prep(cls, obj, escape_backslashes=False): return {"__time__": 0, "data": "%s" % (obj)} elif isinstance(obj, (str, bool, float, int)): return obj + elif isinstance(obj, numbers.Integral): + # Some other integer type: gmpy2's, numpy's, or Sage's when + # psycodict was imported without Sage. Stored as the integer it + # is, as :func:`homogenize_int_arrays` sends one to Postgres. + # This branch comes after the one above so that ``bool``, which + # Python counts as an integer and Postgres does not, stays a bool. + return int(obj) else: raise ValueError("Unsupported type: %s" % (type(obj))) diff --git a/tests/conftest.py b/tests/conftest.py index 4238275..c04c6ab 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -22,12 +22,44 @@ checkout without PostgreSQL is not a wall of errors. Continuous integration sets ``PSYCODICT_TEST_DB_REQUIRED=1``, which turns that skip into a failure. """ +import numbers import os import uuid import pytest +class FakeInteger: + """ + An integer type that is not a subclass of ``int``, standing in for Sage's + ``Integer``. + + psycodict does not depend on Sage, so the type that motivates + :func:`psycodict.encoding.homogenize_int_arrays` cannot be imported by the + unit CI jobs. Registering with ``numbers.Integral`` reproduces everything + that function and psycopg look at: a distinct type, integral, convertible + with ``int()``. + """ + + def __init__(self, n): + self.n = n + + def __int__(self): + return self.n + + def __eq__(self, other): + return self.n == int(other) + + def __hash__(self): + return hash(self.n) + + def __repr__(self): + return "FakeInteger(%d)" % self.n + + +numbers.Integral.register(FakeInteger) + + def _connection_kwargs(): return { "host": os.environ.get("PGHOST", "localhost"), diff --git a/tests/test_encoding.py b/tests/test_encoding.py index 925ba89..2fd3f86 100644 --- a/tests/test_encoding.py +++ b/tests/test_encoding.py @@ -13,7 +13,16 @@ import pytest import psycodict.encoding -from psycodict.encoding import SAGE_MODE, Array, Json, copy_dumps, numeric_converter +from conftest import FakeInteger +from psycodict.encoding import ( + SAGE_MODE, + Array, + Json, + copy_dumps, + homogenize_int_arrays, + homogenize_values, + numeric_converter, +) # COPY FROM applies these escapes to every text field it reads; undoing them is @@ -599,3 +608,112 @@ def test_array_getquoted(seq, expected): # array literal instead (sent with unknown oid, so the server casts by # context, via ArrayDumper). assert Array(seq).getquoted() == expected + + +# --------------------------------------------------------------------------- +# arrays of more than one integer type +# --------------------------------------------------------------------------- + +def flatten(seq): + """ + Every non-list entry of a possibly nested list, in order. + """ + for item in seq: + if isinstance(item, list): + yield from flatten(item) + else: + yield item + + +@pytest.mark.parametrize("value,expected", [ + # The shape that broke LMFDB: a list assembled from a computed Sage + # integer and a couple of literals. + ([FakeInteger(1), -1, 1], [1, -1, 1]), + ([1, -1, FakeInteger(1)], [1, -1, 1]), + # A nested array is a single array to Postgres, and to psycopg, so the + # types have to agree across the whole of it and not row by row. + ([[FakeInteger(1), 2], [3, 4]], [[1, 2], [3, 4]]), + ([[1, 2], [FakeInteger(3), 4]], [[1, 2], [3, 4]]), + ([[[FakeInteger(1)], [2]], [[3], [4]]], [[[1], [2]], [[3], [4]]]), + # NULL takes no part in the comparison and stays NULL. + ([None, FakeInteger(1), 2], [None, 1, 2]), + ([[None, FakeInteger(1)], [2, None]], [[None, 1], [2, None]]), + # int() of an integral value is exact however big it is. + ([FakeInteger(2**80), 1], [2**80, 1]), +]) +def test_homogenize_converts_arrays_of_several_integer_types(value, expected): + out = homogenize_int_arrays(value) + assert out == expected + assert {type(x) for x in flatten(out) if x is not None} == {int} + + +@pytest.mark.parametrize("value", [ + # One type is all psycopg asks for, whatever that type is. + [1, -1, 1], + [FakeInteger(1), FakeInteger(2)], + [[1, 2], [3, 4]], + [[FakeInteger(1)], [FakeInteger(2)]], + # Nothing to compare. + [], + [None, None], + # Not an array at all. A tuple is a composite value to psycopg, not an + # array, so it is none of this function's business either. + 3, + FakeInteger(3), + "abc", + (FakeInteger(1), 2), + # Not all integers: what psycopg makes of these is for psycopg to say. + [0.5, 1], + [FakeInteger(1), 0.5], + ["a", FakeInteger(1)], + # bool is an integer to Python but a distinct type to Postgres, so a + # list holding one is never quietly turned into a list of numbers. + [True, FakeInteger(1)], + [True, False], +]) +def test_homogenize_leaves_everything_else_alone(value): + assert homogenize_int_arrays(value) is value + + +@pytest.mark.parametrize("wrapper", ["Int2", "Int4", "Int8", "IntNumeric"]) +def test_homogenize_keeps_the_oid_psycopgs_wrappers_ask_for(wrapper): + # These subclass int precisely to pin down the oid an integer is sent + # with; replacing them with plain ints would discard the choice. + import psycopg.types.numeric + + value = [getattr(psycopg.types.numeric, wrapper)(1), 2] + assert homogenize_int_arrays(value) is value + + +def test_homogenize_values_fixes_every_parameter(): + assert homogenize_values(["a", [FakeInteger(1), 2], 3]) == ["a", [1, 2], 3] + + +def test_homogenize_values_fixes_every_row_of_a_multi_row_insert(): + rows = [["a", [FakeInteger(1), 2]], ["b", [3, 4]]] + assert homogenize_values(rows, values_list=True) == [["a", [1, 2]], ["b", [3, 4]]] + + +@pytest.mark.parametrize("values,values_list", [ + (["a", [1, 2], 3], False), + ([["a", [1, 2]], ["b", [3, 4]]], True), + ([], False), + (None, False), +]) +def test_homogenize_values_returns_what_it_was_given_when_nothing_needs_fixing(values, values_list): + # Every query psycodict runs goes through this, so the ordinary case of a + # query with no mixed array in it must not copy the values. + assert homogenize_values(values, values_list) is values + + +@pytest.mark.skipif(not SAGE_MODE, reason="needs SageMath") +def test_with_sage_arrays_of_integers_and_ints_become_ints(): + from sage.rings.integer import Integer + + # The real thing, and the real query: the CM field lookup on an elliptic + # curve isogeny class page built its coefficient list this way, and every + # such page returned a 500 until psycodict homogenized it. + D = Integer(-4) + out = homogenize_int_arrays([-D, 0, 1]) + assert out == [4, 0, 1] + assert {type(x) for x in out} == {int} diff --git a/tests/test_search.py b/tests/test_search.py index 81cde03..db81137 100644 --- a/tests/test_search.py +++ b/tests/test_search.py @@ -13,7 +13,7 @@ import pytest -from conftest import sample_row +from conftest import FakeInteger, sample_row @pytest.fixture @@ -476,6 +476,31 @@ def test_array_contains(filled_table): assert filled_table.count({"vec": {"$contains": 5}}) == 2 +def test_array_of_several_integer_types(filled_table): + # psycopg dumps an array with one dumper for the whole of it, and refuses + # a list whose entries are integers of different types. A caller building + # a query array out of a computed value and a literal or two produces such + # a list without noticing, so psycodict makes it homogeneous first. + assert filled_table.search({"vec": [FakeInteger(5), 6, 0]}, "n", limit=5) == [5] + assert filled_table.search({"mat": [FakeInteger(3), 9]}, "n", limit=5) == [3] + assert filled_table.count({"vec": [FakeInteger(5), 6, 1]}) == 0 + # and through the array operators, which build their arrays the same way + assert filled_table.search({"vec": {"$contains": [FakeInteger(5), 6]}}, "n", limit=5) == [5] + assert filled_table.search( + {"vec": {"$in": [[FakeInteger(5), 6, 0], [7, 8, 2]]}}, "n", limit=5 + ) == [5, 7] + + +def test_array_of_several_integer_types_when_nested(table_factory): + # Nesting makes no difference: a two dimensional array is one array to + # Postgres, so its entries must agree throughout, not row by row. + table = table_factory(columns=[("n", "integer"), ("grid", "integer[]")], label_col=None) + table.insert_many([{"n": i, "grid": [[i, i + 1], [i + 2, i + 3]]} for i in range(5)]) + assert table.search({"grid": [[3, 4], [5, 6]]}, "n", limit=5) == [3] + assert table.search({"grid": [[FakeInteger(3), 4], [5, 6]]}, "n", limit=5) == [3] + assert table.search({"grid": [[3, 4], [FakeInteger(5), 6]]}, "n", limit=5) == [3] + + def test_array_containedin(filled_table): assert filled_table.search({"vec": {"$containedin": [0, 1, 2]}}, "n", limit=5) == [0, 1] diff --git a/tests/test_write.py b/tests/test_write.py index 129ffbb..8af99cb 100644 --- a/tests/test_write.py +++ b/tests/test_write.py @@ -14,7 +14,7 @@ import pytest from psycopg import IntegrityError -from conftest import sample_row +from conftest import FakeInteger, sample_row from psycodict.utils import DelayCommit @@ -88,6 +88,26 @@ def test_insert_many_roundtrips_arrays(empty_table): assert second == {"vec": [], "mat": [1.5, 2.5]} +def test_insert_many_roundtrips_arrays_of_several_integer_types(empty_table): + # The rows of a multi-row insert are bound as one values list, so this is + # the other shape psycodict has to make homogeneous before psycopg sees it. + empty_table.insert_many([ + {"n": 1, "label": "a", "vec": [FakeInteger(3), -1, 0], "mat": [2, FakeInteger(4)]}, + {"n": 2, "label": "b", "vec": [1, 2, 3], "mat": [1, 2]}, + ]) + assert empty_table.lucky({"n": 1}, projection=["vec", "mat"]) == { + "vec": [3, -1, 0], "mat": [2, 4], + } + assert empty_table.lucky({"n": 2}, projection=["vec", "mat"]) == { + "vec": [1, 2, 3], "mat": [1, 2], + } + + +def test_update_with_an_array_of_several_integer_types(filled_table): + filled_table.update({"n": 3}, {"vec": [FakeInteger(9), 8, 7]}, restat=False) + assert filled_table.lucky({"n": 3}, projection="vec") == [9, 8, 7] + + def test_insert_many_roundtrips_double_precision_and_boolean(empty_table): empty_table.insert_many([{"n": 1, "label": "a", "x": -0.125, "flag": False}]) record = empty_table.lucky({"n": 1}, projection=["x", "flag"])