Skip to content

Count the online visitors in SQL instead of in PHP - #65

Draft
boo-code wants to merge 1 commit into
PrestaShop:devfrom
boo-code:fix/count-online-visitors-in-sql-34227
Draft

Count the online visitors in SQL instead of in PHP#65
boo-code wants to merge 1 commit into
PrestaShop:devfrom
boo-code:fix/count-online-visitors-in-sql-34227

Conversation

@boo-code

@boo-code boo-code commented Sep 6, 2026

Copy link
Copy Markdown
Questions Answers
Description? The Online Visitors tile only ever used the NUMBER of rows its query returned - it ran executeS() and then read NumRows(). Every online visitor was therefore carried into PHP with five columns each and discarded, and the database was asked to sort them by date for an order nothing reads. Both branches now count in SQL. The two joins to page and page_type only produced the page name the hook discards and both join on a primary key, so removing them cannot change the count; connections_page is the one table that can match a connection more than once, which is what the GROUP BY collapsed and COUNT(DISTINCT) still does. Reading the value with getValue() also replaces a pair of Db::getInstance() lookups that asked one instance for the query and another for its row count.
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? Fixes PrestaShop/PrestaShop#34227.
How to test? With PS_STATSDATA_CUSTOMER_PAGESVIEWS on, browse the front office from a few guest sessions and check the Online Visitors tile still shows the same number as before this change. EXPLAIN on the query the hook builds should no longer report Using temporary; Using filesort, and the plan should have three tables instead of five.

What this does and does not fix

It does not, on its own, fix the slow query reported in PrestaShop/PrestaShop#34227, and the PR should
not be read as claiming that. Measured on a seeded scratch database (400k connections,
1.2M connections_page, 400k guest, MySQL 8.4, medians of 5):

                         30-min window (default)   whole table
current query                      530 ms             1717 ms
this PR                            499 ms              830 ms
current query      + index          34 ms             1403 ms
this PR            + index          34 ms              843 ms

At the default DASHACTIVITY_VISITOR_ONLINE of 30 minutes this is worth about 6%, because the
dominant cost is elsewhere: ps_connections_page carries no index but its primary key
(id_connections, id_page, time_start), so the time_start window cannot be seeked and the plan
full-scans ps_connections. That is a core db_structure.sql change; the measurement for it is on
PrestaShop/PrestaShop#39960, which is open, approved and already adds indexes to that table.

What this PR is worth on its own merits: the plan loses Using temporary; Using filesort and two
join steps, the result stops being transferred to PHP row by row, and the gain grows with the number
of visitors actually online - 1717 ms to 830 ms when the window covers the whole table.

Correctness

COUNT(DISTINCT c.id_connections) is the exact equivalent of counting the GROUP BY c.id_connections
rows. connections_page is the only table that can match a connection more than once - page,
page_type and guest are all joined on their primary key - and the cp.time_end IS NULL and
time_start predicates sit in the WHERE, so nothing about the count depended on the LEFT-ness of
the joins that are removed.

Checked against a control rather than asserted, since a plain COUNT(*) would have been wrong:

same window, without DISTINCT   120 rows
same window, with DISTINCT       42 rows      <- matches the old query's row count
whole table, old GROUP BY    257143 rows
whole table, COUNT(DISTINCT) 257143

The other branch (PS_STATSDATA_CUSTOMER_PAGESVIEWS off, which is the default) has no GROUP BY and
no join that can fan out, so COUNT(*) is its equivalent; it already uses the existing date_add
key (type=range, rows=54) and is not the slow one.

Both branches were run through PrestaShop's own DB layer on 9.2.0 with dashactivity 2.1.2 and return
an integer; Shop::addSqlRestriction(false, 'c') renders as AND c.id_shop IN (1), which is the
shape the benchmarks used. dashactivity has no PHPUnit harness (tests/ is index.php, php,
phpstan.sh), so the evidence above stands in for a unit test.

Side effect worth noting

The GROUP BY c.id_connections selecting a non-aggregated pt.name is only legal because PrestaShop
blanks sql_mode on connect (DbPDO.php:120, DbMySQLi.php:71); under the MySQL default
ONLY_FULL_GROUP_BY it is an error, not a slow query. The rewrite has no non-aggregated column and
does not depend on that.

Related

  • PrestaShop/dashactivity#62 (mine) touches this file ~70 lines away, no overlap.
  • PrestaShop/dashactivity#63 (nicosomb) is +357/-12 on this file but almost purely additive around
    line 80; it does not touch this query.

The Online Visitors tile only ever used the NUMBER of rows the query
returned: it ran executeS() and then read NumRows(). So every online
visitor was carried into PHP with five columns each and thrown away, and
the database was asked to sort them by date for an order nothing reads.

Both branches now count in SQL. The two joins to page and page_type only
produced the page name the hook discards and both join on a primary key,
so removing them cannot change the count; connections_page is the one
table that can match a connection more than once, which is what the
GROUP BY collapsed and COUNT(DISTINCT) still does. Reading the value with
getValue() also replaces a pair of Db::getInstance() lookups that asked
two different instances for the query and for its row count.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Slow queries from Prestashop Statistics

1 participant