Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.x] Ability to log fake queries #9695

Merged
merged 34 commits into from
Apr 1, 2024
Merged

[5.x] Ability to log fake queries #9695

merged 34 commits into from
Apr 1, 2024

Conversation

JohnathonKoster
Copy link
Contributor

@JohnathonKoster JohnathonKoster commented Mar 10, 2024

Edit by Jason:

It's enabled by default if you have debug enabled. But you can override it and disable it if you want in config/statamic/system.php like this now:

-'fake_sql_queries' => config('app.debug'),
+'fake_sql_queries' => false,

Queries will show up in Ray or Debugbar just like a regular database query. There's no need to opt into dumping values.

SQLite is now required (only for this fake query feature).


This PR adds the ability to log Stache "queries" like regular database queries. This is disabled by default. It can be configured by setting new configuration values within config/statamic/stache.php configuration file:

return [

    // ....

    /*
    |--------------------------------------------------------------------------
    | Query Logging
    |--------------------------------------------------------------------------
    |
    | When enabled, the Stache query builders will log queries
    | like normal SQL Queries. To log actual query values,
    | set the dump_values configuration option to true.
    |
    */

    'query_logging' => [
        'enabled' => false,
        'dump_values' => false,
    ],

];
  • query_logging.enabled: Determines whether the query logger is enabled
  • query_logging.dump_values: Determines whether the query logger will dump "real" values or placeholders

When enabled, the query logger will emit Laravel's Illuminate\Database\Events\QueryExecuted event. This event will receive a database connection whose closure resolves to null, and no bindings.

By emitting this event existing tools, like Ray, DebugBar or Telescope will be able to pick up on the executed flat-file queries.

Example with dump_values set to false:

image

Example with dump_values set to true:

image

Important Note: The goal of this feature was not to output perfectly valid SQL statements, but rather to be able to observe what the Stache builders are doing in a familiar way with existing tools.

@duncanmcclean
Copy link
Member

It might be nice if calling ->dd() on a query gave you the same thing?

Uses a real real in-memory sqlite connection. Small price to pay to make it simpler.

Remove config option as we'll now always put values into bindings.

Now that the QueryExecuted event is populated with more realistic data, bindings are automatically populated within the SQL when using Ray.
- rename trait to fakesqueries
- rework to a closure based method that does the timing etc
- add to iterator builder
- pass table name rather than store, and add methods for figuring out the names
- add to iterator builder (e.g. search)
- pass the query into the dumper class instead of a bunch of arguments
- use a method on the trait to get all the appropriate wheres etc for the dumper
- using that new method, provide the additional where clauses for the entries query that way instead of a "extra from statement"
- remove dump trait method in favor of creating a new dumper
… filtering pages etc. move it to just the search query.
…e builders. removed the condition entirely. not sure when there would be no builder.
this allows to avoid the `$this->columns ?? $columns` situation. `$this->columns` will always be what you need.

also fixes `$query->get(['col1', 'col2'])` not showing proper `select`ed columns within our fake queries
@jasonvarga
Copy link
Member

jasonvarga commented Apr 1, 2024

This was really cool. I couldn't help but make a bunch of tweaks. Now it looks more consistent with what happens with regular Laravel DB/Eloquent queries.

CleanShot 2024-04-01 at 15 50 00

CleanShot 2024-04-01 at 15 49 29

  • The event contains actual bindings so now we just always track them and they'll get replaced in the output where appropriate. There's no need for a separate opt-in setting for seeing values.
  • Moved things around quite a bit. Made it more generic. We have non-Stache query builders (like search).
  • I was able to clean things up significantly by using a real DB connection to an in-memory SQL database. The small downside it SQLite is required for this. Only for this feature though, not the whole CMS. Worth the trade off IMO.

@jasonvarga jasonvarga changed the title [5.x] Adds support for logging Stache queries [5.x] Adds support for logging fake queries Apr 1, 2024
@jasonvarga jasonvarga changed the title [5.x] Adds support for logging fake queries [5.x] Ability to log fake queries Apr 1, 2024
@jasonvarga jasonvarga merged commit 6b626b8 into statamic:master Apr 1, 2024
19 checks passed
@jasonvarga jasonvarga deleted the adds-stache-query-logging-support branch April 1, 2024 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants