run_name_filters makes an assumption about entity type.

[mattinbits]

Describe the bug
run_name_filters makes an assumption which breaks extending with different types of entity.

https://github.com/sqlalchemy/alembic/blob/master/alembic/autogenerate/api.py#L317

if "schema_name" in parent_names:
    if type_ == "table":
        table_name = name
    else:
        table_name = parent_names["table_name"]

This code appears to assume that if a target object has a schema as a parent, but is not a table, it must also have a table as a parent. This is true of columns, indexes, contraints and so on. However when extending Alembic, for example to implement support for replacable entities such as Views, the assumption doesn't hold true.

In the alembic_utils project, this necessitates a work around setting a dummy "table_name" to avoid a KeyError. https://github.com/olirice/alembic_utils/blob/master/src/alembic_utils/replaceable_entity.py#L378

Expected behavior
run_name_filters can accommodate target objects which are not tables but do not have tables as parents.

To Reproduce
I'm afraid I don't have a minimal example but running alembic_utils without the work around linked to above causes a KeyError when parent_names["table_name"] is accessed.

[zzzeek]

hi -

I am sort of understanding the issue as given, but not really understanding how that happens. how is a non Table object getting in there in the first place? can you please at least provide a stack trace if not an MCVE ? are you the maintainer of the alembic_utils package?

[olirice]

Hi,

I'm the author of alembic_utils, a lib to support autogenerate on views, functions, triggers etc that came out of this convo last year.

In a recent (yesterday) update we switched method of the configuration from passing a schema allowlist and denylist when registering entities with autogen, to attempting to follow the behavior described in the alembic docs for controlling what to autogenerate

Specifically, the method for excluding entities by name or object described here and here

---

how is a non Table object getting in there in the first place?

To achieve the behavior described above, each reflected and local entity is filtered through the AutogenContext.run_name_filters and AutogenContext.run_object_filters while a revision is being generated.

If helpful for context, this is a stripped down copy of the implementation

def include_entity(entity: T, autogen_context: AutogenContext, reflected: bool) -> bool:
    """decide if an entity should be included in autogeneraetd revision"""

    name = f"{entity.schema}.{entity.signature}"
    parent_names = {
        "schema_name": entity.schema,
        "table_name": f"Not applicable for type {entity.type_}",
    }
    # the name filter is only relevant for reflected objects
    if reflected:
        name_result = autogen_context.run_name_filters(name, entity.type_, parent_names)
    else:
        name_result = True

    # Object filters should be applied to object from local metadata and to reflected objects
    object_result = autogen_context.run_object_filters(
        entity, name, entity.type_, reflected=reflected, compare_to=None
    )
    return name_result and object_result

---

The issue that @mattinbits raised is that when you call AutogenContext.run_name_filters, failing to include the key "table_name" in the parent_names dictionary when type_ != "table" results in a KeyError because of this guy:

alembic/alembic/autogenerate/api.py

Line 333 in 9ec084a

table_name = parent_names["table_name"]

So I think the questions are:

• Is hooking into run_name_filters and run_object_filters your recommended way to control exclusion for extensions or is that considered internal behavior?
• If so, would you consider making AutogenContext.run_name_filters more permissive for extensions?

Thanks!

[zzzeek]

alrighty, you are calling it directly with a name such as "view", gotcha.

So does this patch work? can you come up with some tests for this so that it doesn't get broken? probably has to be it's own thing and the best file for the moment would be tests/test_autogen_diffs.py since that's where other include_name things are tested.

diff --git a/alembic/autogenerate/api.py b/alembic/autogenerate/api.py
index 030bc8b..bdcfebd 100644
--- a/alembic/autogenerate/api.py
+++ b/alembic/autogenerate/api.py
@@ -330,15 +330,16 @@ class AutogenContext(object):
             if type_ == "table":
                 table_name = name
             else:
-                table_name = parent_names["table_name"]
-            schema_name = parent_names["schema_name"]
-            if schema_name:
-                parent_names["schema_qualified_table_name"] = "%s.%s" % (
-                    schema_name,
-                    table_name,
-                )
-            else:
-                parent_names["schema_qualified_table_name"] = table_name
+                table_name = parent_names.get("table_name", None)
+            if table_name:
+                schema_name = parent_names["schema_name"]
+                if schema_name:
+                    parent_names["schema_qualified_table_name"] = "%s.%s" % (
+                        schema_name,
+                        table_name,
+                    )
+                else:
+                    parent_names["schema_qualified_table_name"] = table_name
 
         for fn in self._name_filters:
 

[sqla-tester]

Oliver Rice has proposed a fix for this issue in the master branch:

enable extensions to use AutogenContext.run_name_filters #813 https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/2620

[zzzeek]

thanks all!

[zzzeek]

1.5.7 is now released with this fix.

[olirice]

awesome, thanks!