`orm.composite` with polymorphic "selectin" loading raises `AttributeError: _wildcard_token` about sqlalchemy

[kkroening]

I think I see the problem:

Mapper._subclass_load_via_in loops through all the mapper attributes and attempts to use disable_opt._set_generic_strategy to set a "do_nothing" strategy option:

        # ... in `Mapper._subclass_load_via_in(...)` - trimmed for brevity:

        for prop in self.attrs:
            if prop.key not in self.class_manager:
                continue

            if prop.parent in classes_to_include or prop in keep_props:
                if not isinstance(prop, StrategizedProperty):
                    continue

                enable_opt = enable_opt._set_generic_strategy(
                    (getattr(entity.entity_namespace, prop.key),),
                    dict(prop.strategy_key),
                    _reconcile_to_other=True,
                )
            else:
                disable_opt = disable_opt._set_generic_strategy(  # (blows up here, deep in PropRegistry)
                    (getattr(entity.entity_namespace, prop.key),),
                    {"do_nothing": True},
                    _reconcile_to_other=False,
                )

The else case is where it's blowing up, deep down in PropRegistry where it tries to access prop._wildcard_token which comes from StrategizedProperty.

---

Note: The PropRegistry class has an annotation that claims prop: MapperProperty[Any], but apparently it actually expects the prop to be a StrategizedProperty - a more specialized subclass of MapperProperty, which can be seen in the following SQLAlchemy class hierarchy diagram:

Notice towards the bottom, highlighted in green: the typical ColumnProperty and RelationshipProperty implement StrategizedProperty, but CompositeProperty does not.

---

The code path in question above (disable_opt._set_generic_strategy(...)) apparently works fine as long as it's dealing with a RelationshipProperty - e.g. ColumnProperty or RelationshipProperty - but if it trips over a CompositeProperty, the _set_generic_strategy blows up downstream with the AttributeError: _wildcard_token.

It seems that in the if case, there's already a check to skip over non-StrategizedProperty attributes (which would avoid tripping over CompositeProperty), so maybe that check just needs to be moved up a level so that it applies more broadly.

         for prop in self.attrs:
             if prop.key not in self.class_manager:
                 continue

+            if not isinstance(prop, StrategizedProperty):
+                continue
+
             if prop.parent in classes_to_include or prop in keep_props:
-                if not isinstance(prop, StrategizedProperty):
-                    continue
-
                 enable_opt = enable_opt._set_generic_strategy(
                     (getattr(entity.entity_namespace, prop.key),),
                     dict(prop.strategy_key),
                     _reconcile_to_other=True,
                 )
             else:
                 disable_opt = disable_opt._set_generic_strategy(
                     (getattr(entity.entity_namespace, prop.key),),
                     {"do_nothing": True},
                     _reconcile_to_other=False,
                 )

The change indeed fixes the issue shown in the "To Reproduce" example script above.

Not sure if there are any other gotchas with the proposed fix, but hopefully this info helps.

[zzzeek]

Hi, thanks for that research. I havent had time to look at this one yet but it was clear based on use of relatively new features in sqlalchemy this was some kind of bug. Usually I can turn these around in half a day but am traveling this week, which is adding to delay on this end.

[kkroening]

No worries, @zzzeek . FWIW, we're unblocked by using the inline polymorphic loader pattern for now, so mainly just surfacing the issue in case anyone else runs into it. Thanks for all your hard work and dedication over the years with SQLAlchemy.

[CaselIT]

thanks for the analysis.

@kkroening that graph is interesting, did you use a particular tool or was it done manually?

[sqla-tester]

Federico Caselli has proposed a fix for this issue in the main branch:

try the proposed patch https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/5253

[CaselIT]

Uploaded the proposed patch to see if it breaks the ci

[kkroening]

@kkroening that graph is interesting, did you use a particular tool or was it done manually?

It was just a quick one-off script using graphviz:

import graphviz  # type: ignore
import importlib
from collections.abc import Iterable
from typing import Any


def _should_visit_class(cls: type[Any]) -> bool:
    # TBD: customize as desired to skip uninteresting classes.
    return cls is not object and cls.__module__ not in {'typing', 'abc'}


def _get_base_classes(cls: type[Any]) -> Iterable[type[Any]]:
    return filter(_should_visit_class, cls.__bases__)


def _find_classes(
    subclasses: set[type[Any]],
) -> set[type[Any]]:
    classes = set()

    def visit(cls: type[Any]) -> None:
        classes.add(cls)
        for base_class in _get_base_classes(cls):
            visit(base_class)

    for cls in filter(_should_visit_class, subclasses):
        visit(cls)
    return classes


def _get_class_label(
    cls: type[Any],
) -> str:
    return f'{cls.__module__}.{cls.__qualname__}'


def visualize_class_hierarchy(
    subclasses: set[type[Any]],
) -> graphviz.Digraph:
    classes = _find_classes(subclasses)
    module_names = {x.__module__ for x in classes}

    graph = graphviz.Digraph(
        comment='Class Hierarchy',
        format='svg',
        engine='dot',
        graph_attr=dict(
            rankdir='BT',
            nodesep='0.4',
            ranksep='1.5',
        ),
        node_attr=dict(
            fillcolor='snow',
            fontname='Input',
            shape='box',
            style='filled',
        ),
    )

    for module_name in module_names:
        with graph.subgraph(
            name='cluster_' + module_name.replace('.', '_'),
        ) as subgraph:
            subgraph.attr(
                'graph',
                penwidth='2',
                fillcolor='lightsteelblue1',
                fontname='Input Bold',
                fontsize='18',
                label=module_name,
                labelloc='b',
                style='filled',
            )
            module_classes = {x for x in classes if x.__module__ == module_name}
            for cls in module_classes:
                cls_label = _get_class_label(cls)
                node_kwargs: dict[str, Any] = {}
                if cls in subclasses:
                    node_kwargs.update(fillcolor='lightgreen')
                subgraph.node(
                    cls_label,
                    label=cls.__name__,
                    **node_kwargs,
                )

    for cls in classes:
        cls_label = _get_class_label(cls)
        for parent_class in _get_base_classes(cls):
            parent_label = _get_class_label(parent_class)
            graph.edge(cls_label, parent_label)

    return graph

Example usage:

import sqlalchemy.orm

visualize_class_hierarchy(
    {
        sqlalchemy.orm.ColumnProperty,
        sqlalchemy.orm.Composite,
        sqlalchemy.orm.RelationshipProperty,
        sqlalchemy.orm.interfaces.StrategizedProperty,
    }
)

[CaselIT]

interesting, thanks for sharing!

[zzzeek]

pretty strange we have a unit test for this exact case. sometihng clearly different here

[sqla-tester]

Mike Bayer has proposed a fix for this issue in the main branch:

only consider column / relationship attrs for subclass IN https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/5271

[sqla-tester]

Mike Bayer has proposed a fix for this issue in the rel_2_0 branch:

only consider column / relationship attrs for subclass IN https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/5272