Re: [sqlalchemy] Migration issues - Column value None on select with joins
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 On Sunday, May 14, 2023 at 7:42:54 AM UTC+10 Shane Holcombe wrote: > 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. > > On Sunday, May 14, 2023 at 2:37:37 AM UTC+10 Mike Bayer wrote: > >> 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. >> >> On Sat, May 13, 2023, at 3:36 AM, Shane Holcombe wrote: >> >> 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 >> On Saturday, May 13, 2023 at 1:50:56 PM UTC+10 Mike Bayer wrote: >> >> >> 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. >> >> On Fri, May 12, 2023, at 11:30 PM, Mike Bayer wrote: >> >> 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, >> >> >> >> >> On Fri, May 12, 2023, at 11:05 PM, Shane Holcombe wrote:
Re: [sqlalchemy] Migration issues - Column value None on select with joins
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. On Sunday, May 14, 2023 at 2:37:37 AM UTC+10 Mike Bayer wrote: > 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. > > On Sat, May 13, 2023, at 3:36 AM, Shane Holcombe wrote: > > 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 > On Saturday, May 13, 2023 at 1:50:56 PM UTC+10 Mike Bayer wrote: > > > 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. > > On Fri, May 12, 2023, at 11:30 PM, Mike Bayer wrote: > > 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, > > > > > On Fri, May 12, 2023, at 11:05 PM, Shane Holcombe wrote: > > 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 > On Saturday, May 13, 2023 at 12:59:33 PM UTC+10 Mike Bayer wrote: > > > 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/li
Re: [sqlalchemy] Migration issues - Column value None on select with joins
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 On Saturday, May 13, 2023 at 1:50:56 PM UTC+10 Mike Bayer wrote: > 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. > > On Fri, May 12, 2023, at 11:30 PM, Mike Bayer wrote: > > 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, > > > > > On Fri, May 12, 2023, at 11:05 PM, Shane Holcombe wrote: > > 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 > On Saturday, May 13, 2023 at 12:59:33 PM UTC+10 Mike Bayer wrote: > > > 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 ? > > > On Fri, May 12, 2023, at 10:47 PM, Mike Bayer wrote: > > OK, maybe you are onto something with the theory re: > JoinedLoader._generate_row_adapter(). will look at that > > On Fri, May 12, 2023, at 6:16 PM, Shane Holcombe wrote: > > Thanks for linking that github issue, completely missed
Re: [sqlalchemy] Migration issues - Column value None on select with joins
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 On Saturday, May 13, 2023 at 12:59:33 PM UTC+10 Mike Bayer wrote: > 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 ? > > > On Fri, May 12, 2023, at 10:47 PM, Mike Bayer wrote: > > OK, maybe you are onto something with the theory re: > JoinedLoader._generate_row_adapter(). will look at that > > On Fri, May 12, 2023, at 6:16 PM, Shane Holcombe wrote: > > 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))
Re: [sqlalchemy] Migration issues - Column value None on select with joins
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 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 On Saturday, May 13, 2023 at 1:28:27 AM UTC+10 Mike Bayer wrote: > 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 > > On Fri, May 12, 2023, at 11:25 AM, Mike Bayer wrote: > > > > 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) > > > -- > 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 > >
[sqlalchemy] Migration issues - Column value None on select with joins
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 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 -- 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+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/sqlalchemy/f6ee8641-0dc8-4eec-a79e-941ac3d814b9n%40googlegroups.com.
