nicob3y commented on PR #35662: URL: https://github.com/apache/superset/pull/35662#issuecomment-6475093984
Thanks for the feedback. I actually decided to pivot on the implementation here: I’ve fully reverted the core changes to the existing dataset macro - while keeping the introduction of the alias - and instead introduced a brand new get_dataset_id macro. This new approach neatly resolves both of your architectural concerns: 1. The ValueError propagation: Since we are no longer tweaking the core logic inside the original macro, that ValueError edge case is entirely avoided. 2. The no-qualifier name lookup ambiguity: The new get_dataset_id macro automatically inherits the current query's database context. This explicitly scopes the name lookup by default and completely eliminates the risk of cross-database resolution ambiguity. The main benefit of introducing get_dataset_id is that it can now be used in tandem with the dataset or metric macro. I've also cleaned up the redundant skip_base_filter check in daos/dataset.py to keep it consistent with the rest of the BaseDAO helpers. Let me know what you think of this refactor! -- 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]
