Skip to content

Add database query summary metrics - #22

Open
Alistar84 wants to merge 3 commits into
phalcon:masterfrom
Alistar84:feature/database-summary
Open

Add database query summary metrics#22
Alistar84 wants to merge 3 commits into
phalcon:masterfrom
Alistar84:feature/database-summary

Conversation

@Alistar84

@Alistar84 Alistar84 commented Sep 1, 2026

Copy link
Copy Markdown

Hello!

  • Type: new feature
  • Link to issue: N/A

In raising this pull request, I confirm the following:

  • I have read and understood the Contributing Guidelines
  • I have checked that another pull request for this purpose does not exist
  • I wrote tests for this PR
  • I have updated the relevant CHANGELOG
  • Documentation for this change is included in this PR

Small description of change:

Adds a structured summary to the Database panel with:

  • total query count;
  • duplicate runs, defined as executions of a normalized statement after its first execution;
  • accumulated SQL execution time.

For example, executing the same normalized statement three times contributes two duplicate runs. Statement normalization collapses whitespace and deliberately ignores binding values, so repeated executions of one prepared statement are grouped together.

Each repeated row exposes its total occurrences as structured data. The frontend uses that value to apply a duplicate-row CSS class and display Executed N times without changing the duration label. The summary remains visible with zero values when no queries are executed.

The collector and renderer tests cover repeated statements with different bindings, unique statements, per-row occurrence counts, summary values, and the zero-query state. The behavior is also documented in docs/index.md.

Thanks

@Alistar84
Alistar84 force-pushed the feature/database-summary branch from 7a50f9c to 4b7c8a1 Compare September 2, 2026 15:12
@Alistar84
Alistar84 marked this pull request as ready for review September 2, 2026 15:12

@niden niden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this work. Some comments that I would like see addressed - some nitpicks but some design decisions.

Thanks again, this looks super!

Comment thread src/DebugBar/Collector/DatabaseCollector.php Outdated
Comment thread tests/Unit/DebugBar/Collector/DatabaseCollectorTest.php
Comment thread src/DebugBar/Collector/DatabaseCollector.php Outdated
Comment thread src/DebugBar/Collector/DatabaseCollector.php Outdated
Comment thread src/DebugBar/Collector/DatabaseCollector.php
Comment thread src/DebugBar/Collector/DatabaseCollector.php
Comment thread tests/Unit/DebugBar/Collector/DatabaseCollectorTest.php Outdated
@Alistar84

Copy link
Copy Markdown
Author

Thanks for the review. I addressed all the requested changes in 2a2bcd6:

  • duplicate information is now exposed through a structured occurrences row attribute;
  • query durations are no longer modified;
  • duplicate rows receive a dedicated CSS class and display Executed N times;
  • summaries now use an extensible list of label/value elements with snake_case identifiers;
  • normalized query keys are computed once and reused;
  • zero-query behavior is documented;
  • duplicate detection tests now cover multiple repeated and unique statements.

The relevant tests and static analysis checks pass locally. Could you please take another look?

@niden

niden commented Sep 3, 2026

Copy link
Copy Markdown
Member

@Alistar84 Thank you for the quick turnaround. One note before I go for second pass, reminder https://docs.phalcon.io/5.18/ai-development/. If you need to change the commit messages go for it.

@niden

niden commented Sep 3, 2026

Copy link
Copy Markdown
Member

@Alistar84 Also before I forget, this will need a few lines in docs/index because it is new behavior

@niden niden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more issues to resolve.

Also note the AI doc in my other document (main thread) and please make sure to update the PR description as well since it is a bit off after these changes.

Thanks

Comment thread src/DebugBar/Collector/DatabaseCollector.php
Comment thread src/DebugBar/Collector/DatabaseCollector.php Outdated
@Alistar84
Alistar84 force-pushed the feature/database-summary branch from 2a2bcd6 to d0a78b7 Compare September 4, 2026 07:13
@Alistar84

Copy link
Copy Markdown
Author

Thanks. The latest feedback has been addressed in d0a78b7:

  • query-key caching is now expressed as an explicit index map;
  • the summary metric is named duplicate_runs and documented as executions after the first;
  • docs/index.md, the changelog, and the PR description now describe the current behavior;
  • all three commits now carry the required Assisted-by: Codex trailer.

The focused collector/renderer tests, PHPStan, code style, and collector coverage checks pass locally.

@niden niden added the enhancement New feature or request label Sep 4, 2026

@niden niden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alistar84 Two more nitpics. This is nearly ready for merge.

Thanks again

tr.appendChild(el('td', 'phalcon-debugbar-key', scalar(row.label)));
tr.appendChild(el('td', 'phalcon-debugbar-value', scalar(row.message)));
value.appendChild(el('span', 'phalcon-debugbar-message', scalar(row.message)));
if (occurrences > 1) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two checks for occurences > 1. Could we make this one?

border-radius: 999px;
background: #8a4b08;
color: #fff1d6;
font-size: 10px;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick but... Can we make this 11px since that is what we use everywhere else

@niden niden added this to Phalcon v5 Sep 5, 2026
@niden niden moved this from Backlog to In Progress in Phalcon v5 Sep 5, 2026
@github-project-automation github-project-automation Bot moved this to Backlog in Phalcon v5 Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants