Re: [sqlalchemy] Inconsistent SQL Generation in 2.0

2023-05-13 Thread Mike Bayer


On Sat, May 13, 2023, at 5:12 PM, Benjamin Taub wrote:
> Thank you, Mike, but aren't I using the correct dialect with this 
> create_engine() command?
> 
> *qry_engine = create_engine('mysql+pymysql://' + db_uid + ':' + db_pw 
> + '@' + db_addr, connect_args=connect_args,
>pool_recycle=3600, echo=False, 
> future=True)*
> 
> Or, am I missing something? (BTW, I'm using core, not ORM).

looks fine.  I would need to see a complete example in how you are seeing it 
generate quotes.  if you try the program I attached previously, you should note 
that it generates backticks.

> Thanks again!
> Ben 
> On Friday, May 12, 2023 at 4:48:45 PM UTC-4 Mike Bayer wrote:
>> 
>> 
>> On Fri, May 12, 2023, at 4:30 PM, Benjamin Taub wrote:
>>> I have code that worked under SQLAlchemy 1.4 but recently upgraded to 2. I 
>>> am using the add_columns() method to add columns to an existing SQL 
>>> statement. The resultant queries sometimes, but not always, crash. I 
>>> believe the issue happens when the schema/database name (I'm using MySQL) 
>>> starts with a number. When the schema name starts with a letter, the result 
>>> runs fine. However, when it starts with a number, the query double-quotes 
>>> the schema name, causing the query to crash.
>>> 
>>> Here is an example...
>>> My code: *sql = sql.add_columns(self.tbl.c[field])*
>>> 
>>> When the schema holding self.tbl.c[field] starts with a letter 
>>> (c6961a19b7ed031ce902f056c725b3e3), the following SQL is generated:
>>> *SELECT NULL AS "Application Id", NULL AS "First Name", NULL AS "Last 
>>> Name", NULL AS "Email", 
>>> c6961a19b7ed031ce902f056c725b3e3.t_31392eb2e6980f4d5082b7861182f2b4.master_key
>>>  
>>> FROM c6961a19b7ed031ce902f056c725b3e3.t_31392eb2e6980f4d5082b7861182f2b4*
>>> 
>>> However, when the schema name starts with a number 
>>> (283ac7717fe770c5ed6d425c0c739cba), the following SQL results:
>>> *SELECT NULL AS "Application Id", NULL AS "First Name", NULL AS "Last 
>>> Name", NULL AS "Email", 
>>> "283ac7717fe770c5ed6d425c0c739cba".t_59a33cbea3617986d810e9fbae60ba19.master_key
>>>  
>>> FROM "283ac7717fe770c5ed6d425c0c739cba".t_59a33cbea3617986d810e9fbae60ba19*
>>> 
>>> Note the double quotes around the schema name. This second SQL crashes as 
>>> invalid. Back quotes (`) would probably work fine in this situation, and 
>>> could be helpful, but double quotes (") are, I think, the cause of my 
>>> problem.
>>> 
>>> Is there some parameter or assumption that I'm not understanding, or did I 
>>> find a bug?
>> 
>> The quoting, if it were the correct quoting format, should be fine.   As to 
>> why it's the quote char and not the backtick, are you compiling these 
>> queries manually?You would want to make sure a MySQL dialect is in use, 
>> which would be using backticks for quoting, unless that dialect were 
>> initialized against a MySQL database that has ANSI_QUOTES set.
>> 
>> TL;DR quoting is a new thing here but SQLAlchemy should render the correct 
>> quotes when used with the MySQL dialect.
>> 
>> Here's a demo:
>> 
>> from sqlalchemy import Column
>> from sqlalchemy import create_engine
>> from sqlalchemy import Integer
>> from sqlalchemy import select
>> from sqlalchemy import String
>> from sqlalchemy.orm import declarative_base
>> Base = declarative_base()
>> 
>> 
>> class A(Base):
>> __tablename__ = 't_59a33cbea3617986d810e9fbae60ba19'
>> 
>> __table_args__ = {
>> "schema": "283ac7717fe770c5ed6d425c0c739cba"
>> }
>> id = Column(Integer, primary_key=True)
>> data = Column(String)
>> 
>> e = create_engine("mysql://")
>> 
>> stmt = select(A)
>> 
>> print(stmt.compile(e))
>> 
>> output:
>> 
>> SELECT 
>> `283ac7717fe770c5ed6d425c0c739cba`.t_59a33cbea3617986d810e9fbae60ba19.id, 
>> `283ac7717fe770c5ed6d425c0c739cba`.t_59a33cbea3617986d810e9fbae60ba19.data 
>> FROM `283ac7717fe770c5ed6d425c0c739cba`.t_59a33cbea3617986d810e9fbae60ba19
>> 
>> 
>> 
>>> 
>>> Thank you!
>>> Ben
>>> 
>>> 
>>> 
>>> 
>>> --
>>> 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/76d38390-e17f-4b90-a438-ee078944b5ffn%40googlegroups.com
>>>  
>>> .
>> 
> 
> 
> -- 
> SQLAlchemy - 
> The Python SQL Toolkit and Object Relational Mapper
>  
> http://www.sqlalchemy.org/
>  
> To post example code, please provide an MCVE: 

Re: [sqlalchemy] Inconsistent SQL Generation in 2.0

2023-05-13 Thread Benjamin Taub
For what it's worth, I believe this double quoting issue is also happening 
with the .label() method of the column object (assuming that I do have the 
dialect appropriately set). The following code...

*tbl_col = self.tbl.c[formula_col['table_column']].label(tmplt_col['name'])*

Results in the following result in the Pycharm debugger (notice the double 
quotes)...


*'s_42331a8d8a8f3ca73660cc078731dc72.t_ec37b6f3271c0f97429d5301a41be3da."ApplicationId"'*

and then running...

*sql = sql.add_columns(tbl_col)*

Results in the following...


*SELECT 
s_42331a8d8a8f3ca73660cc078731dc72.t_ec37b6f3271c0f97429d5301a41be3da."ApplicationId"
 
AS "Application Id" FROM 
s_42331a8d8a8f3ca73660cc078731dc72.t_ec37b6f3271c0f97429d5301a41be3da*

Any suggestions? Thanks for your help!
Ben

On Saturday, May 13, 2023 at 5:12:11 PM UTC-4 Benjamin Taub wrote:

> Thank you, Mike, but aren't I using the correct dialect with this 
> create_engine() command?
>
>
>
> *qry_engine = create_engine('mysql+pymysql://' + db_uid + ':' + 
> db_pw + '@' + db_addr, connect_args=connect_args,  
>  pool_recycle=3600, echo=False, future=True)*
>
> Or, am I missing something? (BTW, I'm using core, not ORM).
> Thanks again!
> Ben 
> On Friday, May 12, 2023 at 4:48:45 PM UTC-4 Mike Bayer wrote:
>
>>
>>
>> On Fri, May 12, 2023, at 4:30 PM, Benjamin Taub wrote:
>>
>> I have code that worked under SQLAlchemy 1.4 but recently upgraded to 2. 
>> I am using the add_columns() method to add columns to an existing SQL 
>> statement. The resultant queries sometimes, but not always, crash. I 
>> believe the issue happens when the schema/database name (I'm using MySQL) 
>> starts with a number. When the schema name starts with a letter, the result 
>> runs fine. However, when it starts with a number, the query double-quotes 
>> the schema name, causing the query to crash.
>>
>> Here is an example...
>> My code: *sql = sql.add_columns(self.tbl.c[field])*
>>
>> When the schema holding self.tbl.c[field] starts with a letter 
>> (c6961a19b7ed031ce902f056c725b3e3), the following SQL is generated:
>>
>> *SELECT NULL AS "Application Id", NULL AS "First Name", NULL AS "Last 
>> Name", NULL AS "Email", 
>> c6961a19b7ed031ce902f056c725b3e3.t_31392eb2e6980f4d5082b7861182f2b4.master_key
>>  
>> FROM c6961a19b7ed031ce902f056c725b3e3.t_31392eb2e6980f4d5082b7861182f2b4*
>>
>> However, when the schema name starts with a number 
>> (283ac7717fe770c5ed6d425c0c739cba), the following SQL results:
>>
>> *SELECT NULL AS "Application Id", NULL AS "First Name", NULL AS "Last 
>> Name", NULL AS "Email", 
>> "283ac7717fe770c5ed6d425c0c739cba".t_59a33cbea3617986d810e9fbae60ba19.master_key
>>  
>> FROM "283ac7717fe770c5ed6d425c0c739cba".t_59a33cbea3617986d810e9fbae60ba19*
>>
>> Note the double quotes around the schema name. This second SQL crashes as 
>> invalid. Back quotes (`) would probably work fine in this situation, and 
>> could be helpful, but double quotes (") are, I think, the cause of my 
>> problem.
>>
>> Is there some parameter or assumption that I'm not understanding, or did 
>> I find a bug?
>>
>>
>> The quoting, if it were the correct quoting format, should be fine.   As 
>> to why it's the quote char and not the backtick, are you compiling these 
>> queries manually?You would want to make sure a MySQL dialect is in use, 
>> which would be using backticks for quoting, unless that dialect were 
>> initialized against a MySQL database that has ANSI_QUOTES set.
>>
>> TL;DR quoting is a new thing here but SQLAlchemy should render the 
>> correct quotes when used with the MySQL dialect.
>>
>> Here's a demo:
>>
>> from sqlalchemy import Column
>> from sqlalchemy import create_engine
>> from sqlalchemy import Integer
>> from sqlalchemy import select
>> from sqlalchemy import String
>> from sqlalchemy.orm import declarative_base
>> Base = declarative_base()
>>
>>
>> class A(Base):
>> __tablename__ = 't_59a33cbea3617986d810e9fbae60ba19'
>>
>> __table_args__ = {
>> "schema": "283ac7717fe770c5ed6d425c0c739cba"
>> }
>> id = Column(Integer, primary_key=True)
>> data = Column(String)
>>
>> e = create_engine("mysql://")
>>
>> stmt = select(A)
>>
>> print(stmt.compile(e))
>>
>> output:
>>
>> SELECT `283ac7717fe770c5ed6d425c0c739cba`.
>> t_59a33cbea3617986d810e9fbae60ba19.id, 
>> `283ac7717fe770c5ed6d425c0c739cba`.t_59a33cbea3617986d810e9fbae60ba19.data 
>> FROM `283ac7717fe770c5ed6d425c0c739cba`.t_59a33cbea3617986d810e9fbae60ba19
>>
>>
>>
>>
>> Thank you!
>> Ben
>>
>>
>>
>> -- 
>> 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 

Re: [sqlalchemy] Migration issues - Column value None on select with joins

2023-05-13 Thread 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.

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/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:
> 

Re: [sqlalchemy] Inconsistent SQL Generation in 2.0

2023-05-13 Thread Benjamin Taub
Thank you, Mike, but aren't I using the correct dialect with this 
create_engine() command?



*qry_engine = create_engine('mysql+pymysql://' + db_uid + ':' + 
db_pw + '@' + db_addr, connect_args=connect_args,  
 pool_recycle=3600, echo=False, future=True)*

Or, am I missing something? (BTW, I'm using core, not ORM).
Thanks again!
Ben 
On Friday, May 12, 2023 at 4:48:45 PM UTC-4 Mike Bayer wrote:

>
>
> On Fri, May 12, 2023, at 4:30 PM, Benjamin Taub wrote:
>
> I have code that worked under SQLAlchemy 1.4 but recently upgraded to 2. I 
> am using the add_columns() method to add columns to an existing SQL 
> statement. The resultant queries sometimes, but not always, crash. I 
> believe the issue happens when the schema/database name (I'm using MySQL) 
> starts with a number. When the schema name starts with a letter, the result 
> runs fine. However, when it starts with a number, the query double-quotes 
> the schema name, causing the query to crash.
>
> Here is an example...
> My code: *sql = sql.add_columns(self.tbl.c[field])*
>
> When the schema holding self.tbl.c[field] starts with a letter 
> (c6961a19b7ed031ce902f056c725b3e3), the following SQL is generated:
>
> *SELECT NULL AS "Application Id", NULL AS "First Name", NULL AS "Last 
> Name", NULL AS "Email", 
> c6961a19b7ed031ce902f056c725b3e3.t_31392eb2e6980f4d5082b7861182f2b4.master_key
>  
> FROM c6961a19b7ed031ce902f056c725b3e3.t_31392eb2e6980f4d5082b7861182f2b4*
>
> However, when the schema name starts with a number 
> (283ac7717fe770c5ed6d425c0c739cba), the following SQL results:
>
> *SELECT NULL AS "Application Id", NULL AS "First Name", NULL AS "Last 
> Name", NULL AS "Email", 
> "283ac7717fe770c5ed6d425c0c739cba".t_59a33cbea3617986d810e9fbae60ba19.master_key
>  
> FROM "283ac7717fe770c5ed6d425c0c739cba".t_59a33cbea3617986d810e9fbae60ba19*
>
> Note the double quotes around the schema name. This second SQL crashes as 
> invalid. Back quotes (`) would probably work fine in this situation, and 
> could be helpful, but double quotes (") are, I think, the cause of my 
> problem.
>
> Is there some parameter or assumption that I'm not understanding, or did I 
> find a bug?
>
>
> The quoting, if it were the correct quoting format, should be fine.   As 
> to why it's the quote char and not the backtick, are you compiling these 
> queries manually?You would want to make sure a MySQL dialect is in use, 
> which would be using backticks for quoting, unless that dialect were 
> initialized against a MySQL database that has ANSI_QUOTES set.
>
> TL;DR quoting is a new thing here but SQLAlchemy should render the correct 
> quotes when used with the MySQL dialect.
>
> Here's a demo:
>
> from sqlalchemy import Column
> from sqlalchemy import create_engine
> from sqlalchemy import Integer
> from sqlalchemy import select
> from sqlalchemy import String
> from sqlalchemy.orm import declarative_base
> Base = declarative_base()
>
>
> class A(Base):
> __tablename__ = 't_59a33cbea3617986d810e9fbae60ba19'
>
> __table_args__ = {
> "schema": "283ac7717fe770c5ed6d425c0c739cba"
> }
> id = Column(Integer, primary_key=True)
> data = Column(String)
>
> e = create_engine("mysql://")
>
> stmt = select(A)
>
> print(stmt.compile(e))
>
> output:
>
> SELECT `283ac7717fe770c5ed6d425c0c739cba`.
> t_59a33cbea3617986d810e9fbae60ba19.id, 
> `283ac7717fe770c5ed6d425c0c739cba`.t_59a33cbea3617986d810e9fbae60ba19.data 
> FROM `283ac7717fe770c5ed6d425c0c739cba`.t_59a33cbea3617986d810e9fbae60ba19
>
>
>
>
> Thank you!
> Ben
>
>
>
> -- 
> 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/76d38390-e17f-4b90-a438-ee078944b5ffn%40googlegroups.com
>  
> 
> .
>
>
>

-- 
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 

Re: [sqlalchemy] Migration issues - Column value None on select with joins

2023-05-13 Thread 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.   

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/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 

Re: [sqlalchemy] Migration issues - Column value None on select with joins

2023-05-13 Thread 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

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 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 
>