sadpandajoe commented on code in PR #45133: URL: https://github.com/apache/superset/pull/45133#discussion_r4265262577
########## tests/unit_tests/semantic_layers/cache_or_filters_test.py: ########## @@ -0,0 +1,220 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Grouped predicates remain distinct, exact-match-only cache constraints.""" + +from dataclasses import replace +from unittest.mock import MagicMock + +import pyarrow as pa +import pytest +from superset_core.semantic_layers.types import ( + Filter, + FilterExpression, + FilterValues, + GroupLimit, + Operator, + OrFilter, + PredicateType, + SemanticQuery, + SemanticResult, +) + +from superset.semantic_layers.cache import ( + SemanticCacheOutcome, + SemanticCacheService, + SemanticCacheState, +) +from superset.semantic_layers.cache_identity import SemanticCacheIdentityFactory +from superset.semantic_layers.cache_policy import ContainmentCapabilities +from superset.semantic_layers.cache_repository import SemanticCacheRepository +from tests.unit_tests.semantic_layers.cache_repository_test import ( + _Backend, + _Coordinator, +) +from tests.unit_tests.semantic_layers.conftest import ( + build_semantic_query, + build_view_meta, +) + + +def _group(query: SemanticQuery, value: FilterValues = "GB") -> OrFilter: + """Build the mapper's mixed positive NULL selection using real SDK types.""" + return OrFilter( + frozenset( + { + Filter( + PredicateType.WHERE, + query.dimensions[0], + Operator.IN, + frozenset({value}), + ), + Filter( + PredicateType.WHERE, query.dimensions[0], Operator.IS_NULL, None + ), + } + ) + ) + + +def test_group_identity_is_order_independent_and_not_flattened() -> None: + query: SemanticQuery = build_semantic_query() + group: OrFilter = _group(query) + reverse: OrFilter = OrFilter(frozenset(reversed(tuple(group.filters)))) Review Comment: This reversal may not exercise the order independence it names: `frozenset(reversed(tuple(group.filters)))` for two leaves often iterates in the same order as the original, so the assertion would still pass if the canonical leaf sorting in `_canonical_filters` were removed. Across workers (different hash seeds) the iteration order really does differ, so equivalent groups could then get different cache keys. Could the test build the two groups with a forced differing leaf order (e.g. by patching the iteration order) so a missing sort fails it? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
