mikebridge commented on code in PR #45904:
URL: https://github.com/apache/superset/pull/45904#discussion_r4448661283


##########
superset-core/src/superset_core/semantic_layers/testing.py:
##########
@@ -154,6 +158,100 @@ def test_query_outputs(
             d.name for d in query.dimensions
         } | {m.name for m in query.metrics}
 
+    @pytest.mark.parametrize("operator", [Operator.IN, Operator.NOT_IN])
+    def test_empty_membership(
+        self,
+        operator: Operator,
+        semantic_view: SemanticView,
+        semantic_query_cases: Mapping[str, tuple[SemanticQuery, pa.Table]],
+    ) -> None:
+        """Empty IN is FALSE and empty NOT IN is TRUE where the filter sits.
+
+        Derived from the ``identity`` case. The filtered member is the one 
with a
+        NULL group, so an empty NOT IN that drops NULL rows fails.
+        """
+        query: SemanticQuery
+        expected: pa.Table
+        query, expected = semantic_query_cases["identity"]
+        dimension: Dimension | None = next(
+            (d for d in query.dimensions if 
expected.column(d.name).null_count),
+            None,
+        )
+        assert dimension is not None, (
+            "identity fixture requires a NULL group to witness empty NOT IN"
+        )
+        empty: Filter = Filter(PredicateType.WHERE, dimension, operator, 
frozenset())
+        filters: set[FilterExpression] = set(query.filters or set())
+        grouped: SemanticResult = semantic_view.get_table(
+            replace(query, filters={*filters, empty})
+        )
+        _assert_result_equal(
+            grouped.results,
+            expected if operator == Operator.NOT_IN else expected.slice(0, 0),
+        )
+        if not query.metrics:
+            return
+        # Without grouping, SQL still aggregates zero input rows into one row.
+        # Keep the fixture's other fields, such as its selection identity 
version.
+        ungrouped: SemanticQuery = replace(
+            query,
+            dimensions=[],
+            filters={*filters, empty},
+            order=None,
+            limit=None,
+            offset=None,
+            group_limit=None,
+        )
+        actual: pa.Table = semantic_view.get_table(ungrouped).results
+        if operator == Operator.NOT_IN:
+            unfiltered: SemanticQuery = replace(ungrouped, filters=filters or 
None)
+            _assert_result_equal(actual, 
semantic_view.get_table(unfiltered).results)
+            return
+        assert actual.num_rows == 1, (
+            "an ungrouped aggregate over an empty IN returns one row"
+        )
+        metric: Metric
+        for metric in query.metrics:
+            value: object = actual.column(metric.name)[0].as_py()
+            if metric.aggregation in _COUNTS:

Review Comment:
   Fixed in 0690ba8d99. The example provider includes COUNT(*) with 
AggregationType.COUNT, and the identity fixture requests it alongside SUM. The 
new mutation returns a typed NULL for the ungrouped empty-IN count and must 
raise the count-specific AssertionError. It failed before the COUNT fixture was 
added, passes with it, and fails again when the COUNT assertion branch is 
disabled. The existing wrong-SUM mutation preserves the added count column. 
Final semantic-layer, SDK and MCP chart-data tests: 2,301 passed, 4 skipped.



-- 
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]

  • [PR] fix(se... mikebridge (via GitHub) via notifications
    • Re: [P... netlify[bot] (via GitHub) via notifications
    • Re: [P... codecov[bot] (via GitHub) via notifications
    • Re: [P... mikebridge (via GitHub) via notifications
    • Re: [P... sadpandajoe (via GitHub) via notifications
    • Re: [P... mikebridge (via GitHub) via notifications
    • Re: [P... sadpandajoe (via GitHub) via notifications
    • Re: [P... sadpandajoe (via GitHub) via notifications
    • Re: [P... codeant-ai-for-open-source[bot] (via GitHub) via notifications
    • Re: [P... mikebridge (via GitHub) via notifications
    • Re: [P... mikebridge (via GitHub) via notifications

Reply via email to