Re: [PR] fix(chart-data): A lot of idle in transaction session when executing long chart data queries [superset]

["rusackas (via GitHub)" <gi...@apache.org>]

rusackas commented on PR #33300:
URL: https://github.com/apache/superset/pull/33300#issuecomment-4043488438

   Closing this one out based on the thread discussion. As @Always-prog noted after extensive investigation, the detach/reattach approach has fundamental issues — particularly around lazy-loaded relationships causing `DetachedInstanceError`, and the need for `flush` + `merge` (rather than `add`) to safely round-trip objects through a session boundary. The reviewers raised the same concerns.
   
   The good news is that @mistercrunch has been working on the same problem in #31315 with a different approach (`session.close()` + letting Flask-SQLAlchemy handle lazy resume), and has been testing it at Preset. That's probably the better place to continue the effort.
   
   If you find a clean solution to the session management problem that addresses the concerns raised here, please feel free to reopen this PR or open a fresh one. If the scope of the fix turns out to be broader and architectural, a SIP would be the right vehicle to get alignment before implementation.
   
   Thanks for the investigation and the detailed write-up — the thread is genuinely useful context for whoever picks this up next.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org