fix: avoid to dump [multi]range as text when binary is requested

If a range has no type information (e.g. is empty and it is a base
class, not a specific range subtype), `upgrade()` would have returned a
text dumper. This would have resulted in protocol violation, such as
requesting unhealthy amount of memory to the backend... and it was
tested too!

Restoring sanity has a drawback. If we have an array of ranges, none of
which has type information, we now fail binary dumping. Previously this
worked, provided that a cast `::int8range[]` was provided, but I think
with an extreme stretch of the protocol: we were producing binary arrays
containing text representation of the ranges.

I don't think the latter is quite valid. So I have a preference for
dropping the feature in exchange of not violating the protocol. We trade
a better dumping of the basic type (an xfail less there) in exchange of
losing the ability of dumping an array of ranges, all empty, using a
cast, in binary format. Binary dump works using a subclass such as
`Int8Range` instead of `Range`.

The lost use case is so marginal that I don't feel the need to document
it. We don't even have documentation for the `Range` subclasses.

diff --git a/psycopg/psycopg/types/multirange.py b/psycopg/psycopg/types/multirange.py
index 735333049b3fb1e9fb7a774adf560a5768a24333..1bf98ed347a7847ef6e25d742a20453ec2987c05 100644 (file)
--- a/psycopg/psycopg/types/multirange.py
+++ b/psycopg/psycopg/types/multirange.py
@@ -206,7 +206,7 @@ class BaseMultirangeDumper(RecursiveDumper):
 
         item = self._get_item(obj)
         if item is None:
-            return MultirangeDumper(self.cls)
+            return self
 
         dumper: BaseMultirangeDumper
         if type(item) is int:

diff --git a/psycopg/psycopg/types/range.py b/psycopg/psycopg/types/range.py
index 665c9a6de396596a7755d0f47548e1f0d84d7662..3327a8411ae299c296b3686831b2214c30c78ec4 100644 (file)
--- a/psycopg/psycopg/types/range.py
+++ b/psycopg/psycopg/types/range.py
@@ -310,7 +310,7 @@ class BaseRangeDumper(RecursiveDumper):
 
         item = self._get_item(obj)
         if item is None:
-            return RangeDumper(self.cls)
+            return self
 
         dumper: BaseRangeDumper
         if type(item) is int:

diff --git a/tests/types/test_multirange.py b/tests/types/test_multirange.py
index 4175eebe18543dc6af9b4fc482bcb23dd1c3b57a..fc074a4197d782cd9f1f8140e21fd4f239189c23 100644 (file)
--- a/tests/types/test_multirange.py
+++ b/tests/types/test_multirange.py
@@ -5,7 +5,6 @@ from decimal import Decimal
 import pytest
 
 from psycopg import pq, sql
-from psycopg import errors as e
 from psycopg.adapt import PyFormat
 from psycopg.types.range import Range
 from psycopg.types import multirange
@@ -196,22 +195,21 @@ def test_dump_builtin_empty_wrapper(conn, wrapper, fmt_in):
     assert rec[2] == dumper.oid
 
 
-@pytest.mark.parametrize("pgtype", mr_names)
-@pytest.mark.parametrize(
-    "fmt_in",
-    [
-        PyFormat.AUTO,
-        PyFormat.TEXT,
-        # There are many ways to work around this (use text, use a cast on the
-        # placeholder, use specific Range subclasses).
-        pytest.param(
-            PyFormat.BINARY,
-            marks=pytest.mark.xfail(
-                reason="can't dump array of untypes binary multirange without cast"
-            ),
+pyformat_array = [
+    PyFormat.AUTO,
+    PyFormat.TEXT,
+    # There are ways to work around this (use text, use specific Range subclasses).
+    pytest.param(
+        PyFormat.BINARY,
+        marks=pytest.mark.xfail(
+            reason="can't dump array of untyped binary multirange without cast"
         ),
-    ],
-)
+    ),
+]
+
+
+@pytest.mark.parametrize("pgtype", mr_names)
+@pytest.mark.parametrize("fmt_in", pyformat_array)
 def test_dump_builtin_array(conn, pgtype, fmt_in):
     mr1 = Multirange()  # type: ignore[var-annotated]
     mr2 = Multirange([Range(bounds="()")])  # type: ignore[var-annotated]
@@ -223,7 +221,7 @@ def test_dump_builtin_array(conn, pgtype, fmt_in):
 
 
 @pytest.mark.parametrize("pgtype", mr_names)
-@pytest.mark.parametrize("fmt_in", PyFormat)
+@pytest.mark.parametrize("fmt_in", pyformat_array)
 def test_dump_builtin_array_with_cast(conn, pgtype, fmt_in):
     mr1 = Multirange()  # type: ignore[var-annotated]
     mr2 = Multirange([Range(bounds="()")])  # type: ignore[var-annotated]
@@ -241,8 +239,8 @@ def test_dump_builtin_array_with_cast(conn, pgtype, fmt_in):
 @pytest.mark.parametrize("fmt_in", PyFormat)
 def test_dump_builtin_array_wrapper(conn, wrapper, fmt_in):
     wrapper = getattr(multirange, wrapper)
-    mr1 = Multirange()  # type: ignore[var-annotated]
-    mr2 = Multirange([Range(bounds="()")])  # type: ignore[var-annotated]
+    mr1 = wrapper()
+    mr2 = wrapper([Range(bounds="()")])
     cur = conn.execute(
         f"""select '{{"{{}}","{{(,)}}"}}' = %{fmt_in.value}""", ([mr1, mr2],)
     )
@@ -315,14 +313,8 @@ def test_copy_in(conn, min, max, bounds, format):
         r = Range(empty=True)
 
     mr = Multirange([r])
-    try:
-        with cur.copy(f"copy copymr (mr) from stdin (format {format.name})") as copy:
-            copy.write_row([mr])
-    except e.InternalError_:
-        if not min and not max and format == pq.Format.BINARY:
-            pytest.xfail("TODO: add annotation to dump multirange with no type info")
-        else:
-            raise
+    with cur.copy(f"copy copymr (mr) from stdin (format {format.name})") as copy:
+        copy.write_row([mr])
 
     rec = cur.execute("select mr from copymr order by id").fetchone()
     if not r.isempty:

diff --git a/tests/types/test_range.py b/tests/types/test_range.py
index 08b2387ab3d3e468a1ecdc8921c50b03ba9aa519..bb90fcd8b6007e87fc54d6a01fa44494f93d40ce 100644 (file)
--- a/tests/types/test_range.py
+++ b/tests/types/test_range.py
@@ -5,7 +5,6 @@ from decimal import Decimal
 import pytest
 
 from psycopg import pq, sql
-from psycopg import errors as e
 from psycopg.adapt import PyFormat
 from psycopg.types import range as range_module
 from psycopg.types.range import Range, RangeInfo, register_range
@@ -78,22 +77,21 @@ def test_dump_builtin_empty_wrapper(conn, wrapper, fmt_in):
     assert cur.fetchone()[0] is True
 
 
-@pytest.mark.parametrize("pgtype", range_names)
-@pytest.mark.parametrize(
-    "fmt_in",
-    [
-        PyFormat.AUTO,
-        PyFormat.TEXT,
-        # There are many ways to work around this (use text, use a cast on the
-        # placeholder, use specific Range subclasses).
-        pytest.param(
-            PyFormat.BINARY,
-            marks=pytest.mark.xfail(
-                reason="can't dump an array of untypes binary range without cast"
-            ),
+pyformat_array = [
+    PyFormat.AUTO,
+    PyFormat.TEXT,
+    # There are ways to work around this (use text, use specific Range subclasses).
+    pytest.param(
+        PyFormat.BINARY,
+        marks=pytest.mark.xfail(
+            reason="can't dump an array of untyped binary range without cast"
         ),
-    ],
-)
+    ),
+]
+
+
+@pytest.mark.parametrize("pgtype", range_names)
+@pytest.mark.parametrize("fmt_in", pyformat_array)
 def test_dump_builtin_array(conn, pgtype, fmt_in):
     r1 = Range(empty=True)  # type: ignore[var-annotated]
     r2 = Range(bounds="()")  # type: ignore[var-annotated]
@@ -105,7 +103,7 @@ def test_dump_builtin_array(conn, pgtype, fmt_in):
 
 
 @pytest.mark.parametrize("pgtype", range_names)
-@pytest.mark.parametrize("fmt_in", PyFormat)
+@pytest.mark.parametrize("fmt_in", pyformat_array)
 def test_dump_builtin_array_with_cast(conn, pgtype, fmt_in):
     r1 = Range(empty=True)  # type: ignore[var-annotated]
     r2 = Range(bounds="()")  # type: ignore[var-annotated]
@@ -211,14 +209,8 @@ def test_copy_in(conn, min, max, bounds, format):
     else:
         r = Range(empty=True)
 
-    try:
-        with cur.copy(f"copy copyrange (r) from stdin (format {format.name})") as copy:
-            copy.write_row([r])
-    except e.ProtocolViolation:
-        if not min and not max and format == pq.Format.BINARY:
-            pytest.xfail("TODO: add annotation to dump ranges with no type info")
-        else:
-            raise
+    with cur.copy(f"copy copyrange (r) from stdin (format {format.name})") as copy:
+        copy.write_row([r])
 
     rec = cur.execute("select r from copyrange order by id").fetchone()
     assert rec[0] == r