Fix returning more pageviews with a visit property filter (#2612)

* fix subquery for sessions in base_event_query/2

As the 'sessions' table is using the CollapsingMergeTree engine, we have
to select session_id's distinctively. Otherwise we will get multiple rows
(with sign -1 and 1) as long as the background merge hasn't happened.

* update changelog

* use GROUP BY instead of SELECT DISTINCT

* remove comma
This commit is contained in:
RobertJoonas 2023-01-23 12:14:27 +02:00 committed by GitHub
parent 8640ba499f
commit bd0de97521
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 30 additions and 1 deletions

View File

@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file.
- 'Last updated X seconds ago' info to 'current visitors' tooltips - 'Last updated X seconds ago' info to 'current visitors' tooltips
### Fixed ### Fixed
- Fix [more pageviews with session prop filter than with no filters](https://github.com/plausible/analytics/issues/1666)
- Cascade delete sent_renewal_notifications table when user is deleted plausible/analytics#2549 - Cascade delete sent_renewal_notifications table when user is deleted plausible/analytics#2549
- Show appropriate top-stat metric labels on the realtime dashboard when filtering by a goal - Show appropriate top-stat metric labels on the realtime dashboard when filtering by a goal
- Fix breakdown API pagination when using event metrics plausible/analytics#2562 - Fix breakdown API pagination when using event metrics plausible/analytics#2562

View File

@ -14,7 +14,9 @@ defmodule Plausible.Stats.Base do
sessions_q = sessions_q =
from( from(
s in query_sessions(site, query), s in query_sessions(site, query),
select: %{session_id: s.session_id} select: %{session_id: s.session_id},
where: s.sign == 1,
group_by: s.session_id
) )
from( from(

View File

@ -798,5 +798,31 @@ defmodule PlausibleWeb.Api.ExternalStatsController.AggregateTest do
assert json_response(conn, 200)["results"] == %{"visitors" => %{"value" => 3}} assert json_response(conn, 200)["results"] == %{"visitors" => %{"value" => 3}}
end end
test "joins correctly with the sessions (CollapsingMergeTree) table", %{
conn: conn,
site: site
} do
create_sessions([
%{domain: site.domain, session_id: 1000, country_code: "EE", sign: 1, events: 1},
%{domain: site.domain, session_id: 1000, country_code: "EE", sign: -1, events: 1},
%{domain: site.domain, session_id: 1000, country_code: "EE", sign: 1, events: 2}
])
create_events([
%{domain: site.domain, session_id: 1000, country_code: "EE", name: "pageview"},
%{domain: site.domain, session_id: 1000, country_code: "EE", name: "pageview"},
%{domain: site.domain, session_id: 1000, country_code: "EE", name: "pageview"}
])
conn =
get(conn, "/api/v1/stats/aggregate", %{
"site_id" => site.domain,
"metrics" => "pageviews",
"filters" => "visit:country==EE"
})
assert json_response(conn, 200)["results"] == %{"pageviews" => %{"value" => 3}}
end
end end
end end