@tsteur opened this Pull Request on October 7th 2019 Member

fix https://github.com/matomo-org/matomo/issues/13861

also refs https://github.com/matomo-org/matomo/issues/9200 and now we no longer need workaround in https://github.com/matomo-org/matomo/pull/10024

Tried various different solutions until I realised we can simplify this actually all quite a lot by no longer using any subselects at all and likely also making regular visitor logs without segment a bit faster with some logic I added recently.

Now we basically have no more subqueries in the visitor log (similar to archiving yay). Instead we apply the group by, order, and limit directly. More on this in the comments...

You can see in the tests how this simplifies queries. When segments were applied we had before 3 subqueries (or one query with 2 nested inner queries), and when no segment was applied we had 2 subqueries. Now there's always only one query. It also removes a group by when no segment is applied.

@tsteur commented on October 7th 2019 Member

@sgiehl or @mattab if someone could have a review that be great? @mattab be good you can confirm it should be fine from a performance perspective. From what I see it should be fine... or there is at least no other way to have segmenting fully work with the visitor log. In regards to your comment in https://github.com/matomo-org/matomo/issues/13861#issuecomment-457159967 we basically only added this to the visitor log and only when segmenting. Considering we remove a few subselects it might not even be any slower or even faster?

@mattab commented on October 14th 2019 Member

From performance perspective it should be fine :+1:
Haven't reviewed the exact code in detail, but overall the SQL change looks great and I can't think of a way this could create a problem.

This Pull Request was closed on October 14th 2019
Powered by GitHub Issue Mirror