Repository: ibis-project/ibis

Issue:
test(mssql): flaky test around dataframe interchange protocol
[This CI run](https://github.com/ibis-project/ibis/actions/runs/10042271184/job/27752348001) fails every once in a while in the same way. Not sure what's happening but it would be nice if we could track down the flakiness at some point.

Discussion:

Patch:
diff --git a/ibis/backends/mssql/__init__.py b/ibis/backends/mssql/__init__.py
index a1adff0fb..a25985076 100644
--- a/ibis/backends/mssql/__init__.py
+++ b/ibis/backends/mssql/__init__.py
@@ -114,8 +114,7 @@ class Backend(SQLBackend, CanCreateCatalog, CanCreateDatabase, CanCreateSchema,
 
         con = pyodbc.connect(
             user=user,
-            server=host,
-            port=port,
+            server=f"{host},{port}",
             password=password,
             database=database,
             driver=driver,
@@ -252,6 +251,11 @@ class Backend(SQLBackend, CanCreateCatalog, CanCreateDatabase, CanCreateSchema,
 
     @contextlib.contextmanager
     def begin(self):
+        with contextlib.closing(self.con.cursor()) as cur:
+            yield cur
+
+    @contextlib.contextmanager
+    def _ddl_begin(self):
         con = self.con
         cur = con.cursor()
         try:
@@ -273,6 +277,15 @@ class Backend(SQLBackend, CanCreateCatalog, CanCreateDatabase, CanCreateSchema,
             cur.execute(query, *args, **kwargs)
             yield cur
 
+    @contextlib.contextmanager
+    def _safe_ddl(self, query, *args, **kwargs):
+        with contextlib.suppress(AttributeError):
+            query = query.sql(self.dialect)
+
+        with self._ddl_begin() as cur:
+            cur.execute(query, *args, **kwargs)
+            yield cur
+
     def raw_sql(self, query: str | sg.Expression, **kwargs: Any) -> Any:
         with contextlib.suppress(AttributeError):
             query = query.sql(self.dialect)
@@ -280,15 +293,8 @@ class Backend(SQLBackend, CanCreateCatalog, CanCreateDatabase, CanCreateSchema,
         con = self.con
         cursor = con.cursor()
 
-        try:
-            cursor.execute(query, **kwargs)
-        except Exception:
-            con.rollback()
-            cursor.close()
-            raise
-        else:
-            con.commit()
-            return cursor
+        cursor.execute(query, **kwargs)
+        return cursor
 
     def create_catalog(self, name: str, force: bool = False) -> None:
         expr = (
@@ -309,11 +315,11 @@ GO"""
             if force
             else stmt
         )
-        with self._safe_raw_sql(create_stmt):
+        with self._safe_ddl(create_stmt):
             pass
 
     def drop_catalog(self, name: str, force: bool = False) -> None:
-        with self._safe_raw_sql(
+        with self._safe_ddl(
             sge.Drop(
                 kind="DATABASE",
                 this=sg.to_identifier(name, quoted=self.compiler.quoted),
@@ -584,7 +590,7 @@ GO"""
 
         this = sg.table(name, catalog=catalog, db=db, quoted=self.compiler.quoted)
         raw_this = sg.table(name, catalog=catalog, db=db, quoted=False)
-        with self._safe_raw_sql(create_stmt) as cur:
+        with self._safe_ddl(create_stmt) as cur:
             if query is not None:
                 # You can specify that a table is temporary for the sqlglot `Create` but not
                 # for the subsequent `Insert`, so we need to shove a `#` in
@@ -666,7 +672,7 @@ GO"""
             data = df.itertuples(index=False)
 
             insert_stmt = self._build_insert_template(name, schema=schema, columns=True)
-            with self._safe_raw_sql(create_stmt) as cur:
+            with self._safe_ddl(create_stmt) as cur:
                 if not df.empty:
                     cur.executemany(insert_stmt, data)
 
diff --git a/ibis/common/selectors.py b/ibis/common/selectors.py
new file mode 100644
index 000000000..10e1a96f1
--- /dev/null
+++ b/ibis/common/selectors.py
@@ -0,0 +1,31 @@
+from __future__ import annotations
+
+import abc
+from typing import TYPE_CHECKING
+
+from ibis.common.grounds import Concrete
+
+if TYPE_CHECKING:
+    from collections.abc import Sequence
+
+    import ibis.expr.types as ir
+
+
+class Selector(Concrete):
+    """A column selector."""
+
+    @abc.abstractmethod
+    def expand(self, table: ir.Table) -> Sequence[ir.Value]:
+        """Expand `table` into value expressions that match the selector.
+
+        Parameters
+        ----------
+        table
+            An ibis table expression
+
+        Returns
+        -------
+        Sequence[Value]
+            A sequence of value expressions that match the selector
+
+        """
diff --git a/ibis/expr/builders.py b/ibis/expr/builders.py
index 59d131123..1cb5b194c 100644
--- a/ibis/expr/builders.py
+++ b/ibis/expr/builders.py
@@ -13,6 +13,7 @@ from ibis.common.annotations import annotated, attribute
 from ibis.common.deferred import Deferred, Resolver, deferrable
 from ibis.common.exceptions import IbisInputError
 from ibis.common.grounds import Concrete
+from ibis.common.selectors import Selector  # noqa: TCH001
 from ibis.common.typing import VarTuple  # noqa: TCH001
 
 if TYPE_CHECKING:
@@ -145,8 +146,8 @@ class WindowBuilder(Builder):
     how: Literal["rows", "range"] = "rows"
     start: Optional[RangeWindowBoundary] = None
     end: Optional[RangeWindowBoundary] = None
-    groupings: VarTuple[Union[str, Resolver, ops.Value]] = ()
-    orderings: VarTuple[Union[str, Resolver, ops.SortKey]] = ()
+    groupings: VarTuple[Union[str, Resolver, Selector, ops.Value]] = ()
+    orderings: VarTuple[Union[str, Resolver, Selector, ops.SortKey]] = ()
 
     @attribute
     def _table(self):
diff --git a/ibis/expr/types/numeric.py b/ibis/expr/types/numeric.py
index 36da3740a..c99c357c7 100644
--- a/ibis/expr/types/numeric.py
+++ b/ibis/expr/types/numeric.py
@@ -836,7 +836,10 @@ class NumericColumn(Column, NumericValue):
             The correlation of `left` and `right`
         """
         return ops.Correlation(
-            self, right, how=how, where=self._bind_to_parent_table(where)
+            self,
+            self._bind_to_parent_table(right),
+            how=how,
+            where=self._bind_to_parent_table(where),
         ).to_expr()
 
     def cov(
@@ -862,7 +865,10 @@ class NumericColumn(Column, NumericValue):
             The covariance of `self` and `right`
         """
         return ops.Covariance(
-            self, right, how=how, where=self._bind_to_parent_table(where)
+            self,
+            self._bind_to_parent_table(right),
+            how=how,
+            where=self._bind_to_parent_table(where),
         ).to_expr()
 
     def mean(
diff --git a/ibis/expr/types/relations.py b/ibis/expr/types/relations.py
index 7fd384e4b..0198f5a53 100644
--- a/ibis/expr/types/relations.py
+++ b/ibis/expr/types/relations.py
@@ -18,12 +18,12 @@ import ibis.expr.operations as ops
 import ibis.expr.schema as sch
 from ibis import util
 from ibis.common.deferred import Deferred, Resolver
+from ibis.common.selectors import Selector
 from ibis.expr.rewrites import DerefMap
 from ibis.expr.types.core import Expr, _FixedTextJupyterMixin
 from ibis.expr.types.generic import Value, literal
 from ibis.expr.types.pretty import to_rich
 from ibis.expr.types.temporal import TimestampColumn
-from ibis.selectors import Selector
 from ibis.util import deprecated
 
 if TYPE_CHECKING:
diff --git a/ibis/selectors.py b/ibis/selectors.py
index 6af620c6c..2b89e1ffb 100644
--- a/ibis/selectors.py
+++ b/ibis/selectors.py
@@ -50,7 +50,6 @@ Using a composition of selectors this is much less tiresome:
 
 from __future__ import annotations
 
-import abc
 import functools
 import inspect
 import operator
@@ -67,27 +66,8 @@ from ibis import util
 from ibis.common.collections import frozendict  # noqa: TCH001
 from ibis.common.deferred import Deferred, Resolver
 from ibis.common.exceptions import IbisError
-from ibis.common.grounds import Concrete, Singleton
-
-
-class Selector(Concrete):
-    """A column selector."""
-
-    @abc.abstractmethod
-    def expand(self, table: ir.Table) -> Sequence[ir.Value]:
-        """Expand `table` into value expressions that match the selector.
-
-        Parameters
-        ----------
-        table
-            An ibis table expression
-
-        Returns
-        -------
-        Sequence[Value]
-            A sequence of value expressions that match the selector
-
-        """
+from ibis.common.grounds import Singleton
+from ibis.common.selectors import Selector
 
 
 class Predicate(Selector):


Test patch:
diff --git a/ibis/expr/tests/test_reductions.py b/ibis/expr/tests/test_reductions.py
index ccca62988..5f27f25c4 100644
--- a/ibis/expr/tests/test_reductions.py
+++ b/ibis/expr/tests/test_reductions.py
@@ -106,3 +106,10 @@ def test_argminmax_deferred(func_name):
     t = ibis.table({"a": "int", "b": "int"}, name="t")
     func = getattr(t.a, func_name)
     assert func(_.b).equals(func(t.b))
+
+
+@pytest.mark.parametrize("func_name", ["cov", "corr"])
+def test_cov_corr_deferred(func_name):
+    t = ibis.table({"a": "int", "b": "int"}, name="t")
+    func = getattr(t.a, func_name)
+    assert func(_.b).equals(func(t.b))
diff --git a/ibis/tests/expr/test_selectors.py b/ibis/tests/expr/test_selectors.py
index 62c7821e9..fac487163 100644
--- a/ibis/tests/expr/test_selectors.py
+++ b/ibis/tests/expr/test_selectors.py
@@ -494,3 +494,26 @@ def test_order_by_with_selectors(penguins):
 
     with pytest.raises(exc.IbisError):
         penguins.order_by(~s.all())
+
+
+def test_window_function_group_by(penguins):
+    expr = penguins.species.count().over(group_by=s.c("island"))
+    assert expr.equals(penguins.species.count().over(group_by=penguins.island))
+
+
+def test_window_function_order_by(penguins):
+    expr = penguins.island.count().over(order_by=s.c("species"))
+    assert expr.equals(penguins.island.count().over(order_by=penguins.species))
+
+
+def test_window_function_group_by_order_by(penguins):
+    expr = penguins.species.count().over(
+        group_by=s.c("island"),
+        order_by=s.c("year") | (~s.c("island", "species") & s.of_type("str")),
+    )
+    assert expr.equals(
+        penguins.species.count().over(
+            group_by=penguins.island,
+            order_by=[penguins.sex, penguins.year],
+        )
+    )