search

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
719 stars 820 forks source link

NEW Add a trace comment for queries in dev mode #11065

Closed GuySartorelli closed 5 months ago

GuySartorelli commented 7 months ago

Issue

GuySartorelli commented 7 months ago

mariadb failure is unrelated.

GuySartorelli commented 5 months ago

Rebased to resolve that old mariadb failure

GuySartorelli commented 5 months ago

Try installing debugbar - it also shows where sql originated from, though in some cases there were different results Either this is correct and debugbar is wrong, or vice versa

Looks like both were wrong 😅 I've made the tracing more robust here. See https://github.com/lekoala/silverstripe-debugbar/issues/152 for the problem in debugbar.

Also there were some cases where debugbar was getting the source from and this wasn't

I've identified a couple of scenarios where that happens - one is Permission::permissions_for_member() and the other is Versioned::determineVersionNumberByStage(). Both of those skip this code, by making truly raw SQL queries. In those cases, I don't think we should try to be clever and inject stuff into the query - if it's a truly raw query like this it should be left untouched. Probably we should update those queries to use the ORM but that's not in scope for this card.

Also probably exclude running this code if debugbar is running, as there's no need to output where the call originates from twice

I'm not going to add coupling from framework to debugbar. I'll was going to open a card in debugbar to strip these comments since it has its own way of pointing to the query origin - but now that this PR introduces config to opt into the trace comment, developers should just not opt in if they're using debugbar.

GuySartorelli commented 5 months ago

I've removed the withEnvironmentType() stuff now, since we're using config and environment variables to control whether comments are included or not.

GuySartorelli commented 5 months ago

Noticed a handful of queries not getting comments when running ?showqueries=1 inside the cms

I addressed that in https://github.com/silverstripe/silverstripe-framework/pull/11065#issuecomment-1933091154

Also I think this is worth being to be able to enable via an environment variable so that platform engineers can switch it on more easily

Done