bito-code-review[bot] commented on code in PR #44454: URL: https://github.com/apache/superset/pull/44454#discussion_r4059740202
########## superset/semantic_layers/dimension_resolution.py: ########## @@ -0,0 +1,81 @@ +# 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. + +"""Default preference for immutable semantic dimension variants.""" + +from collections.abc import Iterable + +from flask_babel import gettext as _ +from superset_core.semantic_layers.types import Dimension, Grain + +from superset.exceptions import QueryObjectValidationError + +# Known grains rank finest to coarsest, ahead of unknown representations. +_GRAIN_FINENESS: dict[str, int] = { + "PT1S": 0, + "PT1M": 1, + "PT1H": 2, + "P1D": 3, + "P1W": 4, + "P1M": 5, + "P3M": 6, + "P1Y": 7, +} + + +def grain_preference(dimension: Dimension) -> tuple[int, int, str]: + """Prefer raw, then finest known grain, then lexical representation.""" + grain: Grain | None = dimension.grain + if grain is None: + return (0, 0, "") + representation: str = grain.representation + return ( + 1, + _GRAIN_FINENESS.get(representation, len(_GRAIN_FINENESS)), + representation, + ) + + +def resolve_dimension_defaults( + dimensions: Iterable[Dimension], +) -> dict[str, Dimension]: + """Resolve names without hiding conflicting identities for any grain. + + Return the original provider objects. Even non-preferred grains must be + unambiguous: explicit grouping can select them independently of defaults. + """ + defaults: dict[str, Dimension] = {} + identities: dict[tuple[str, Grain | None], str] = {} + for dimension in dimensions: + key: tuple[str, Grain | None] = (dimension.name, dimension.grain) + previous_id: str | None = identities.get(key) + if previous_id is not None and previous_id != dimension.id: + raise QueryObjectValidationError( + _( + "Semantic dimension '%(name)s' has ambiguous variants for " + "grain '%(grain)s'. Use one ID per name and grain.", + name=dimension.name, + grain=dimension.grain.representation if dimension.grain else "raw", + ) + ) + identities[key] = dimension.id + preferred: Dimension | None = defaults.get(dimension.name) + if preferred is None or grain_preference(dimension) < grain_preference( + preferred + ): Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Redundant per-iteration recompute</b></div> <div id="fix"> `grain_preference(preferred)` is recomputed on every iteration for the incumbent `defaults[dimension.name]`, so each name's key is re-derived O(n) times instead of once. Hoist the preferred key out of the comparison (or cache it next to the stored `Dimension`) to make the pass single-evaluation per item. </div> </div> <small><i>Code Review Run #87c627</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them -- 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]
