Migration issues - Column value None on select with joins

[Shane Holcombe]

Hi there,

I've been working on migrating our codebase from SQLAlchemy 1.4 to 2.0 and have encountered an issue I can't seem to get around.

The error itself is

  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/orm/context.py", line 1101, in create_for_statement
    SelectState.__init__(self, self.statement, compiler, **kw)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/selectable.py", line 4501, in __init__
    self.froms = self._get_froms(statement)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/selectable.py", line 4599, in _get_froms
    [
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/selectable.py", line 4600, in <listcomp>
    element._from_objects
AttributeError: 'NoneType' object has no attribute '_from_objects'

(I've shortened this from the entire traceback)

From the digging around that I've done, it seems to be the ColumnLoader strategy setup_query method sometimes has a value for 'c' of None where an adapter is present but check_for_adapt is False. This is from the c = adapter.columns[c] line.
This happens only when called from the JoinedLoader class, it seems the _generate_row_adapter method returns a clauses object with a columns dict that has all the keys from the underlying table, but some of the values are None.

All of this behaviour is fairly hard to get to happen however it seems to occur after the warning "Loader depth for query is excessively deep; caching will be disabled for additional loaders.  Consider using the recursion_depth feature for deeply nested recursive eager loaders." occurs.

This warning comes from a different part of the system where the base level object, 'User', is very deeply nested, and on some later request to a different part of the system, fetching a different 'User' relationship at a less deep level triggers the above error.

In our use case we have some fairly deeply nested relationships, 6 or so levels deep, however we never used selectinload or immediateload to my knowledge so there is no way to change the recursion depth.

Any help on this would be fantastic,

Thanks,

Shane

[Mike Bayer]

this has also been reported at https://github.com/sqlalchemy/sqlalchemy/discussions/9666

On Fri, May 12, 2023, at 1:25 AM, Shane Holcombe wrote:

From the digging around that I've done, it seems to be the ColumnLoader strategy setup_query method sometimes has a value for 'c' of None where an adapter is present but check_for_adapt is False. This is from the c = adapter.columns[c] line.

This happens only when called from the JoinedLoader class, it seems the _generate_row_adapter method returns a clauses object with a columns dict that has all the keys from the underlying table, but some of the values are None.

All of this behaviour is fairly hard to get to happen however it seems to occur after the warning "Loader depth for query is excessively deep; caching will be disabled for additional loaders.  Consider using the recursion_depth feature for deeply nested recursive eager loaders." occurs.

This warning comes from a different part of the system where the base level object, 'User', is very deeply nested, and on some later request to a different part of the system, fetching a different 'User' relationship at a less deep level triggers the above error.

In our use case we have some fairly deeply nested relationships, 6 or so levels deep, however we never used selectinload or immediateload to my knowledge so there is no way to change the recursion depth.

we would need a reproduction case in order to see what this is about

Any help on this would be fantastic,

Thanks,

Shane

--

SQLAlchemy -

The Python SQL Toolkit and Object Relational Mapper

 

http://www.sqlalchemy.org/

 

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example. See http://stackoverflow.com/help/mcve for a full description.

---

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/f6ee8641-0dc8-4eec-a79e-941ac3d814b9n%40googlegroups.com.

[Mike Bayer]

On Fri, May 12, 2023, at 1:25 AM, Shane Holcombe wrote:

AttributeError: 'NoneType' object has no attribute '_from_objects'

(I've shortened this from the entire traceback)

OK what kind of traceback, is it a recursion overflow kind of traceback ?  (e.g. many hundreds of repeated series of lines)?     that would explain the nonsensical error at least (there's no way for None to be in the place that this error reports it)

[Mike Bayer]

I really need to see:

1. actual models to the degree they illustrate the default loaders set up on relationships

2. query in use

3. **complete** stack trace, put it on a github gist if it's too long.  every line, the error, top to bottom.  thanks

--

SQLAlchemy -

The Python SQL Toolkit and Object Relational Mapper

 

http://www.sqlalchemy.org/

 

To post example code, please provide an MCVE: Minimal, Complete, and Verifiable Example. See http://stackoverflow.com/help/mcve for a full description.

---

You received this message because you are subscribed to the Google Groups "sqlalchemy" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sqlalchemy+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/9762bf53-8443-4780-9091-136863716290%40app.fastmail.com.

[Shane Holcombe]

Thanks for linking that github issue, completely missed that one.

Here's the complete stack trace, sorry for not including that at the start

Traceback (most recent call last):
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/werkzeug/serving.py", line 333, in run_wsgi
    execute(self.server.app)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/werkzeug/serving.py", line 320, in execute
    application_iter = app(environ, start_response)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask/app.py", line 2551, in __call__
    return self.wsgi_app(environ, start_response)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask/app.py", line 2531, in wsgi_app
    response = self.handle_exception(e)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask_cors/extension.py", line 165, in wrapped_function
    return cors_after_request(app.make_response(f(*args, **kwargs)))
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask_restx/api.py", line 673, in error_router
    return original_handler(f)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask_restx/api.py", line 671, in error_router
    return self.handle_error(e)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask/app.py", line 2528, in wsgi_app
    response = self.full_dispatch_request()
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask/app.py", line 1825, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask_cors/extension.py", line 165, in wrapped_function
    return cors_after_request(app.make_response(f(*args, **kwargs)))
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask_restx/api.py", line 673, in error_router
    return original_handler(f)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask_restx/api.py", line 671, in error_router
    return self.handle_error(e)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask/app.py", line 1823, in full_dispatch_request
    rv = self.dispatch_request()
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask/app.py", line 1799, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask_restx/api.py", line 404, in wrapper
    resp = resource(*args, **kwargs)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask/views.py", line 107, in view
    return current_app.ensure_sync(self.dispatch_request)(**kwargs)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask_restx/resource.py", line 46, in dispatch_request
    resp = meth(*args, **kwargs)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/flask_cors/decorator.py", line 128, in wrapped_function
    resp = make_response(f(*args, **kwargs))
  File "/Users/sfh/PycharmProjects/vita/stockholm/geneva/api/auth.py", line 151, in decorated
    return f(*args, **kwargs)
  File "/Users/sfh/PycharmProjects/vita/stockholm/geneva/api/sales/lead_api.py", line 116, in get
    lead = lead_factory.extended_lead_for_id(lead_id)
  File "/Users/sfh/PycharmProjects/vita/stockholm/geneva/sales/lead_factory.py", line 59, in extended_lead_for_id
    return db.session.execute(stmt).scalar_one_or_none()
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/orm/scoping.py", line 724, in execute
    return self._proxied.execute(
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/orm/session.py", line 2232, in execute
    return self._execute_internal(
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/orm/session.py", line 2127, in _execute_internal
    result: Result[Any] = compile_state_cls.orm_execute_statement(
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/orm/context.py", line 292, in orm_execute_statement
    result = conn.execute(
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1413, in execute
    return meth(
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/elements.py", line 483, in _execute_on_connection
    return connection._execute_clauseelement(
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/engine/base.py", line 1629, in _execute_clauseelement
    compiled_sql, extracted_params, cache_hit = elem._compile_w_cache(
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/elements.py", line 671, in _compile_w_cache
    compiled_sql = self._compiler(
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/elements.py", line 288, in _compiler
    return dialect.statement_compiler(dialect, self, **kw)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/compiler.py", line 1426, in __init__
    Compiled.__init__(self, dialect, statement, **kwargs)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/compiler.py", line 867, in __init__
    self.string = self.process(self.statement, **compile_kwargs)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/compiler.py", line 912, in process
    return obj._compiler_dispatch(self, **kwargs)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/visitors.py", line 143, in _compiler_dispatch
    return meth(self, **kw)  # type: ignore  # noqa: E501
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/compiler.py", line 4593, in visit_select
    compile_state = select_stmt._compile_state_factory(
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/base.py", line 678, in create_for_statement
    return klass.create_for_statement(statement, compiler, **kw)

  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/orm/context.py", line 1101, in create_for_statement
    SelectState.__init__(self, self.statement, compiler, **kw)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/selectable.py", line 4501, in __init__
    self.froms = self._get_froms(statement)
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/selectable.py", line 4599, in _get_froms
    [
  File "/Users/sfh/env/vita/lib/python3.9/site-packages/sqlalchemy/sql/selectable.py", line 4600, in <listcomp>
    element._from_objects

AttributeError: 'NoneType' object has no attribute '_from_objects'

In this case the statement we use, extended_lead_for_id, is

from geneva.models import person_factory
stmt = select(Lead).where(Lead.id == lead_id).options(
joinedload(Lead.lead_person_client),
joinedload(Lead.lead_person_primary_contact),
joinedload(Lead.person),
joinedload(Lead.person).joinedload(person_factory.Person.contract),
joinedload(Lead.assigned_to_application_user),
joinedload(Lead.in_progress_by_application_user),
joinedload(Lead.closed_by_application_user),
joinedload(Lead.lu_client_type),
joinedload(Lead.lu_lead_origin_type),
joinedload(Lead.lu_lead_status),
joinedload(Lead.lu_lead_closed_state),
joinedload(Lead.lu_lead_lost_reason),
joinedload(Lead.lu_marketing_channel),
joinedload(Lead.lu_marketing_channel_digital),
joinedload(Lead.lu_marketing_channel_on_site),
joinedload(Lead.lu_marketing_channel_out_of_home),
joinedload(Lead.lu_marketing_channel_print),
joinedload(Lead.lu_marketing_channel_radio),
joinedload(Lead.lu_marketing_channel_referral),
joinedload(Lead.lu_marketing_channel_tv),
joinedload(Lead.lu_marketing_channel_paid_directory),
joinedload(Lead.lu_marketing_channel_direct_mail),
joinedload(Lead.referral_supplier),
joinedload(Lead.council),
joinedload(Lead.hospital),
)
return db.session.execute(stmt).scalar_one_or_none()

As for the models, we don't really use any default loaders on relationship creation, we've tended to avoid that and put the joins in the select statements we create

If you really need to see a particular model from this let me know.

Thanks again for the help

[Mike Bayer]

OK, maybe you are onto something with the theory re: JoinedLoader._generate_row_adapter(). will look at that

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/36d0bc21-98b3-46ef-9085-c786f98234fen%40googlegroups.com.

[Mike Bayer]

well, not sure yet how a None is coming in there that is sporadic, or even at all, but if that's what's happening then we would think this patch would fix the problem, the Q is does the query still produce the correct results:

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py

index 8e06c4f598..2bac1aad48 100644

--- a/lib/sqlalchemy/orm/strategies.py

+++ b/lib/sqlalchemy/orm/strategies.py

@@ -218,10 +218,10 @@ class ColumnLoader(LoaderStrategy):

             if adapter:

                 if check_for_adapt:

                     c = adapter.adapt_check_present(c)

-                    if c is None:

-                        return

                 else:

                     c = adapter.columns[c]

+                if c is None:

+                    return

 

             compile_state._append_dedupe_col_collection(c, column_collection)

can you try that patch ?

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/b84e367a-788e-4034-952d-3bc76c67d71c%40app.fastmail.com.

[Shane Holcombe]

There seems to be a few moving parts to this so it's been hard to track down, I can give that fix a try in a few hours when I'm home.

I was worried something like that might lose some columns but I'll try and see what happens.

Thanks

[Mike Bayer]

I've opened https://github.com/sqlalchemy/sqlalchemy/issues/9777 for this.

I have another theory where something might be going wrong earler in the chain.     This change would be "simpler" in that it removes something that's complicated, prone to race conditions hence seems a good possibility of being involved here.   This would be the patch:

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py

index 8e06c4f598..bd0193b905 100644

--- a/lib/sqlalchemy/orm/strategies.py

+++ b/lib/sqlalchemy/orm/strategies.py

@@ -2324,8 +2324,7 @@ class JoinedLoader(AbstractRelationshipLoader):

         else:

             context.attributes[key] = idx = context.attributes[key] + 1

 

-        if idx >= len(self._aliased_class_pool):

-            to_adapt = orm_util.AliasedClass(

+        return orm_util.AliasedClass(

                 self.mapper,

                 alias=alt_selectable._anonymous_fromclause(flat=True)

                 if alt_selectable is not None

@@ -2334,14 +2333,6 @@ class JoinedLoader(AbstractRelationshipLoader):

                 use_mapper_path=True,

             )

 

-            # load up the .columns collection on the Alias() before

-            # the object becomes shared among threads.  this prevents

-            # races for column identities.

-            inspect(to_adapt).selectable.c

-            self._aliased_class_pool.append(to_adapt)

-

-        return self._aliased_class_pool[idx]

-

     def _generate_row_adapter(

         self,

         compile_state,

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/d86f6a2d-b2c5-4d06-b9e9-f8e2977858bdn%40googlegroups.com.

[Mike Bayer]

in the below mentioned issue I've created a patch at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4615 that removes a thread-sensitive approach to generating an internal structure, which is where I suspect this might be happening (if anywhere), and then it also adds a guard against the None objects in any case.    you'd want to take a look at that and see if it resolves.

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/8debdf3a-6097-44b6-998e-4011785b15b2%40app.fastmail.com.

[Shane Holcombe]

I've tested both solutions, the first one, moving the if c is None: return down a few lines seems to work and the generated SQL still seems to be correct in the instances where that code is triggered.

From some early testing as well the other solution seems to work as well, it's hard to test 100% as this was always a bit troublesome to replicate, I had a series of events that seemed to trigger it somewhat regularly and it hasn't yet.

The idea this might be a race condition does make sense as the model that seems to more often than not return the missing columns is used basically everywhere and will be joined to from other models very frequently, often multiple times from the same query, with multiple different queries on a page load, the 'xx_application_user' model in the query I sent.

I'll test it again a few more times/ways later tonight/tomorrow morning when I get a chance again and let you know

Thanks so much

[Mike Bayer]

the second solution has not only the removal of where the theoretical "race condition" might happen, but it also removes the "None" entries in any case, so even if the "None" thing is coming from somewhere else, they won't make it to the select() statement at the end.  

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/58c796ed-d9c2-4252-ac0c-51a9d54064edn%40googlegroups.com.

[Shane Holcombe]

Yes so I'm testing the second solution with a breakpoint on the area that should filter out those Nones, I wanted to see wether the Nones are being removed or are no longer showing up. So far I haven't seen that code be used, which is great, means the race condition thing must have helped. I can't be 100% sure as it was hard to trigger but I haven't managed to just yet and I've tried what usually caused the issue. I'll keep testing some more and let you know if it shows up again but this seems much better, thank so much.

[Shane Holcombe]

I've tested all the ways I could previously trigger the issue and still, no Nones seem to even be produced to be filtered out, that fix definitely seems to have sorted the issue out

[Mike Bayer]

thanks!

To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/2f84d244-5669-4914-8d12-df903d236befn%40googlegroups.com.