Fix segment table logic might falsely remove a where statement #17525
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
I noticed a problem in the log aggregator that affects especially non-core plugins that aren't tracking. For example A/B testing were
where
clauses ignored when a segment is used.For example when calling
Then it would generate below query:
as you notice the where clause is completely gone! And it wasn't applied in the table
matomo_logtmpsegmentad7af0eba5c424ef32a77fe905923136
because we create the tmp table only once during the entire archiving and there is only this where applied: https://github.com/matomo-org/matomo/blob/4.3.0-b3/core/DataAccess/LogAggregator.php#L360 which isThe where part of the actual query gets lost in https://github.com/matomo-org/matomo/blob/4.3.0-b3/core/DataAccess/LogAggregator.php#L382-L391 because it removes the where part thinking it is already applied in the logtmpsegment table but this is not the case.
With this PR it actually results in the correct query:
I'm not 100% sure why no tests are failing now in core itself. Either they always included log_visit table and where condition or the where condition started differently. It is definitely an issue for A/B testing and from what I can see also other premium features.
Review