Skip to content

Commit

Permalink
chore(entity): Remove runtime config for illegal aggregation validator (
Browse files Browse the repository at this point in the history
#5443)

* remove runtime config for illegal aggregation validator

* fix tests

* remove invalid test. aggregation function should not be in WHERE clause

* fix test
  • Loading branch information
enochtangg committed Jan 25, 2024
1 parent b5311b4 commit 27aec85
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 98 deletions.
20 changes: 7 additions & 13 deletions snuba/query/validation/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from enum import Enum
from typing import Optional, Sequence, Type, cast

from snuba import state
from snuba.clickhouse.translators.snuba.allowed import DefaultNoneColumnMapper
from snuba.clickhouse.translators.snuba.function_call_mappers import (
AggregateCurriedFunctionMapper,
Expand Down Expand Up @@ -507,15 +506,10 @@ def find_illegal_aggregate_functions(
)
return matches

enable_illegal_aggregate_validator = state.get_config(
"enable_illegal_aggregate_in_condition_validator", 0
)
assert enable_illegal_aggregate_validator is not None
if int(enable_illegal_aggregate_validator) == 1:
conditions = query.get_condition()
if conditions:
matches = find_illegal_aggregate_functions(conditions)
if len(matches) > 0:
raise InvalidQueryException(
"Aggregate function found in WHERE clause of query"
)
conditions = query.get_condition()
if conditions:
matches = find_illegal_aggregate_functions(conditions)
if len(matches) > 0:
raise InvalidQueryException(
"Aggregate function found in WHERE clause of query"
)
2 changes: 0 additions & 2 deletions tests/clickhouse/query_dsl/test_project_id.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@
["event_id", "=", "something"],
[["ifNull", ["partition", 0]], "=", 1],
["project_id", "IN", [100, 200, 300]],
[["count", ["offset"]], "=", 10],
["project_id", "=", 100],
],
},
Expand All @@ -128,7 +127,6 @@
["project_id", "IN", [100, 200, 300]],
[
[["ifNull", ["project_id", 1000]], "=", 100],
[["count", ["offset"]], "=", 10],
[["ifNull", ["project_id", 1000]], "=", 200],
],
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

import pytest

from snuba import state
from snuba.datasets.entities.entity_key import EntityKey
from snuba.datasets.entities.factory import get_entity
from snuba.query import SelectedExpression
Expand Down Expand Up @@ -138,7 +137,6 @@ def test_illegal_aggregate_in_condition_validator(
having: Optional[Expression],
exception: Exception,
) -> None:
state.set_config("enable_illegal_aggregate_in_condition_validator", 1)
query = LogicalQuery(
QueryEntity(EntityKey.EVENTS, get_entity(EntityKey.EVENTS).get_data_model()),
selected_columns=[
Expand Down
81 changes: 0 additions & 81 deletions tests/query/processors/test_prewhere.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,87 +209,6 @@
),
True,
),
pytest.param(
{"conditions": [[["uniq", ["environment"]], "=", "abc"]]},
[
"event_id",
"release",
"message",
"transaction_name",
"environment",
"project_id",
],
[],
FunctionCall(
None,
OPERATOR_TO_FUNCTION["="],
(
FunctionCall(
None,
"uniq",
(Column("_snuba_environment", None, "environment"),),
),
Literal(None, "abc"),
),
),
FunctionCall(
None,
OPERATOR_TO_FUNCTION["="],
(
Column("_snuba_project_id", None, "project_id"),
Literal(None, 1),
),
),
False,
id="Do not promote a column that is in a uniq function",
),
pytest.param(
{
"conditions": [
[["countIf", [["equals", ["environment", "'production'"]]]], "=", "abc"]
]
},
[
"event_id",
"release",
"message",
"transaction_name",
"environment",
"project_id",
],
[],
FunctionCall(
None,
OPERATOR_TO_FUNCTION["="],
(
FunctionCall(
None,
"countIf",
(
FunctionCall(
None,
"equals",
(
Column("_snuba_environment", None, "environment"),
Literal(None, "production"),
),
),
),
),
Literal(None, "abc"),
),
),
FunctionCall(
None,
OPERATOR_TO_FUNCTION["="],
(
Column("_snuba_project_id", None, "project_id"),
Literal(None, 1),
),
),
False,
id="Do not promote a column that is in a countIf function",
),
]


Expand Down

0 comments on commit 27aec85

Please sign in to comment.