Skip to content

Commit

Permalink
Revert "feat(replays): Remove scalar query optimization (#61815)"
Browse files Browse the repository at this point in the history
This reverts commit ed3d807.

Co-authored-by: cmanallen <[email protected]>
  • Loading branch information
getsentry-bot and cmanallen committed Dec 21, 2023
1 parent c02e3a1 commit 08821e1
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 33 deletions.
6 changes: 5 additions & 1 deletion src/sentry/replays/lib/new_query/parsers.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,8 @@ def parse_uuid(value: str) -> uuid.UUID:
try:
return uuid.UUID(value)
except ValueError:
raise CouldNotParseValue("Failed to parse uuid.")
# 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")
35 changes: 26 additions & 9 deletions src/sentry/replays/usecases/query/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,11 @@ 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.scalar import scalar_search_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,
)

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

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

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"
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"

if pagination:
query = query.set_limit(pagination.limit)
Expand Down
14 changes: 0 additions & 14 deletions src/sentry/replays/usecases/query/conditions/event_ids.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,7 @@ 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 @@ -92,12 +89,9 @@ 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 @@ -107,13 +101,9 @@ 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 @@ -122,13 +112,9 @@ 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: 7 additions & 9 deletions tests/sentry/replays/test_organization_replay_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,11 +605,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 duration:0) AND (duration:>15 OR platform:nothing))",
f"((id:{replay1_id} OR id:b) AND (duration:>15 OR id:d))",
# Implicit paren wrapped expression.
f"(id:{replay1_id} OR duration:0) AND (duration:>15 OR platform:nothing)",
f"(id:{replay1_id} OR id:b) AND (duration:>15 OR id:d)",
# Implicit And.
f"(id:{replay1_id} OR duration:0) OR (duration:>15 platform:javascript)",
f"(id:{replay1_id} OR id:b) OR (duration:>15 platform:javascript)",
# Tag filters.
"tags[a]:m",
"a:m",
Expand Down Expand Up @@ -641,24 +641,22 @@ 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",
f"error_ids:{missing_uuid}",
"error_ids:123",
"!error_id:a3a62ef6ac86415b83c2416fc2f76db1",
f"error_id:{missing_uuid}",
"error_id:123",
"!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:{missing_uuid}",
f"id:{replay1_id} AND id:b",
f"id:{replay1_id} AND duration:>1000",
f"id:{missing_uuid} OR duration:>1000",
"id:b OR duration:>1000",
"a:o",
"a:[o,p]",
"releases:a",
Expand Down

0 comments on commit 08821e1

Please sign in to comment.