From 27aec85c458167b26b1387d9642de4808f553b41 Mon Sep 17 00:00:00 2001 From: Enoch Tang Date: Thu, 25 Jan 2024 16:13:30 -0500 Subject: [PATCH] chore(entity): Remove runtime config for illegal aggregation validator (#5443) * remove runtime config for illegal aggregation validator * fix tests * remove invalid test. aggregation function should not be in WHERE clause * fix test --- snuba/query/validation/validators.py | 20 ++--- tests/clickhouse/query_dsl/test_project_id.py | 2 - ...illegal_aggregate_conditions_validation.py | 2 - tests/query/processors/test_prewhere.py | 81 ------------------- 4 files changed, 7 insertions(+), 98 deletions(-) diff --git a/snuba/query/validation/validators.py b/snuba/query/validation/validators.py index 70218625e0..2759441a66 100644 --- a/snuba/query/validation/validators.py +++ b/snuba/query/validation/validators.py @@ -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, @@ -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" + ) diff --git a/tests/clickhouse/query_dsl/test_project_id.py b/tests/clickhouse/query_dsl/test_project_id.py index 1e96b70e9d..964c0b43c8 100644 --- a/tests/clickhouse/query_dsl/test_project_id.py +++ b/tests/clickhouse/query_dsl/test_project_id.py @@ -101,7 +101,6 @@ ["event_id", "=", "something"], [["ifNull", ["partition", 0]], "=", 1], ["project_id", "IN", [100, 200, 300]], - [["count", ["offset"]], "=", 10], ["project_id", "=", 100], ], }, @@ -128,7 +127,6 @@ ["project_id", "IN", [100, 200, 300]], [ [["ifNull", ["project_id", 1000]], "=", 100], - [["count", ["offset"]], "=", 10], [["ifNull", ["project_id", 1000]], "=", 200], ], ], diff --git a/tests/datasets/validation/test_illegal_aggregate_conditions_validation.py b/tests/datasets/validation/test_illegal_aggregate_conditions_validation.py index bebd5fdbb9..ec94bb0428 100644 --- a/tests/datasets/validation/test_illegal_aggregate_conditions_validation.py +++ b/tests/datasets/validation/test_illegal_aggregate_conditions_validation.py @@ -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 @@ -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=[ diff --git a/tests/query/processors/test_prewhere.py b/tests/query/processors/test_prewhere.py index 392e5355ae..9061deedc3 100644 --- a/tests/query/processors/test_prewhere.py +++ b/tests/query/processors/test_prewhere.py @@ -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", - ), ]