handle mssql error

Author: goldmedal

ibis-server/app/model/connector.py

@@ -94,6 +94,20 @@ class MSSqlConnector(SimpleConnector):
     def __init__(self, connection_info: ConnectionInfo):
         super().__init__(DataSource.mssql, connection_info)
 
+    @tracer.start_as_current_span("connector_query", kind=trace.SpanKind.CLIENT)
+    def query(self, sql: str, limit: int | None = None) -> pa.Table:
+        try:
+            return super().query(sql, limit)
+        except Exception as e:
+            # To descirbe the query result, ibis will wrap the query with a subquery. MSSQL doesn't
+            # allow order by without limit in a subquery, so we need to handle this error and provide a more user-friendly error message.
+            # error code 1033: https://learn.microsoft.com/zh-tw/sql/relational-databases/errors-events/database-engine-events-and-errors-1000-to-1999?view=sql-server-ver15
+            if "(1033)" in e.args[1]:
+                raise GenericUserError(
+                    "The query with order-by requires a specific limit to be set in MSSQL."
+                )
+            raise
+
     def dry_run(self, sql: str) -> None:
         try:
             super().dry_run(sql)
@@ -327,3 +341,12 @@ def _get_pg_type_names(connection: BaseBackend) -> dict[int, str]:
 
 class QueryDryRunError(UnprocessableEntityError):
     pass
+
+
+class GenericUserError(UnprocessableEntityError):
+    def __init__(self, message: str):
+        super().__init__(message)
+        self.message = message
+
+    def __str__(self) -> str:
+        return self.message

ibis-server/tests/routers/v2/connector/test_mssql.py

@@ -64,6 +64,17 @@
             ],
             "primaryKey": "orderkey",
         },
+        {
+            "name": "null_test",
+            "tableReference": {
+                "schema": "dbo",
+                "table": "null_test",
+            },
+            "columns": [
+                {"name": "id", "type": "integer"},
+                {"name": "letter", "type": "varchar"},
+            ],
+        },
     ],
 }
 
@@ -103,6 +114,13 @@ def mssql(request) -> SqlServerContainer:
                     @level2type = N'COLUMN', @level2name = 'o_comment';
             """)
         )
+        conn.execute(text('CREATE TABLE "null_test" ("id" INT, "letter" TEXT)'))
+        conn.execute(
+            text(
+                "INSERT INTO \"null_test\" (\"id\", \"letter\") VALUES (1, 'one'), (2, 'two'), (NULL, 'three')"
+            )
+        )
+
     request.addfinalizer(mssql.stop)
     return mssql
 
@@ -438,6 +456,58 @@ async def test_password_with_special_characters(client):
         assert "Microsoft SQL Server 2019" in response.text
 
 
+async def test_order_by_nulls_last(client, manifest_str, mssql: SqlServerContainer):
+    connection_info = _to_connection_info(mssql)
+    response = await client.post(
+        url=f"{base_url}/query",
+        json={
+            "connectionInfo": connection_info,
+            "manifestStr": manifest_str,
+            "sql": 'SELECT letter FROM "null_test" ORDER BY id',
+        },
+        params={"limit": 3},
+    )
+    assert response.status_code == 200
+    result = response.json()
+    assert len(result["data"]) == 3
+    assert result["data"][0][0] == "one"
+    assert result["data"][1][0] == "two"
+    assert result["data"][2][0] == "three"
+
+    connection_info = _to_connection_info(mssql)
+    response = await client.post(
+        url=f"{base_url}/query",
+        json={
+            "connectionInfo": connection_info,
+            "manifestStr": manifest_str,
+            "sql": 'SELECT letter FROM "null_test" ORDER BY id LIMIT 3',
+        },
+    )
+    assert response.status_code == 200
+    result = response.json()
+    assert len(result["data"]) == 3
+    assert result["data"][0][0] == "one"
+    assert result["data"][1][0] == "two"
+    assert result["data"][2][0] == "three"
+
+
+async def test_order_by_require_limit(client, manifest_str, mssql: SqlServerContainer):
+    connection_info = _to_connection_info(mssql)
+    response = await client.post(
+        url=f"{base_url}/query",
+        json={
+            "connectionInfo": connection_info,
+            "manifestStr": manifest_str,
+            "sql": 'SELECT letter FROM "null_test" ORDER BY id NULLS LAST',
+        },
+    )
+    assert response.status_code == 422
+    assert (
+        "The query with order-by requires a specific limit to be set in MSSQL."
+        in response.text
+    )
+
+
 def _to_connection_info(mssql: SqlServerContainer):
     return {
         "host": mssql.get_container_host_ip(),