APIv2: Remove cruft (v2 flag, experimental_reduced_joins) (#4780)

* Remove query.v2 flag

This was originally used for making sure queries use the right table as
migrating to APIv2. This is no longer needed

* Remove experimental_reduced_joins flag
This commit is contained in:
Karl-Aksel Puulmann 2024-11-06 14:08:20 +02:00 committed by GitHub
parent dbf7a099a3
commit eed21a0138
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 61 additions and 123 deletions

View File

@ -16,25 +16,25 @@ env:
jobs:
build:
name: "Build and test (${{ matrix.mix_env }}, ${{ matrix.postgres_image }}${{ matrix.test_read_team_schemas_and_experimental_reduced_joins == '1' && ', read_team_schemas_and_experimental_reduced_joins' || '' }})"
name: "Build and test (${{ matrix.mix_env }}, ${{ matrix.postgres_image }}${{ matrix.test_read_team_schemas == '1' && ', read_team_schemas' || '' }})"
runs-on: ubuntu-latest
strategy:
matrix:
mix_env: ["test", "ce_test"]
postgres_image: ["postgres:16"]
test_read_team_schemas_and_experimental_reduced_joins: ["0"]
test_read_team_schemas: ["0"]
include:
- mix_env: "test"
postgres_image: "postgres:15"
test_read_team_schemas_and_experimental_reduced_joins: "0"
test_read_team_schemas: "0"
- mix_env: "test"
postgres_image: "postgres:16"
test_read_team_schemas_and_experimental_reduced_joins: "1"
test_read_team_schemas: "1"
env:
MIX_ENV: ${{ matrix.mix_env }}
TEST_READ_TEAM_SCHEMAS_AND_EXPERIMENTAL_REDUCED_JOINS: ${{ matrix.test_read_team_schemas_and_experimental_reduced_joins }}
TEST_READ_TEAM_SCHEMAS: ${{ matrix.test_read_team_schemas }}
services:
postgres:
image: ${{ matrix.postgres_image }}

View File

@ -31,7 +31,6 @@ defmodule Plausible.Stats.Breakdown do
dimensions: transform_dimensions(dimension),
filters: query.filters ++ dimension_filters(dimension),
pagination: %{limit: limit, offset: (page - 1) * limit},
v2: true,
# Allow pageview and event metrics to be queried off of sessions table
legacy_breakdown: true,
remove_unavailable_revenue_metrics: true

View File

@ -22,7 +22,6 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do
|> put_preloaded_goals(site)
|> put_order_by(params)
|> put_include_comparisons(site, params)
|> Query.put_experimental_reduced_joins(site, params)
|> Query.put_imported_opts(site, params)
on_ee do

View File

@ -10,12 +10,10 @@ defmodule Plausible.Stats.Query do
include_imported: false,
skip_imported_reason: nil,
now: nil,
experimental_reduced_joins?: false,
latest_import_end_date: nil,
metrics: [],
order_by: nil,
timezone: nil,
v2: false,
legacy_breakdown: false,
remove_unavailable_revenue_metrics: false,
preloaded_goals: [],
@ -34,8 +32,7 @@ defmodule Plausible.Stats.Query do
query =
struct!(__MODULE__, Map.to_list(query_data))
|> put_imported_opts(site, %{})
|> put_experimental_reduced_joins(site, params)
|> struct!(v2: true, now: DateTime.utc_now(:second), debug_metadata: debug_metadata)
|> struct!(now: DateTime.utc_now(:second), debug_metadata: debug_metadata)
on_ee do
query = Plausible.Stats.Sampling.put_threshold(query, site, params)
@ -52,18 +49,6 @@ defmodule Plausible.Stats.Query do
Legacy.QueryBuilder.from(site, params, debug_metadata, now)
end
def put_experimental_reduced_joins(query, site, params) do
if Map.has_key?(params, "experimental_reduced_joins") do
struct!(query,
experimental_reduced_joins?: Map.get(params, "experimental_reduced_joins") == "true"
)
else
struct!(query,
experimental_reduced_joins?: FunWithFlags.enabled?(:experimental_reduced_joins, for: site)
)
end
end
def date_range(query, options \\ []) do
date_range = Plausible.Stats.DateTimeRange.to_date_range(query.utc_time_range, query.timezone)

View File

@ -7,8 +7,6 @@ defmodule Plausible.Stats.SQL.WhereBuilder do
import Plausible.Stats.Time, only: [utc_boundaries: 2]
import Plausible.Stats.Filters.Utils, only: [page_regex: 1]
alias Plausible.Stats.Query
use Plausible.Stats.SQL.Fragments
require Logger
@ -121,10 +119,10 @@ defmodule Plausible.Stats.SQL.WhereBuilder do
defp add_filter(
:events,
%Query{experimental_reduced_joins?: true},
_query,
[_, "visit:" <> key | _rest] = filter
) do
# Filter events query if experimental_reduced_joins? is true
# Filter events query with visit dimension if possible
field_name = String.to_existing_atom(key)
if Enum.member?(@sessions_only_visit_fields, field_name) do
@ -134,10 +132,6 @@ defmodule Plausible.Stats.SQL.WhereBuilder do
end
end
defp add_filter(:events, _query, [_, "visit:" <> _key | _rest]) do
true
end
defp add_filter(:sessions, _query, [_, "visit:entry_props:" <> prop_name | _rest] = filter) do
filter_custom_prop(prop_name, :entry_meta, filter)
end

View File

@ -12,13 +12,13 @@ defmodule Plausible.Stats.TableDecider do
def events_join_sessions?(query) do
query.filters
|> dimensions_used_in_filters()
|> Enum.any?(&(filters_partitioner(query, &1) == :session))
|> Enum.any?(&(dimension_partitioner(query, &1) == :session))
end
def sessions_join_events?(query) do
query.filters
|> dimensions_used_in_filters()
|> Enum.any?(&(filters_partitioner(query, &1) == :event))
|> Enum.any?(&(dimension_partitioner(query, &1) == :event))
end
def partition_metrics(metrics, query) do
@ -34,10 +34,10 @@ defmodule Plausible.Stats.TableDecider do
%{event: event_only_filters, session: session_only_filters} =
query.filters
|> dimensions_used_in_filters()
|> partition(query, &filters_partitioner/2)
|> partition(query, &dimension_partitioner/2)
%{event: event_only_dimensions, session: session_only_dimensions} =
partition(query.dimensions, query, &filters_partitioner/2)
partition(query.dimensions, query, &dimension_partitioner/2)
cond do
# Only one table needs to be queried
@ -62,16 +62,16 @@ defmodule Plausible.Stats.TableDecider do
end
end
defp metric_partitioner(%Query{v2: true}, :conversion_rate), do: :either
defp metric_partitioner(%Query{v2: true}, :group_conversion_rate), do: :either
defp metric_partitioner(%Query{v2: true}, :visitors), do: :either
defp metric_partitioner(%Query{v2: true}, :visits), do: :either
# Note: This is inaccurate when filtering but required for old backwards compatibility
defp metric_partitioner(%Query{legacy_breakdown: true}, :pageviews), do: :either
defp metric_partitioner(%Query{legacy_breakdown: true}, :events), do: :either
defp metric_partitioner(_, :conversion_rate), do: :event
defp metric_partitioner(_, :group_conversion_rate), do: :event
defp metric_partitioner(_, :conversion_rate), do: :either
defp metric_partitioner(_, :group_conversion_rate), do: :either
defp metric_partitioner(_, :visitors), do: :either
defp metric_partitioner(_, :visits), do: :either
defp metric_partitioner(_, :percentage), do: :either
defp metric_partitioner(_, :average_revenue), do: :event
defp metric_partitioner(_, :total_revenue), do: :event
defp metric_partitioner(_, :pageviews), do: :event
@ -80,42 +80,21 @@ defmodule Plausible.Stats.TableDecider do
defp metric_partitioner(_, :visit_duration), do: :session
defp metric_partitioner(_, :views_per_visit), do: :session
# Metrics which used to only be queried from one table but can be calculated from either
defp metric_partitioner(%Query{experimental_reduced_joins?: true}, :visits), do: :either
defp metric_partitioner(%Query{experimental_reduced_joins?: true}, :visitors), do: :either
defp metric_partitioner(_, :visits), do: :session
defp metric_partitioner(_, :visitors), do: :event
# Calculated metrics - handled on callsite separately from other metrics.
defp metric_partitioner(_, :time_on_page), do: :other
defp metric_partitioner(_, :total_visitors), do: :other
defp metric_partitioner(_, :percentage), do: :either
# Sample percentage is included in both tables if queried.
defp metric_partitioner(_, :sample_percent), do: :sample_percent
defp metric_partitioner(%Query{experimental_reduced_joins?: false}, unknown) do
raise ArgumentError, "Metric #{unknown} not supported without experimental_reduced_joins?"
end
defp dimension_partitioner(_, "event:" <> _), do: :event
defp dimension_partitioner(_, "visit:entry_page"), do: :session
defp dimension_partitioner(_, "visit:entry_page_hostname"), do: :session
defp dimension_partitioner(_, "visit:exit_page"), do: :session
defp dimension_partitioner(_, "visit:exit_page_hostname"), do: :session
defp metric_partitioner(_, _), do: :either
defp dimension_partitioner(_, "visit:" <> _), do: :either
defp filters_partitioner(_, "event:" <> _), do: :event
defp filters_partitioner(_, "visit:entry_page"), do: :session
defp filters_partitioner(_, "visit:entry_page_hostname"), do: :session
defp filters_partitioner(_, "visit:exit_page"), do: :session
defp filters_partitioner(_, "visit:exit_page_hostname"), do: :session
defp filters_partitioner(%Query{experimental_reduced_joins?: true}, "visit:" <> _),
do: :either
defp filters_partitioner(_, "visit:" <> _),
do: :session
defp filters_partitioner(%Query{experimental_reduced_joins?: false}, {unknown, _}) do
raise ArgumentError, "Filter #{unknown} not supported without experimental_reduced_joins?"
end
defp filters_partitioner(_, _), do: :either
defp dimension_partitioner(_, _), do: :either
@default %{event: [], session: [], either: [], other: [], sample_percent: []}
defp partition(values, query, partitioner) do

View File

@ -23,7 +23,6 @@ defmodule Plausible.Stats.Timeseries do
metrics: transform_metrics(metrics, %{conversion_rate: :group_conversion_rate}),
dimensions: [time_dimension(query)],
order_by: [{time_dimension(query), :asc}],
v2: true,
remove_unavailable_revenue_metrics: true
)
|> QueryOptimizer.optimize()

View File

@ -5,69 +5,66 @@ defmodule Plausible.Stats.TableDeciderTest do
import Plausible.Stats.TableDecider
describe "events_join_sessions?" do
test "with experimental_reduced_joins disabled" do
assert not events_join_sessions?(make_query(false, []))
assert not events_join_sessions?(make_query(false, ["event:name"]))
assert events_join_sessions?(make_query(false, ["visit:source"]))
assert events_join_sessions?(make_query(false, ["visit:entry_page"]))
assert events_join_sessions?(make_query(false, ["visit:exit_page"]))
end
test "with experimental_reduced_joins enabled" do
assert not events_join_sessions?(make_query(true, []))
assert not events_join_sessions?(make_query(true, ["event:name"]))
assert not events_join_sessions?(make_query(true, ["visit:source"]))
assert events_join_sessions?(make_query(true, ["visit:entry_page"]))
assert events_join_sessions?(make_query(true, ["visit:exit_page"]))
test "with simple filters" do
assert not events_join_sessions?(make_query([]))
assert not events_join_sessions?(make_query(["event:name"]))
assert not events_join_sessions?(make_query(["visit:source"]))
assert events_join_sessions?(make_query(["visit:entry_page"]))
assert events_join_sessions?(make_query(["visit:exit_page"]))
end
test "with nested filter" do
assert not events_join_sessions?(make_query([["not", ["is", "event:name", []]]]))
assert events_join_sessions?(make_query([["not", ["is", "visit:exit_page", []]]]))
assert not events_join_sessions?(
make_query_full_filters([["not", ["is", "event:name", []]]])
)
assert events_join_sessions?(
make_query_full_filters([["not", ["is", "visit:exit_page", []]]])
)
end
end
describe "partition_metrics" do
test "with no metrics or filters" do
query = make_query(false, [])
query = make_query([])
assert partition_metrics([], query) == {[], [], []}
end
test "session-only metrics accordingly" do
query = make_query(false, [])
query = make_query([])
assert partition_metrics([:bounce_rate, :views_per_visit], query) ==
{[], [:bounce_rate, :views_per_visit], []}
end
test "event-only metrics accordingly" do
query = make_query(false, [])
query = make_query([])
assert partition_metrics([:total_revenue, :visitors], query) ==
{[:total_revenue, :visitors], [], []}
end
test "filters from both, event-only metrics" do
query = make_query(false, ["event:name", "visit:source"])
query = make_query(["event:name", "visit:source"])
assert partition_metrics([:total_revenue], query) == {[:total_revenue], [], []}
end
test "filters from both, session-only metrics" do
query = make_query(false, ["event:name", "visit:source"])
query = make_query(["event:name", "visit:source"])
assert partition_metrics([:bounce_rate], query) == {[], [:bounce_rate], []}
end
test "session filters but no session metrics" do
query = make_query(false, ["visit:source"])
query = make_query(["visit:source"])
assert partition_metrics([:total_revenue], query) == {[:total_revenue], [], []}
end
test "sample_percent is added to both types of metrics" do
query = make_query(false, [])
query = make_query([])
assert partition_metrics([:total_revenue, :sample_percent], query) ==
{[:total_revenue, :sample_percent], [], []}
@ -80,24 +77,14 @@ defmodule Plausible.Stats.TableDeciderTest do
end
test "other metrics put in its own result" do
query = make_query(false, [])
query = make_query([])
assert partition_metrics([:time_on_page, :percentage, :total_visitors], query) ==
{[], [:percentage], [:time_on_page, :total_visitors]}
end
test "raises if unknown metric" do
query = make_query(false, [])
assert_raise ArgumentError, fn ->
partition_metrics([:foobar], query)
end
end
end
describe "partition_metrics with experimental_reduced_joins enabled" do
test "metrics that can be calculated on either when event-only metrics" do
query = make_query(true, [])
query = make_query([])
assert partition_metrics([:total_revenue, :visitors], query) ==
{[:total_revenue, :visitors], [], []}
@ -106,7 +93,7 @@ defmodule Plausible.Stats.TableDeciderTest do
end
test "metrics that can be calculated on either when session-only metrics" do
query = make_query(true, [])
query = make_query([])
assert partition_metrics([:bounce_rate, :visitors], query) ==
{[], [:bounce_rate, :visitors], []}
@ -116,64 +103,63 @@ defmodule Plausible.Stats.TableDeciderTest do
end
test "metrics that can be calculated on either are biased to events" do
query = make_query(true, [])
query = make_query([])
assert partition_metrics([:bounce_rate, :total_revenue, :visitors], query) ==
{[:total_revenue, :visitors], [:bounce_rate], []}
end
test "sample_percent is handled with either metrics" do
query = make_query(true, [])
query = make_query([])
assert partition_metrics([:visitors, :sample_percent], query) ==
{[], [:visitors, :sample_percent], []}
end
test "metric can be calculated on either, but filtering on events" do
query = make_query(true, ["event:name"])
query = make_query(["event:name"])
assert partition_metrics([:visitors], query) == {[:visitors], [], []}
end
test "metric can be calculated on either, but filtering on events and sessions" do
query = make_query(true, ["event:name", "visit:exit_page"])
query = make_query(["event:name", "visit:exit_page"])
assert partition_metrics([:visitors], query) == {[], [:visitors], []}
end
test "metric can be calculated on either, filtering on either" do
query = make_query(true, ["visit:source"])
query = make_query(["visit:source"])
assert partition_metrics([:visitors], query) == {[], [:visitors], []}
end
test "metric can be calculated on either, filtering on sessions" do
query = make_query(true, ["visit:exit_page"])
query = make_query(["visit:exit_page"])
assert partition_metrics([:visitors], query) == {[], [:visitors], []}
end
test "query dimensions lean metric" do
assert partition_metrics([:visitors], make_query(true, [], ["event:name"])) ==
assert partition_metrics([:visitors], make_query([], ["event:name"])) ==
{[:visitors], [], []}
assert partition_metrics([:visitors], make_query(true, [], ["visit:source"])) ==
assert partition_metrics([:visitors], make_query([], ["visit:source"])) ==
{[], [:visitors], []}
assert partition_metrics([:visitors], make_query(true, [], ["visit:exit_page"])) ==
assert partition_metrics([:visitors], make_query([], ["visit:exit_page"])) ==
{[], [:visitors], []}
end
end
defp make_query(experimental_reduced_joins?, filter_keys, dimensions \\ []) do
defp make_query(filter_keys, dimensions \\ []) do
Query.from(build(:site), %{
"experimental_reduced_joins" => to_string(experimental_reduced_joins?),
"filters" => Enum.map(filter_keys, fn filter_key -> ["is", filter_key, []] end),
"dimensions" => dimensions
})
end
defp make_query(filters) do
defp make_query_full_filters(filters) do
Query.from(build(:site), %{
"dimensions" => [],
"filters" => filters

View File

@ -6,14 +6,11 @@ end
Mox.defmock(Plausible.HTTPClient.Mock, for: Plausible.HTTPClient.Interface)
Application.ensure_all_started(:double)
# Temporary flag to test `read_team_schemas` and `experimental_reduced_joins`
# flags on all tests.
if System.get_env("TEST_READ_TEAM_SCHEMAS_AND_EXPERIMENTAL_REDUCED_JOINS") == "1" do
# Temporary flag to test `read_team_schemas` flag on all tests.
if System.get_env("TEST_READ_TEAM_SCHEMAS") == "1" do
FunWithFlags.enable(:read_team_schemas)
FunWithFlags.enable(:experimental_reduced_joins)
else
FunWithFlags.disable(:read_team_schemas)
FunWithFlags.disable(:experimental_reduced_joins)
end
Ecto.Adapters.SQL.Sandbox.mode(Plausible.Repo, :manual)