warn and skip for FKs that refer to invisible cols for Oracle

Supported use case for foreign key constraints where the local column is
marked as "invisible". The errors normally generated when a
:class:`.ForeignKeyConstraint` is created that check for the target column
are disabled when reflecting, and the constraint is skipped with a warning
in the same way which already occurs for an :class:`.Index` with a similar
issue.

tests are added for indexes, unique constraints, and primary key
constraints, which were already working; indexes and uniques warn,
primary keys don't which we would assume is because we never see those
PK columns in the first place.

Constraints now raise an informative ConstraintColumnNotFoundError
in the general case for strings in the "pending colargs" collection
not being resolvable.

Fixes: #9059
Change-Id: I400cf0bff6abba0e0c75f38b07617be1a8ec3453

diff --git a/doc/build/changelog/unreleased_20/9059.rst b/doc/build/changelog/unreleased_20/9059.rst
new file mode 100644 (file)
index 0000000..2fc665d
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/9059.rst
@@ -0,0 +1,10 @@
+.. change::
+    :tags: bug, oracle
+    :tickets: 9059
+
+    Supported use case for foreign key constraints where the local column is
+    marked as "invisible". The errors normally generated when a
+    :class:`.ForeignKeyConstraint` is created that check for the target column
+    are disabled when reflecting, and the constraint is skipped with a warning
+    in the same way which already occurs for an :class:`.Index` with a similar
+    issue.

diff --git a/lib/sqlalchemy/engine/reflection.py b/lib/sqlalchemy/engine/reflection.py
index 7e1fca0e513d9d8e177e348313afed3c1dd3dfa0..941590b13120978086517f22f1741818085f0cdf 100644 (file)
--- a/lib/sqlalchemy/engine/reflection.py
+++ b/lib/sqlalchemy/engine/reflection.py
@@ -1786,16 +1786,25 @@ class Inspector(inspection.Inspectable["Inspector"]):
             else:
                 options = {}
 
-            table.append_constraint(
-                sa_schema.ForeignKeyConstraint(
-                    constrained_columns,
-                    refspec,
-                    conname,
-                    link_to_name=True,
-                    comment=fkey_d.get("comment"),
-                    **options,
+            try:
+                table.append_constraint(
+                    sa_schema.ForeignKeyConstraint(
+                        constrained_columns,
+                        refspec,
+                        conname,
+                        link_to_name=True,
+                        comment=fkey_d.get("comment"),
+                        **options,
+                    )
+                )
+            except exc.ConstraintColumnNotFoundError:
+                util.warn(
+                    f"On reflected table {table.name}, skipping reflection of "
+                    "foreign key constraint "
+                    f"{conname}; one or more subject columns within "
+                    f"name(s) {', '.join(constrained_columns)} are not "
+                    "present in the table"
                 )
-            )
 
     _index_sort_exprs = {
         "asc": operators.asc_op,

diff --git a/lib/sqlalchemy/exc.py b/lib/sqlalchemy/exc.py
index eb633ec8ce7ac9aa08dd7f63d8a120ec75b418f3..82d9d31cee16f02fb0a3c1f72688e8aeecd5f7c1 100644 (file)
--- a/lib/sqlalchemy/exc.py
+++ b/lib/sqlalchemy/exc.py
@@ -165,6 +165,15 @@ class AmbiguousForeignKeysError(ArgumentError):
     between two selectables during a join."""
 
 
+class ConstraintColumnNotFoundError(ArgumentError):
+    """raised when a constraint refers to a string column name that
+    is not present in the table being constrained.
+
+    .. versionadded:: 2.0
+
+    """
+
+
 class CircularDependencyError(SQLAlchemyError):
     """Raised by topological sorts when a circular dependency is detected.
 

diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index 62e962a323b9e97f96baaef171a3aa7dc4a38472..5083940f063c206d2b6b2955f18ec67a2b87620e 100644 (file)
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -4014,10 +4014,17 @@ class ColumnCollectionMixin:
             assert len(result) == len(self._pending_colargs)
             return result
         else:
-            return [
-                parent.c[col] if isinstance(col, str) else col
-                for col in self._pending_colargs
-            ]
+            try:
+                return [
+                    parent.c[col] if isinstance(col, str) else col
+                    for col in self._pending_colargs
+                ]
+            except KeyError as ke:
+                raise exc.ConstraintColumnNotFoundError(
+                    f"Can't create {self.__class__.__name__} "
+                    f"on table '{parent.description}': no column "
+                    f"named '{ke.args[0]}' is present."
+                ) from ke
 
     def _set_parent(self, parent: SchemaEventTarget, **kw: Any) -> None:
         assert isinstance(parent, (Table, Column))
@@ -4519,14 +4526,7 @@ class ForeignKeyConstraint(ColumnCollectionConstraint):
         assert isinstance(table, Table)
         Constraint._set_parent(self, table)
 
-        try:
-            ColumnCollectionConstraint._set_parent(self, table)
-        except KeyError as ke:
-            raise exc.ArgumentError(
-                "Can't create ForeignKeyConstraint "
-                "on table '%s': no column "
-                "named '%s' is present." % (table.description, ke.args[0])
-            ) from ke
+        ColumnCollectionConstraint._set_parent(self, table)
 
         for col, fk in zip(self._columns, self.elements):
             if not hasattr(fk, "parent") or fk.parent is not col:

diff --git a/test/base/test_except.py b/test/base/test_except.py
index 2bf7d1eb8cf18d7a8bc68531429e67216abdfbf3..9353d28ab9fb43714f4bbddf3924818dcb14e5bc 100644 (file)
--- a/test/base/test_except.py
+++ b/test/base/test_except.py
@@ -429,6 +429,7 @@ ALL_EXC = [
         [
             sa_exceptions.ArgumentError,
             sa_exceptions.DuplicateColumnError,
+            sa_exceptions.ConstraintColumnNotFoundError,
             sa_exceptions.NoSuchModuleError,
             sa_exceptions.NoForeignKeysError,
             sa_exceptions.AmbiguousForeignKeysError,

diff --git a/test/dialect/oracle/test_reflection.py b/test/dialect/oracle/test_reflection.py
index afe9ad7aa8a339c510ab01d79b42eb2c7f5d988f..0ac991d2a288066db7b297e6e641f274d20e3ce7 100644 (file)
--- a/test/dialect/oracle/test_reflection.py
+++ b/test/dialect/oracle/test_reflection.py
@@ -37,6 +37,7 @@ from sqlalchemy.testing import AssertsCompiledSQL
 from sqlalchemy.testing import config
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing import expect_raises
+from sqlalchemy.testing import expect_warnings
 from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import is_
 from sqlalchemy.testing import is_true
@@ -345,20 +346,24 @@ class MultiSchemaTest(fixtures.TestBase, AssertsCompiledSQL):
         )
 
 
-class ConstraintTest(fixtures.TablesTest):
+class ConstraintTest(AssertsCompiledSQL, fixtures.TestBase):
 
     __only_on__ = "oracle"
     __backend__ = True
-    run_deletes = None
 
-    @classmethod
-    def define_tables(cls, metadata):
-        Table("foo", metadata, Column("id", Integer, primary_key=True))
+    @testing.fixture
+    def plain_foo_table(self, metadata, connection):
+        foo = Table("foo", metadata, Column("id", Integer, primary_key=True))
+        foo.create(connection)
+        return foo
+
+    def test_oracle_has_no_on_update_cascade(
+        self, metadata, connection, plain_foo_table
+    ):
 
-    def test_oracle_has_no_on_update_cascade(self, connection):
         bar = Table(
             "bar",
-            self.tables_test_metadata,
+            metadata,
             Column("id", Integer, primary_key=True),
             Column(
                 "foo_id", Integer, ForeignKey("foo.id", onupdate="CASCADE")
@@ -368,14 +373,17 @@ class ConstraintTest(fixtures.TablesTest):
 
         bat = Table(
             "bat",
-            self.tables_test_metadata,
+            metadata,
             Column("id", Integer, primary_key=True),
             Column("foo_id", Integer),
             ForeignKeyConstraint(["foo_id"], ["foo.id"], onupdate="CASCADE"),
         )
         assert_warns(exc.SAWarning, bat.create, connection)
 
-    def test_reflect_check_include_all(self, connection):
+    def test_reflect_check_include_all(
+        self, metadata, connection, plain_foo_table
+    ):
+
         insp = inspect(connection)
         eq_(insp.get_check_constraints("foo"), [])
         eq_(
@@ -386,6 +394,150 @@ class ConstraintTest(fixtures.TablesTest):
             ['"ID" IS NOT NULL'],
         )
 
+    @testing.fixture
+    def invisible_fk_fixture(self, metadata, connection):
+        Table("table_b", metadata, Column("id", Integer, primary_key=True))
+        Table(
+            "table_a",
+            metadata,
+            Column("id", Integer, primary_key=True),
+            Column("a_col1", Integer),
+        )
+        metadata.create_all(connection)
+
+        connection.exec_driver_sql(
+            "alter table table_a modify (a_col1 invisible)"
+        )
+
+        connection.exec_driver_sql(
+            "alter table table_a add constraint FK_table_a_a_col1 "
+            "foreign key(a_col1) references table_b"
+        )
+
+    @testing.fixture
+    def invisible_index_fixture(self, metadata, connection):
+        Table(
+            "table_a",
+            metadata,
+            Column("id", Integer, primary_key=True),
+            Column("a_col1", Integer),
+            Index("idx_col1", "a_col1"),
+        )
+        metadata.create_all(connection)
+
+        connection.exec_driver_sql(
+            "alter table table_a modify (a_col1 invisible)"
+        )
+
+    @testing.fixture
+    def invisible_uq_fixture(self, metadata, connection):
+        Table(
+            "table_a",
+            metadata,
+            Column("id", Integer, primary_key=True),
+            Column("a_col1", Integer),
+            UniqueConstraint("a_col1", name="uq_col1"),
+        )
+        metadata.create_all(connection)
+
+        connection.exec_driver_sql(
+            "alter table table_a modify (a_col1 invisible)"
+        )
+
+    @testing.fixture
+    def invisible_pk_fixture(self, metadata, connection):
+        Table(
+            "table_a",
+            metadata,
+            Column("id", Integer, primary_key=True),
+            Column("a_col1", Integer),
+        )
+        Table(
+            "table_b",
+            metadata,
+            Column("comp_id1", Integer, primary_key=True),
+            Column("comp_id2", Integer, primary_key=True),
+            Column("a_col1", Integer),
+        )
+        metadata.create_all(connection)
+
+        connection.exec_driver_sql("alter table table_a modify (id invisible)")
+        connection.exec_driver_sql(
+            "alter table table_b modify (comp_id2 invisible)"
+        )
+
+    def test_no_resolve_fks_w_invisible(
+        self, connection, invisible_fk_fixture
+    ):
+        metadata_reflect = MetaData()
+
+        with expect_warnings(
+            r"On reflected table table_a, skipping reflection of foreign key "
+            r"constraint fk_table_a_a_col1; one or more subject columns "
+            r"within name\(s\) a_col1 are not present in the table"
+        ):
+            metadata_reflect.reflect(connection)
+
+        ta = metadata_reflect.tables["table_a"]
+        tb = metadata_reflect.tables["table_b"]
+        self.assert_compile(
+            select(ta, tb),
+            "SELECT table_a.id, table_b.id AS id_1 FROM table_a, table_b",
+        )
+
+    def test_no_resolve_idx_w_invisible(
+        self, connection, invisible_index_fixture
+    ):
+        metadata_reflect = MetaData()
+
+        with expect_warnings(
+            r"index key 'a_col1' was not located in columns "
+            r"for table 'table_a'"
+        ):
+            metadata_reflect.reflect(connection)
+
+        ta = metadata_reflect.tables["table_a"]
+        self.assert_compile(
+            select(ta),
+            "SELECT table_a.id FROM table_a",
+        )
+
+    def test_no_resolve_uq_w_invisible(self, connection, invisible_uq_fixture):
+        metadata_reflect = MetaData()
+
+        with expect_warnings(
+            r"index key 'a_col1' was not located in columns "
+            r"for table 'table_a'"
+        ):
+            metadata_reflect.reflect(connection)
+
+        ta = metadata_reflect.tables["table_a"]
+        self.assert_compile(
+            select(ta),
+            "SELECT table_a.id FROM table_a",
+        )
+
+    def test_no_resolve_pk_w_invisible(self, connection, invisible_pk_fixture):
+        metadata_reflect = MetaData()
+
+        metadata_reflect.reflect(connection)
+
+        # single col pk fully invisible
+        ta = metadata_reflect.tables["table_a"]
+        eq_(list(ta.primary_key), [])
+        self.assert_compile(
+            select(ta),
+            "SELECT table_a.a_col1 FROM table_a",
+        )
+
+        # composite pk one col invisible
+        tb = metadata_reflect.tables["table_b"]
+        eq_(list(tb.primary_key), [tb.c.comp_id1])
+        self.assert_compile(
+            select(tb),
+            "SELECT table_b.comp_id1, table_b.a_col1 FROM table_b",
+        )
+
 
 class SystemTableTablenamesTest(fixtures.TestBase):
     __only_on__ = "oracle"

diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py
index 56bb89541eb63351a1c310ba9f7694f50d8cee35..7f6bee72bf209a8e0bb0e2ab1517e25235d0cf86 100644 (file)
--- a/test/sql/test_metadata.py
+++ b/test/sql/test_metadata.py
@@ -3946,11 +3946,12 @@ class ConstraintTest(fixtures.TestBase):
 
     def test_raise_index_nonexistent_name(self):
         m = MetaData()
-        # the KeyError isn't ideal here, a nicer message
-        # perhaps
-        assert_raises(
-            KeyError, Table, "t", m, Column("x", Integer), Index("foo", "q")
-        )
+
+        with expect_raises_message(
+            exc.ConstraintColumnNotFoundError,
+            "Can't create Index on table 't': no column named 'q' is present.",
+        ):
+            Table("t", m, Column("x", Integer), Index("foo", "q"))
 
     def test_raise_not_a_column(self):
         assert_raises(exc.ArgumentError, Index, "foo", 5)