fix(performance): Simply project threshold config query (#31685)

The project threshold config column dumps every config it finds into the query.
This can be redundant if it's equal to the default values. This change targets
this specific case to make the final query shorter. Other optimizations are
possible, this was just easy to do right now.

src/sentry/search/events/datasets/discover.py

@@ -764,21 +764,49 @@ class DiscoverDatasetConfig(DatasetConfig):
 
         # Arrays need to have toUint64 casting because clickhouse will define the type as the narrowest possible type
         # that can store listed argument types, which means the comparison will fail because of mismatched types
+        project_thresholds = {}
         project_threshold_config_keys = []
         project_threshold_config_values = []
         for project_id, threshold, metric in project_threshold_configs:
+            metric = TRANSACTION_METRICS[metric]
+            if (
+                threshold == DEFAULT_PROJECT_THRESHOLD
+                and metric == DEFAULT_PROJECT_THRESHOLD_METRIC
+            ):
+                # small optimization, if the configuration is equal to the default,
+                # we can skip it in the final query
+                continue
+
+            project_thresholds[project_id] = (metric, threshold)
             project_threshold_config_keys.append(Function("toUInt64", [project_id]))
-            project_threshold_config_values.append((TRANSACTION_METRICS[metric], threshold))
+            project_threshold_config_values.append((metric, threshold))
 
         project_threshold_override_config_keys = []
         project_threshold_override_config_values = []
         for transaction, project_id, threshold, metric in transaction_threshold_configs:
+            metric = TRANSACTION_METRICS[metric]
+            if (
+                project_id in project_thresholds
+                and threshold == project_thresholds[project_id][1]
+                and metric == project_thresholds[project_id][0]
+            ):
+                # small optimization, if the configuration is equal to the project
+                # configs, we can skip it in the final query
+                continue
+
+            elif (
+                project_id not in project_thresholds
+                and threshold == DEFAULT_PROJECT_THRESHOLD
+                and metric == DEFAULT_PROJECT_THRESHOLD_METRIC
+            ):
+                # small optimization, if the configuration is equal to the default
+                # and no project configs were set, we can skip it in the final query
+                continue
+
             project_threshold_override_config_keys.append(
                 (Function("toUInt64", [project_id]), transaction)
             )
-            project_threshold_override_config_values.append(
-                (TRANSACTION_METRICS[metric], threshold)
-            )
+            project_threshold_override_config_values.append((metric, threshold))
 
         project_threshold_config_index: SelectType = Function(
             "indexOf",
@@ -799,8 +827,8 @@ class DiscoverDatasetConfig(DatasetConfig):
         )
 
         def _project_threshold_config(alias: Optional[str] = None) -> SelectType:
-            return (
-                Function(
+            if project_threshold_config_keys and project_threshold_config_values:
+                return Function(
                     "if",
                     [
                         Function(
@@ -821,15 +849,14 @@ class DiscoverDatasetConfig(DatasetConfig):
                     ],
                     alias,
                 )
-                if project_threshold_configs
-                else Function(
-                    "tuple",
-                    [DEFAULT_PROJECT_THRESHOLD_METRIC, DEFAULT_PROJECT_THRESHOLD],
-                    alias,
-                )
+
+            return Function(
+                "tuple",
+                [DEFAULT_PROJECT_THRESHOLD_METRIC, DEFAULT_PROJECT_THRESHOLD],
+                alias,
             )
 
-        if transaction_threshold_configs:
+        if project_threshold_override_config_keys and project_threshold_override_config_values:
             return Function(
                 "if",
                 [

tests/snuba/api/endpoints/test_organization_events_v2.py

@@ -5,6 +5,8 @@ import pytest
 from django.urls import reverse
 from django.utils import timezone
 from pytz import utc
+from snuba_sdk.column import Column
+from snuba_sdk.function import Function
 
 from sentry.discover.models import TeamKeyTransaction
 from sentry.models import ApiKey, ProjectTeam, ProjectTransactionThreshold, ReleaseStages
@@ -13,6 +15,7 @@ from sentry.models.transaction_threshold import (
     TransactionMetric,
 )
 from sentry.search.events.constants import (
+    DEFAULT_PROJECT_THRESHOLD,
     RELEASE_STAGE_ALIAS,
     SEMVER_ALIAS,
     SEMVER_BUILD_ALIAS,
@@ -4741,3 +4744,149 @@ class OrganizationEventsV2EndpointTestWithSnql(OrganizationEventsV2EndpointTest)
             response = self.do_request(query)
 
             assert response.status_code == 400, query_text
+
+    @mock.patch("sentry.search.events.builder.raw_snql_query")
+    def test_removes_unnecessary_default_project_and_transaction_thresholds(self, mock_snql_query):
+        mock_snql_query.side_effect = [{"meta": {}, "data": []}]
+
+        ProjectTransactionThreshold.objects.create(
+            project=self.project,
+            organization=self.organization,
+            # these are the default values that we use
+            threshold=DEFAULT_PROJECT_THRESHOLD,
+            metric=TransactionMetric.DURATION.value,
+        )
+        ProjectTransactionThresholdOverride.objects.create(
+            transaction="transaction",
+            project=self.project,
+            organization=self.organization,
+            # these are the default values that we use
+            threshold=DEFAULT_PROJECT_THRESHOLD,
+            metric=TransactionMetric.DURATION.value,
+        )
+
+        query = {
+            "field": ["apdex()", "user_misery()"],
+            "query": "event.type:transaction",
+            "project": [self.project.id],
+        }
+
+        response = self.do_request(
+            query,
+            features={
+                "organizations:discover-basic": True,
+                "organizations:global-views": True,
+            },
+        )
+
+        assert response.status_code == 200, response.content
+
+        assert mock_snql_query.call_count == 1
+
+        assert (
+            Function("tuple", ["duration", 300], "project_threshold_config")
+            in mock_snql_query.call_args_list[0][0][0].select
+        )
+
+    @mock.patch("sentry.search.events.builder.raw_snql_query")
+    def test_removes_unnecessary_default_project_and_transaction_thresholds_keeps_others(
+        self, mock_snql_query
+    ):
+        mock_snql_query.side_effect = [{"meta": {}, "data": []}]
+
+        ProjectTransactionThreshold.objects.create(
+            project=self.project,
+            organization=self.organization,
+            # these are the default values that we use
+            threshold=DEFAULT_PROJECT_THRESHOLD,
+            metric=TransactionMetric.DURATION.value,
+        )
+        ProjectTransactionThresholdOverride.objects.create(
+            transaction="transaction",
+            project=self.project,
+            organization=self.organization,
+            # these are the default values that we use
+            threshold=DEFAULT_PROJECT_THRESHOLD,
+            metric=TransactionMetric.DURATION.value,
+        )
+
+        project = self.create_project()
+
+        ProjectTransactionThreshold.objects.create(
+            project=project,
+            organization=self.organization,
+            threshold=100,
+            metric=TransactionMetric.LCP.value,
+        )
+        ProjectTransactionThresholdOverride.objects.create(
+            transaction="transaction",
+            project=project,
+            organization=self.organization,
+            threshold=200,
+            metric=TransactionMetric.LCP.value,
+        )
+
+        query = {
+            "field": ["apdex()", "user_misery()"],
+            "query": "event.type:transaction",
+            "project": [self.project.id, project.id],
+        }
+
+        response = self.do_request(
+            query,
+            features={
+                "organizations:discover-basic": True,
+                "organizations:global-views": True,
+            },
+        )
+
+        assert response.status_code == 200, response.content
+
+        assert mock_snql_query.call_count == 1
+
+        project_threshold_override_config_index = Function(
+            "indexOf",
+            [
+                # only 1 transaction override is present here
+                # because the other use to the default values
+                [(Function("toUInt64", [project.id]), "transaction")],
+                (Column("project_id"), Column("transaction")),
+            ],
+            "project_threshold_override_config_index",
+        )
+
+        project_threshold_config_index = Function(
+            "indexOf",
+            [
+                # only 1 project override is present here
+                # because the other use to the default values
+                [Function("toUInt64", [project.id])],
+                Column("project_id"),
+            ],
+            "project_threshold_config_index",
+        )
+
+        assert (
+            Function(
+                "if",
+                [
+                    Function("equals", [project_threshold_override_config_index, 0]),
+                    Function(
+                        "if",
+                        [
+                            Function("equals", [project_threshold_config_index, 0]),
+                            ("duration", 300),
+                            Function(
+                                "arrayElement", [[("lcp", 100)], project_threshold_config_index]
+                            ),
+                        ],
+                    ),
+                    Function(
+                        "arrayElement",
+                        [[("lcp", 200)], project_threshold_override_config_index],
+                    ),
+                ],
+                "project_threshold_config",
+            )
+            in mock_snql_query.call_args_list[0][0][0].select
+        )