no-hup commented on PR #43990: URL: https://github.com/apache/superset/pull/43990#issuecomment-5601080346
Thanks — all three were right, and the admin one was a genuine regression. Fixed in 2e5f220. **Admin path.** You're correct, and I had this backwards in the description. The short-circuit is inside `is_editor`/`is_viewer` (`if self.is_admin(): return True`), which is *after* my helper runs, so an admin was paying three statements for relationships nothing would read. `prefetch_chart_access` now returns early on `is_admin()`, and there's a test asserting it issues zero statements in that case. Worth noting this doesn't add a new context requirement: `is_editor`/`is_viewer` already call `is_admin()` first, so anything reaching the access check needed `g.user` regardless. It just moves the same call a few lines earlier in the same request. **Parameter list.** Also right, and I checked the 999 figure against the repo rather than taking it as folklore — `migrations/shared/catalogs.py:126` sets `max_sqlite_in = 999` and `tasks/version_history_retention.py:245` documents the same floor. It now selects through `dashboard_slices` on `dashboard.id`, so it's one bind parameter no matter how many charts. **Test.** Fair, and the old assertion really was too weak. It now counts statements at 3 charts and 25 and requires them equal. I checked it actually catches the case you described by temporarily swapping in a loop-and-touch implementation: ``` prefetch scaled with chart count: 8 statements for 3 charts, 52 for 25 ``` With the real implementation both sizes are 3 — the slice query plus one per relationship. `pytest tests/unit_tests/dao tests/unit_tests/dashboards` is 258 passed. One thing I did not check: any of this against a real Postgres or the integration suite — I only ran the unit tests locally. -- 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]
