FULL join for time:hour as well as time:minute (#5715)

* FULL join for time:hour as well as time:minute

Follow-up to https://github.com/plausible/analytics/pull/5694/files#r2321567271

A session might be active over multiple hours, but not (currently)
reported as such when requesting only specific metrics per hour.
This fixes that problem.

* Handle full join logic correctly

---------

Co-authored-by: Uku Taht <[email protected]>

Author: macobo

CHANGELOG.md

@@ -30,6 +30,7 @@ All notable changes to this project will be documented in this file.
 - Realtime and hourly graphs of visit duration, views per visit no longer overcount due to long-lasting sessions, instead showing each visit
   when they occurred.
 - Fixed realtime and hourly graphs of visits overcounting
+- When reporting only `visitors` and `visits` per hour, count visits in each hour they were active in.
 
 ## v3.0.0 - 2025-04-11
 

lib/plausible/stats/query_optimizer.ex

@@ -318,12 +318,12 @@ defmodule Plausible.Stats.QueryOptimizer do
   # Normally we can always LEFT JOIN as this is more performant and tables
   # are expected to contain the same dimensions.
 
-  # The only exception is using the "time:minute" dimension where the sessions
+  # The only exception is using the "time:minute"/"time:hour" dimension where the sessions
   # subquery might return more rows than the events one. That's because we're
   # counting sessions in all time buckets they were active in even if no event
-  # occurred during that particular minute.
+  # occurred during that particular bucket.
   defp set_sql_join_type(query) do
-    if "time:minute" in query.dimensions do
+    if "time:minute" in query.dimensions or "time:hour" in query.dimensions do
       Query.set(query, sql_join_type: :full)
     else
       query

lib/plausible/stats/sql/query_builder.ex

@@ -185,7 +185,7 @@ defmodule Plausible.Stats.SQL.QueryBuilder do
         )
         |> select_join_metrics(query, query.metrics -- [:sample_percent])
     end)
-    |> select_dimensions(main_query)
+    |> select_dimensions(main_query, queries)
   end
 
   # NOTE: Old queries do their own pagination
@@ -223,19 +223,31 @@ defmodule Plausible.Stats.SQL.QueryBuilder do
     end)
   end
 
-  defp select_dimensions(q, query) do
+  defp select_dimensions(q, query, queries) do
     Enum.reduce(query.dimensions, q, fn dimension, q ->
-      # We generally select dimensions from the left-most table. Only exception is time:minute where
-      # we use sessions table as sessions are considered on-going during the whole period.
-      if query.sql_join_type == :full and "time:minute" == dimension do
-        select_merge_as(q, [..., x], %{
-          shortname(query, dimension) => field(x, ^shortname(query, dimension))
-        })
-      else
-        select_merge_as(q, [x], %{
-          shortname(query, dimension) => field(x, ^shortname(query, dimension))
-        })
+      case select_from(dimension, query, queries) do
+        :leftmost_table ->
+          select_merge_as(q, [x], %{
+            shortname(query, dimension) => field(x, ^shortname(query, dimension))
+          })
+
+        :rightmost_table ->
+          select_merge_as(q, [..., x], %{
+            shortname(query, dimension) => field(x, ^shortname(query, dimension))
+          })
       end
     end)
   end
+
+  defp select_from(dimension, query, queries) do
+    smeared? = Enum.any?(queries, fn {_table_type, query, _q} -> query.smear_session_metrics end)
+
+    cond do
+      query.sql_join_type == :left -> :leftmost_table
+      # We generally select dimensions from the left-most table. Only exception is time:minute/time:hour where
+      # we use sessions table as smeared sessions are considered on-going during the whole period.
+      dimension in ["time:minute", "time:hour"] and smeared? -> :rightmost_table
+      true -> :leftmost_table
+    end
+  end
 end

test/plausible/stats/query_optimizer_test.exs

@@ -365,12 +365,14 @@ defmodule Plausible.Stats.QueryOptimizerTest do
   end
 
   describe "set_sql_join_type" do
-    test "updates sql_join_type to :full if time:minute dimension is present" do
+    test "updates sql_join_type to :full if time:minute or time:hour dimension is present" do
       assert perform(%{dimensions: ["time:minute"]}).sql_join_type == :full
+      assert perform(%{dimensions: ["time:hour"]}).sql_join_type == :full
     end
 
     test "keeps default sql_join_type otherwise" do
-      assert perform(%{dimensions: ["time:hour"]}).sql_join_type == :left
+      assert perform(%{dimensions: ["time:day"]}).sql_join_type == :left
+      assert perform(%{dimensions: []}).sql_join_type == :left
     end
   end
 end

test/plausible_web/controllers/api/external_stats_controller/query_test.exs

@@ -1372,6 +1372,29 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do
              ]
     end
 
+    test "breakdown by time:hour (internal API), counts visitors and visits in all buckets their session was active in",
+         %{conn: conn, site: site} do
+      populate_stats(site, [
+        build(:pageview, user_id: @user_id, timestamp: ~N[2021-01-01 00:20:00]),
+        build(:pageview, user_id: @user_id, timestamp: ~N[2021-01-01 00:40:00]),
+        build(:pageview, user_id: @user_id, timestamp: ~N[2021-01-01 01:00:00]),
+        build(:pageview, user_id: @user_id, timestamp: ~N[2021-01-01 01:20:00])
+      ])
+
+      conn =
+        post(conn, "/api/v2/query-internal-test", %{
+          "site_id" => site.domain,
+          "metrics" => ["visitors", "visits", "visit_duration"],
+          "date_range" => ["2021-01-01", "2021-01-02"],
+          "dimensions" => ["time:hour"]
+        })
+
+      assert json_response(conn, 200)["results"] == [
+               %{"dimensions" => ["2021-01-01 00:00:00"], "metrics" => [1, 1, 0]},
+               %{"dimensions" => ["2021-01-01 01:00:00"], "metrics" => [1, 1, 3600]}
+             ]
+    end
+
     test "shows hourly data for a certain date with time_labels", %{conn: conn, site: site} do
       populate_stats(site, [
         build(:pageview, user_id: @user_id, timestamp: ~N[2021-01-01 00:00:00]),