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

2023-05-14 Thread Mike Bayer
thanks!

On Sun, May 14, 2023, at 6:59 PM, Shane Holcombe wrote:
> 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:
>>> 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 

Re: [sqlalchemy] Inconsistent SQL Generation in 2.0

2023-05-14 Thread Mike Bayer


On Sun, May 14, 2023, at 12:39 PM, Benjamin Taub wrote:
> Hi, Mike!
> I tried your code, and it worked after I set the dialect to mysql+pymysql.
> 
> Given this, in my case, I believe the problem stems from the fact that I am 
> starting with a generic SELECT call that isn't moored to a dialect. I start 
> with 
> *sql = select()*

Modern SQLAlchemy SQL constructs like select() have no connection to any 
dialect.Only if you are using 1.x and using the very dated "bound metadata" 
concept which is now removed, is there any truth to this.   But any SQL 
construct can be compiled with any dialect at all times (Assuming it contains 
no dialect-specific constructs).

> 
> Which I have now changed to (self.tbl is a sqlalchemy table object attached 
> to MySQL)...
> *sql = self.tbl.select()
*
> The problem now is that I have a null column object with a label that isn't 
> attached to self.tbl, and I can't figure out how to tell it to generate as 
> MySQL...
> tbl_col = null().label(tmplt_col['name'])
>  
> It is tbl_col that eventually gives me the quote problem in SQL generation. 
> Do you have any ideas for me on how to have this generate in the right 
> dialect? Am I going about this the wrong way?

I would need to see an example of what you're doing as this does not really 
indicate why there would be any problem.   as mentioned previously, any SQL 
construct when compiled is always given a dialect with which to compile with, 
and you can always pass this in.

it's still not clear here if you are using .compile() directly or not as I dont 
really have an understanding of what you're doing.


> 
> Thanks again for your help!
> Ben
> On Saturday, May 13, 2023 at 11:33:03 PM UTC-4 Mike Bayer wrote:
>> 
>> 
>> 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:
 
 

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

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

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

Re: [sqlalchemy] Inconsistent SQL Generation in 2.0

2023-05-14 Thread Benjamin Taub
Hi, Mike!
I tried your code, and it worked after I set the dialect to mysql+pymysql.

Given this, in my case, I believe the problem stems from the fact that I am 
starting with a generic SELECT call that isn't moored to a dialect. I start 
with 
*sql = select()*

Which I have now changed to (self.tbl is a sqlalchemy table object attached 
to MySQL)...

*sql = self.tbl.select()*
The problem now is that I have a null column object with a label that isn't 
attached to self.tbl, and I can't figure out how to tell it to generate as 
MySQL...
tbl_col = null().label(tmplt_col['name'])
 
It is tbl_col that eventually gives me the quote problem in SQL generation. 
Do you have any ideas for me on how to have this generate in the right 
dialect? Am I going about this the wrong way?

Thanks again for your help!
Ben
On Saturday, May 13, 2023 at 11:33:03 PM UTC-4 Mike Bayer wrote:

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