Refactor visits metric internals (#2691)

* Stop relying on implicitly selected `visits` metric

* Make the `visits` metric more explicit in imported data queries

* Rename internally used `visits` to `__internal_visits`

lib/plausible/stats/base.ex

@@ -305,7 +305,7 @@ defmodule Plausible.Stats.Base do
       select_merge: %{
         bounce_rate:
           fragment("toUInt32(ifNotFinite(round(sum(is_bounce * sign) / sum(sign) * 100), 0))"),
-        visits: fragment("toUInt32(sum(sign))")
+        __internal_visits: fragment("toUInt32(sum(sign))")
       }
     )
     |> select_session_metrics(rest)
@@ -353,7 +353,7 @@ defmodule Plausible.Stats.Base do
       select_merge: %{
         :visit_duration =>
           fragment("toUInt32(ifNotFinite(round(sum(duration * sign) / sum(sign)), 0))"),
-        visits: fragment("toUInt32(sum(sign))")
+        __internal_visits: fragment("toUInt32(sum(sign))")
       }
     )
     |> select_session_metrics(rest)

lib/plausible/stats/breakdown.ex

@@ -217,7 +217,7 @@ defmodule Plausible.Stats.Breakdown do
     |> apply_pagination(pagination)
     |> ClickhouseRepo.all()
     |> transform_keys(%{operating_system: :os})
-    |> maybe_remove_visits_metric(metrics)
+    |> remove_internal_visits_metric(metrics)
   end
 
   defp breakdown_events(_, _, _, [], _), do: []
@@ -584,13 +584,13 @@ defmodule Plausible.Stats.Breakdown do
     end)
   end
 
-  defp maybe_remove_visits_metric(results, metrics) do
+  defp remove_internal_visits_metric(results, metrics) do
-    # "visits" is fetched when querying bounce rate and visit duration, as it
+    # "__internal_visits" is fetched when querying bounce rate and visit duration, as it
     # is needed to calculate these from imported data. Let's remove it from the
     # result if it wasn't requested.
-    if (:bounce_rate in metrics or :visit_duration in metrics) and :visits not in metrics do
+    if :bounce_rate in metrics or :visit_duration in metrics do
       results
-      |> Enum.map(&Map.delete(&1, :visits))
+      |> Enum.map(&Map.delete(&1, :__internal_visits))
     else
       results
     end

lib/plausible/stats/imported.ex

@@ -181,13 +181,12 @@ defmodule Plausible.Stats.Imported do
         :entry_page ->
           imported_q
           |> select_merge([i], %{
-            entry_page: i.entry_page,
+            entry_page: i.entry_page
-            visits: sum(i.entrances)
           })
 
         :exit_page ->
           imported_q
-          |> select_merge([i], %{exit_page: i.exit_page, visits: sum(i.exits)})
+          |> select_merge([i], %{exit_page: i.exit_page})
 
         :country ->
           imported_q |> where([i], i.country != "ZZ") |> select_merge([i], %{country: i.country})
@@ -213,7 +212,8 @@ defmodule Plausible.Stats.Imported do
     q =
       from(s in Ecto.Query.subquery(q),
         full_join: i in subquery(imported_q),
-        on: field(s, ^dim) == field(i, ^dim)
+        on: field(s, ^dim) == field(i, ^dim),
+        select: %{}
       )
       |> select_joined_metrics(metrics)
       |> apply_order_by(metrics)
@@ -265,15 +265,13 @@ defmodule Plausible.Stats.Imported do
       :entry_page ->
         q
         |> select_merge([s, i], %{
-          entry_page: fragment("if(empty(?), ?, ?)", i.entry_page, s.entry_page, i.entry_page),
+          entry_page: fragment("if(empty(?), ?, ?)", i.entry_page, s.entry_page, i.entry_page)
-          visits: fragment("? + ?", s.visits, i.visits)
         })
 
       :exit_page ->
         q
         |> select_merge([s, i], %{
-          exit_page: fragment("if(empty(?), ?, ?)", i.exit_page, s.exit_page, i.exit_page),
+          exit_page: fragment("if(empty(?), ?, ?)", i.exit_page, s.exit_page, i.exit_page)
-          visits: fragment("coalesce(?, 0) + coalesce(?, 0)", s.visits, i.visits)
         })
 
       :country ->
@@ -348,17 +346,65 @@ defmodule Plausible.Stats.Imported do
     |> select_imported_metrics(rest)
   end
 
+  defp select_imported_metrics(
+         %Ecto.Query{from: %Ecto.Query.FromExpr{source: {"imported_exit_pages", _}}} = q,
+         [:visits | rest]
+       ) do
+    q
+    |> select_merge([i], %{visits: sum(i.exits)})
+    |> select_imported_metrics(rest)
+  end
+
+  defp select_imported_metrics(
+         %Ecto.Query{from: %Ecto.Query.FromExpr{source: {"imported_entry_pages", _}}} = q,
+         [:visits | rest]
+       ) do
+    q
+    |> select_merge([i], %{visits: sum(i.entrances)})
+    |> select_imported_metrics(rest)
+  end
+
+  defp select_imported_metrics(q, [:visits | rest]) do
+    q
+    |> select_merge([i], %{visits: sum(i.visits)})
+    |> select_imported_metrics(rest)
+  end
+
   defp select_imported_metrics(q, [:pageviews | rest]) do
     q
     |> select_merge([i], %{pageviews: sum(i.pageviews)})
     |> select_imported_metrics(rest)
   end
 
+  defp select_imported_metrics(
+         %Ecto.Query{from: %Ecto.Query.FromExpr{source: {"imported_entry_pages", _}}} = q,
+         [:bounce_rate | rest]
+       ) do
+    q
+    |> select_merge([i], %{
+      bounces: sum(i.bounces),
+      __internal_visits: sum(i.entrances)
+    })
+    |> select_imported_metrics(rest)
+  end
+
   defp select_imported_metrics(q, [:bounce_rate | rest]) do
     q
     |> select_merge([i], %{
       bounces: sum(i.bounces),
-      visits: sum(i.visits)
+      __internal_visits: sum(i.visits)
+    })
+    |> select_imported_metrics(rest)
+  end
+
+  defp select_imported_metrics(
+         %Ecto.Query{from: %Ecto.Query.FromExpr{source: {"imported_entry_pages", _}}} = q,
+         [:visit_duration | rest]
+       ) do
+    q
+    |> select_merge([i], %{
+      visit_duration: sum(i.visit_duration),
+      __internal_visits: sum(i.entrances)
     })
     |> select_imported_metrics(rest)
   end
@@ -367,7 +413,7 @@ defmodule Plausible.Stats.Imported do
     q
     |> select_merge([i], %{
       visit_duration: sum(i.visit_duration),
-      visits: sum(i.visits)
+      __internal_visits: sum(i.visits)
     })
     |> select_imported_metrics(rest)
   end
@@ -384,6 +430,14 @@ defmodule Plausible.Stats.Imported do
   # used as subqueries to a main query that then find the bounce rate/avg
   # visit_duration.
 
+  defp select_joined_metrics(q, [:visits | rest]) do
+    q
+    |> select_merge([s, i], %{
+      :visits => fragment("? + ?", s.visits, i.visits)
+    })
+    |> select_joined_metrics(rest)
+  end
+
   defp select_joined_metrics(q, [:visitors | rest]) do
     q
     |> select_merge([s, i], %{
@@ -408,9 +462,9 @@ defmodule Plausible.Stats.Imported do
           "round(100 * (coalesce(?, 0) + coalesce((? * ? / 100), 0)) / (coalesce(?, 0) + coalesce(?, 0)))",
           i.bounces,
           s.bounce_rate,
-          s.visits,
+          s.__internal_visits,
-          i.visits,
+          i.__internal_visits,
-          s.visits
+          s.__internal_visits
         )
     })
     |> select_joined_metrics(rest)
@@ -424,9 +478,9 @@ defmodule Plausible.Stats.Imported do
           "round((? + ? * ?) / (? + ?), 1)",
           i.visit_duration,
           s.visit_duration,
-          s.visits,
+          s.__internal_visits,
-          s.visits,
+          s.__internal_visits,
-          i.visits
+          i.__internal_visits
         )
     })
     |> select_joined_metrics(rest)