mikebridge commented on code in PR #45108:
URL: https://github.com/apache/superset/pull/45108#discussion_r4235501882
##########
superset-core/src/superset_core/semantic_layers/view.py:
##########
@@ -44,6 +44,32 @@ class SemanticViewFeature(enum.Enum):
class SemanticView(ABC):
"""
Abstract base class for semantic views.
+
+ Implementations own translation and execution; the rules the host relies on
+ are fixed here so every provider agrees on them:
+
+ - **Member identity.** ``Metric.name`` and ``Dimension.name`` are the
stable
+ selection keys; ``verbose_name`` is display only and ``id`` is
+ provider-private. See
:class:`~superset_core.semantic_layers.types.Metric`.
+ A provider that changes what ``name`` means declares a new
+ :attr:`selection_identity_version`.
+ - **Grain variants.** Several dimension variants may share one ``name``, at
+ most one per ``(name, grain)``. See
+ :class:`~superset_core.semantic_layers.types.Grains` for the preference
+ the host applies, and for why grains must not be ordered by name.
Review Comment:
Fixed in c3d1494: the summary now calls it the intended preference that not
every host lookup applies yet. The `Grains` docstring already spells out the
current behavior.
##########
superset-core/src/superset_core/semantic_layers/view.py:
##########
@@ -44,6 +44,32 @@ class SemanticViewFeature(enum.Enum):
class SemanticView(ABC):
"""
Abstract base class for semantic views.
+
+ Implementations own translation and execution; the rules the host relies on
+ are fixed here so every provider agrees on them:
+
+ - **Member identity.** ``Metric.name`` and ``Dimension.name`` are the
stable
+ selection keys; ``verbose_name`` is display only and ``id`` is
+ provider-private. See
:class:`~superset_core.semantic_layers.types.Metric`.
+ A provider that changes what ``name`` means declares a new
+ :attr:`selection_identity_version`.
+ - **Grain variants.** Several dimension variants may share one ``name``, at
+ most one per ``(name, grain)``. See
+ :class:`~superset_core.semantic_layers.types.Grains` for the preference
+ the host applies, and for why grains must not be ordered by name.
+ - **NULL in filters.** ``None`` is never a comparison operand; splitting
+ nullness into ``IS_NULL``/``IS_NOT_NULL`` is the host's responsibility.
Review Comment:
Fixed in c3d1494: the summary now tells providers to keep handling `None`
until the host normalizes it. The `Operator` docstring already notes the
pass-through and points to #45133.
--
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]
