Types: Trace an unresolved column under its own type name

Looking an array up through its element type logged the element's name, which
belongs to no column, so a trace of what the map lacks named types nobody can
add. The lookup is separate from the resolution that records it, and records
the name the column has, once.

`UnresolvedType` reaches the front door, so a caller can tell a column with no
representation from one it can render, in place of skipping whole tables.

Author: florinutz

CHANGES.md

@@ -13,8 +13,8 @@
 - Reflection: A column whose CrateDB type the dialect cannot represent now
   raises `CompileError` naming that type, the table and the column when asked
   to compile it, where it previously raised `AttributeError` from inside
-  SQLAlchemy and named none of them. Reading such a column, and printing its
-  type, are unaffected
+  SQLAlchemy and named none of them. Such a column still reads, and from
+  SQLAlchemy 1.4 onwards printing its type yields the CrateDB type name
 - Reflection: Added `geo_point` and `geo_shape` to the reflected type map,
   resolving to the `Geopoint` and `Geoshape` types the package already offers,
   which lets tables such as `sys.summits` round-trip into DDL

src/sqlalchemy_cratedb/__init__.py

@@ -20,7 +20,7 @@
 # software solely pursuant to the terms of the relevant commercial agreement.
 
 from .compat.api13 import monkeypatch_add_exec_driver_sql
-from .dialect import dialect
+from .dialect import UnresolvedType, dialect
 from .predicate import match
 from .sa_version import SA_1_4, SA_VERSION
 from .support import insert_bulk
@@ -74,6 +74,7 @@
     Geoshape,
     ObjectArray,
     ObjectType,
+    UnresolvedType,
     match,
     knn_match,
     insert_bulk,

src/sqlalchemy_cratedb/compiler.py

@@ -296,9 +296,9 @@ def visit_unresolved(self, type_, **kw):
         """
         Refuse a CrateDB type reflection could not resolve, naming it.
 
-        Only compiling the type has no answer. Reading the column works, and so
-        does printing its type, which SQLAlchemy's string compiler renders from
-        the type itself without coming through here.
+        Compiling the type is what has no answer; reading the column works. From
+        SQLAlchemy 1.4 onwards, printing the type reaches the string compiler,
+        which renders it from the type itself without coming through here.
         """
         raise sa.exc.CompileError(
             "Unable to represent CrateDB type '{0}' in SQLAlchemy".format(type_.type_name)

src/sqlalchemy_cratedb/dialect.py

@@ -27,7 +27,6 @@
 from sqlalchemy.engine import default, reflection
 from sqlalchemy.exc import SQLAlchemyError
 from sqlalchemy.sql import functions
-from sqlalchemy.types import ARRAY
 from sqlalchemy.util import asbool, to_list
 
 from .compiler import (
@@ -48,9 +47,10 @@ class UnresolvedType(sqltypes.UserDefinedType):
     A CrateDB type the dialect has no SQLAlchemy counterpart for.
 
     Reflection yields it for a column it cannot resolve, carrying the CrateDB
-    type name as reported. Reading such a column is unaffected, and so is
-    printing its type, which shows the name. Compiling it as SQL is what has no
-    answer, and `CrateTypeCompiler` refuses there, naming the type.
+    type name as reported. Such a column still reads. Compiling the type as SQL
+    is what has no answer, and `CrateTypeCompiler` refuses there, naming the
+    type. From SQLAlchemy 1.4 onwards, printing the type yields the name; older
+    versions route printing through the same compiler and raise.
     """
 
     __visit_name__ = "unresolved"
@@ -109,7 +109,7 @@ class Double(sqltypes.Float):
 
     TYPES_MAP["double"] = DOUBLE
     TYPES_MAP["double precision"] = DOUBLE_PRECISION
-except Exception:  # noqa: S110
+except ImportError:
     pass
 
 
@@ -517,16 +517,31 @@ def _resolve_type(self, type_):
         Turn a type name, as `information_schema.columns` reports it, into the
         SQLAlchemy type a reflected column of that type carries.
 
-        The map answers first, so that `object_array` keeps the entry it has.
-        Every other name ending in `_array` names an array of the type the rest
-        of the name names, and resolves to an `ARRAY` of that type. Two shapes
-        have no representation and are left unresolved: an array whose element
-        type is itself unresolved, and an array of arrays, which SQLAlchemy's
-        `ARRAY` cannot hold and `CrateTypeCompiler` refuses to render.
+        A name with no type behind it yields an `UnresolvedType` under that same
+        name, so an unresolved array reports the array's name rather than its
+        element's, and the log carries the name the column actually has.
+        """
+        resolved = self._lookup_type(type_)
+        if resolved is None:
+            # Reflecting `pg_catalog` alone leaves twenty-odd columns unresolved,
+            # so this records the name for tracing rather than to raise an alarm
+            # about a schema nobody asked to reflect. What does deserve one is
+            # asking such a column for SQL, and that raises.
+            log.debug("Unable to resolve CrateDB type: %s", type_)
+            return UnresolvedType(type_)
+        return resolved
+
+    def _lookup_type(self, type_):
+        """
+        Find the SQLAlchemy type for a CrateDB type name, or `None` for a name
+        the dialect has nothing to offer for.
 
-        Whatever is left unresolved keeps the name of the type the column
-        actually has, so an unresolved array reports the array's name rather
-        than its element's.
+        The map answers first, so that `object_array` resolves to its own entry.
+        Every other name ending in `_array` names an array of the type the rest
+        of the name names, and yields an `ARRAY` of that type. Two shapes have no
+        representation: an array whose element type is itself unresolved, and an
+        array of arrays, which SQLAlchemy's `ARRAY` cannot hold and
+        `CrateTypeCompiler` refuses to render.
 
         An `ARRAY` built here carries its element type into DDL and into the
         reflected column. It converts no values, because SQLAlchemy's `ARRAY`
@@ -536,17 +551,15 @@ def _resolve_type(self, type_):
         resolved = TYPES_MAP.get(type_)
         if resolved is not None:
             return resolved
-        element_name = type_[: -len(ARRAY_SUFFIX)] if type_.endswith(ARRAY_SUFFIX) else ""
-        if element_name and not element_name.endswith(ARRAY_SUFFIX):
-            element_type = self._resolve_type(element_name)
-            if not isinstance(element_type, UnresolvedType):
-                return ARRAY(element_type)
-        # Reflecting `pg_catalog` alone leaves twenty-odd columns unresolved, so
-        # this records the name for tracing rather than to raise an alarm about
-        # a schema nobody asked to reflect. What does deserve one is asking such
-        # a column for SQL, and that raises.
-        log.debug("Unable to resolve CrateDB type: %s", type_)
-        return UnresolvedType(type_)
+        if not type_.endswith(ARRAY_SUFFIX):
+            return None
+        element_name = type_[: -len(ARRAY_SUFFIX)]
+        if not element_name or element_name.endswith(ARRAY_SUFFIX):
+            return None
+        element_type = self._lookup_type(element_name)
+        if element_type is None:
+            return None
+        return sqltypes.ARRAY(element_type)
 
     def has_ilike_operator(self):
         """

tests/reflection_test.py

@@ -26,12 +26,9 @@
 from sqlalchemy import types as sqltypes
 
 from sqlalchemy_cratedb import Geopoint, Geoshape, ObjectArray
-from sqlalchemy_cratedb.dialect import TYPES_MAP, CrateDialect
+from sqlalchemy_cratedb.dialect import ARRAY_SUFFIX, TYPES_MAP, CrateDialect
 from sqlalchemy_cratedb.sa_version import SA_1_4, SA_2_0, SA_VERSION
 
-# CrateDB names an array type after the type it holds.
-ARRAY_SUFFIX = "_array"
-
 # CrateDB reports `regproc` for the `pg_catalog` columns holding a reference to
 # a function. No table can declare one, so it has no DDL spelling to map to.
 UNRESOLVABLE = "regproc"
@@ -129,10 +126,14 @@ def test_double_precision_array_renders_as_an_array_of_its_element_type():
     assert render_type("double precision_array") == expected
 
 
-def test_every_mapped_type_has_an_array_form():
+def test_every_mapped_type_is_the_element_of_its_array_form():
     """
     A type gains its array form by being mapped, rather than by being listed a
     second time under its array name.
+
+    This pins which type the array holds. Whether that array renders is the
+    element type's business: `float_vector` needs a dimension that reflection
+    cannot recover, and refuses in its array form exactly as it does alone.
     """
     dialect = CrateDialect()
     derived = {
@@ -147,6 +148,16 @@ def test_every_mapped_type_has_an_array_form():
         assert type(resolved.item_type) is type(sqltypes.to_instance(element_type))
 
 
+def test_reflected_numeric_array_refuses_ddl_without_a_precision():
+    """
+    Reflection recovers a type from its name alone, and an array of `NUMERIC` is
+    no more storable without a precision than the scalar is.
+    """
+    with pytest.raises(sa.exc.CompileError) as ex:
+        render_ddl("numeric_array")
+    assert "CrateDB stores a NUMERIC column only with a precision" in str(ex.value)
+
+
 def test_reflected_object_array_keeps_its_own_type():
     """
     An array of `OBJECT` tracks mutation of the objects it holds, which an
@@ -209,13 +220,26 @@ def test_unresolved_type_remains_selectable():
     assert "SELECT t.c" in str(table.select().compile(dialect=dialect))
 
 
-def test_unresolved_type_is_logged(caplog):
+@pytest.mark.parametrize(
+    "data_type",
+    [UNRESOLVABLE, "{0}{1}".format(UNRESOLVABLE, ARRAY_SUFFIX)],
+)
+def test_unresolved_type_is_logged_under_the_column_type_name(caplog, data_type):
     """
-    Reflecting a type the dialect does not know leaves a record of its name.
+    Reflecting a type the dialect does not know leaves one record, naming the
+    type the column has.
+
+    An array is looked up through its element type, whose name belongs to no
+    column, and so belongs in no trace of what the map is missing.
     """
     with caplog.at_level(logging.DEBUG, logger="sqlalchemy_cratedb.dialect"):
-        CrateDialect()._resolve_type(UNRESOLVABLE)
-    assert UNRESOLVABLE in caplog.text
+        CrateDialect()._resolve_type(data_type)
+    messages = [
+        record.getMessage()
+        for record in caplog.records
+        if record.name == "sqlalchemy_cratedb.dialect"
+    ]
+    assert messages == ["Unable to resolve CrateDB type: {0}".format(data_type)]
 
 
 @pytest.mark.skipif(SA_VERSION < SA_1_4, reason="Test case not supported on SQLAlchemy 1.3")