Skip to content

Commit

Permalink
feat(replays): Remove scalar query optimization (#61815)
Browse files Browse the repository at this point in the history
Fixes: #61418
  • Loading branch information
cmanallen authored Dec 19, 2023
1 parent 69fc164 commit ed3d807
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 38 deletions.
6 changes: 1 addition & 5 deletions src/sentry/replays/lib/new_query/parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,4 @@ def parse_uuid(value: str) -> uuid.UUID:
try:
return uuid.UUID(value)
except ValueError:
# Return an empty uuid. This emulates current behavior where inability to parse a UUID
# leads to an empty result-set rather than an error.
#
# TODO: Probably raise an error here...
return uuid.UUID("00000000000000000000000000000000")
raise CouldNotParseValue("Failed to parse uuid.")
35 changes: 9 additions & 26 deletions src/sentry/replays/usecases/query/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,7 @@ def search_filter_to_condition(
# Leaving it here for now so this is easier to review/remove.
from sentry.replays.usecases.query.configs.aggregate import search_config as agg_search_config
from sentry.replays.usecases.query.configs.aggregate_sort import sort_config as agg_sort_config
from sentry.replays.usecases.query.configs.aggregate_sort import sort_is_scalar_compatible
from sentry.replays.usecases.query.configs.scalar import (
can_scalar_search_subquery,
scalar_search_config,
)
from sentry.replays.usecases.query.configs.scalar import scalar_search_config

Paginators = namedtuple("Paginators", ("limit", "offset"))

Expand All @@ -163,27 +159,14 @@ def query_using_optimized_search(
SearchFilter(SearchKey("environment"), "IN", SearchValue(environments)),
]

can_scalar_sort = sort_is_scalar_compatible(sort or "started_at")
can_scalar_search = can_scalar_search_subquery(search_filters)

if can_scalar_sort and can_scalar_search:
query = make_scalar_search_conditions_query(
search_filters=search_filters,
sort=sort,
project_ids=project_ids,
period_start=period_start,
period_stop=period_stop,
)
referrer = "replays.query.browse_scalar_conditions_subquery"
else:
query = make_aggregate_search_conditions_query(
search_filters=search_filters,
sort=sort,
project_ids=project_ids,
period_start=period_start,
period_stop=period_stop,
)
referrer = "replays.query.browse_aggregated_conditions_subquery"
query = make_aggregate_search_conditions_query(
search_filters=search_filters,
sort=sort,
project_ids=project_ids,
period_start=period_start,
period_stop=period_stop,
)
referrer = "replays.query.browse_aggregated_conditions_subquery"

if pagination:
query = query.set_limit(pagination.limit)
Expand Down
14 changes: 14 additions & 0 deletions src/sentry/replays/usecases/query/conditions/event_ids.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ class ErrorIdScalar(ComputedBase):

@classmethod
def visit_eq(cls, value: UUID) -> Condition:
from sentry.replays.usecases.query.conditions.error_ids import ErrorIdsArray

conditions = _make_conditions_from_column_names(cls.event_id_columns, Op.EQ, to_uuid(value))
conditions.append(translate_condition_to_function(ErrorIdsArray.visit_eq(value)))

return Condition(
Function("or", conditions),
Expand All @@ -89,9 +92,12 @@ def visit_eq(cls, value: UUID) -> Condition:

@classmethod
def visit_neq(cls, value: UUID) -> Condition:
from sentry.replays.usecases.query.conditions.error_ids import ErrorIdsArray

conditions = _make_conditions_from_column_names(
cls.event_id_columns, Op.NEQ, to_uuid(value)
)
conditions.append(translate_condition_to_function(ErrorIdsArray.visit_neq(value)))

return Condition(
Function("and", conditions),
Expand All @@ -101,9 +107,13 @@ def visit_neq(cls, value: UUID) -> Condition:

@classmethod
def visit_in(cls, value: list[UUID]) -> Condition:
from sentry.replays.usecases.query.conditions.error_ids import ErrorIdsArray

conditions = _make_conditions_from_column_names(
cls.event_id_columns, Op.IN, [str(v) for v in value]
)
conditions.append(translate_condition_to_function(ErrorIdsArray.visit_in(value)))

return Condition(
Function("or", conditions),
Op.EQ,
Expand All @@ -112,9 +122,13 @@ def visit_in(cls, value: list[UUID]) -> Condition:

@classmethod
def visit_not_in(cls, value: list[UUID]) -> Condition:
from sentry.replays.usecases.query.conditions.error_ids import ErrorIdsArray

conditions = _make_conditions_from_column_names(
cls.event_id_columns, Op.NOT_IN, [str(v) for v in value]
)
conditions.append(translate_condition_to_function(ErrorIdsArray.visit_not_in(value)))

return Condition(
Function("and", conditions),
Op.EQ,
Expand Down
16 changes: 9 additions & 7 deletions tests/sentry/replays/test_organization_replay_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -603,11 +603,11 @@ def test_get_replays_user_filters(self):
# Or expression.
f"id:{replay1_id} OR id:{uuid.uuid4().hex} OR id:{uuid.uuid4().hex}",
# Paren wrapped expression.
f"((id:{replay1_id} OR id:b) AND (duration:>15 OR id:d))",
f"((id:{replay1_id} OR duration:0) AND (duration:>15 OR platform:nothing))",
# Implicit paren wrapped expression.
f"(id:{replay1_id} OR id:b) AND (duration:>15 OR id:d)",
f"(id:{replay1_id} OR duration:0) AND (duration:>15 OR platform:nothing)",
# Implicit And.
f"(id:{replay1_id} OR id:b) OR (duration:>15 platform:javascript)",
f"(id:{replay1_id} OR duration:0) OR (duration:>15 platform:javascript)",
# Tag filters.
"tags[a]:m",
"a:m",
Expand Down Expand Up @@ -639,22 +639,24 @@ def test_get_replays_user_filters(self):
response_data = response.json()
assert len(response_data["data"]) == 1, "all queries"

missing_uuid = "f8a783a4261a4b559f108c3721fc05cc"

# Assert returns empty result sets.
null_queries = [
"!replay_type:session",
"!error_ids:a3a62ef6ac86415b83c2416fc2f76db1",
"error_ids:123",
f"error_ids:{missing_uuid}",
"!error_id:a3a62ef6ac86415b83c2416fc2f76db1",
"error_id:123",
f"error_id:{missing_uuid}",
"!trace_ids:4491657243ba4dbebd2f6bd62b733080",
"!trace_id:4491657243ba4dbebd2f6bd62b733080",
"!trace:4491657243ba4dbebd2f6bd62b733080",
"count_urls:0",
"count_dead_clicks:>0",
"count_rage_clicks:>0",
f"id:{replay1_id} AND id:b",
f"id:{replay1_id} AND id:{missing_uuid}",
f"id:{replay1_id} AND duration:>1000",
"id:b OR duration:>1000",
f"id:{missing_uuid} OR duration:>1000",
"a:o",
"a:[o,p]",
"releases:a",
Expand Down

0 comments on commit ed3d807

Please sign in to comment.