ktmud commented on PR #23908: URL: https://github.com/apache/superset/pull/23908#issuecomment-1533753058
I agree this logic is probably better to live in the backend---it's more useful to record the original SQL users wrote, too. Not sure how comments are removed in the backend but my guess is it happens after we render the Jinja template (which is why we saw the error in https://github.com/apache/superset/pull/23378 in the first place). Maybe we can change it to remove comments before rendering, but how would you know the comments are safe to remove without rendering the template? Also, if we are removing comments using a SQL parser, the parser is likely to fail before you render the template. Not sure what's the best way forward, but since we are already remove comments in the frontend, it's probably still worth merging this PR first. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
