mayurnewase commented on pull request #13255: URL: https://github.com/apache/superset/pull/13255#issuecomment-792256798
> Sorry, hit Approve by mistake. > > I think we need to think deeper on how to structure the SQL query generator in an extensible and maintainable way. There's nothing wrong with splitting things into various smaller functions, but considering how many variables we need to manipulate and manage, a `QueryBuilder` class might make more sense. There's definitely room for util functions, but it is better to make sure they only accept a couple of arguments (<= 4), so we don't have to pass a lot of parameters around. Yeah,I went through how sqlalchemy uses `Query` class to generate queries which makes it easier. I will try to do in that fashion in future prs if required. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
