From fa7dd188e7bf8844107f17328aa48f08b63af4e0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= <aljaz@edgedb.com>
Date: Fri, 15 Nov 2024 15:32:30 +0100
Subject: [PATCH 1/2] Improve unsupported error message over SQL adapter

---
 edb/pgsql/parser/ast_builder.py |  6 ++++--
 edb/pgsql/parser/exceptions.py  | 17 ++++++++++++++---
 edb/pgsql/resolver/command.py   |  6 +-----
 edb/pgsql/resolver/dispatch.py  | 20 ++++++++++++++++++--
 tests/test_sql_dml.py           |  4 ++--
 tests/test_sql_query.py         | 24 ++++++++++++++++++++++++
 6 files changed, 63 insertions(+), 14 deletions(-)

diff --git a/edb/pgsql/parser/ast_builder.py b/edb/pgsql/parser/ast_builder.py
index b7688634497..1b62b424cc4 100644
--- a/edb/pgsql/parser/ast_builder.py
+++ b/edb/pgsql/parser/ast_builder.py
@@ -36,7 +36,7 @@
 
 from edb.pgsql import ast as pgast
 from edb.edgeql import ast as qlast
-from edb.pgsql.parser.exceptions import PSqlUnsupportedError
+from edb.pgsql.parser.exceptions import PSqlUnsupportedError, get_node_name
 
 
 @dataclasses.dataclass(kw_only=True)
@@ -134,7 +134,9 @@ def _enum(
         if outer_fallback:
             return None  # type: ignore
 
-        raise PSqlUnsupportedError(node, ", ".join(node.keys()))
+        raise PSqlUnsupportedError(
+            node, ", ".join(get_node_name(k) for k in node.keys())
+        )
     finally:
         ctx.has_fallback = outer_fallback
 
diff --git a/edb/pgsql/parser/exceptions.py b/edb/pgsql/parser/exceptions.py
index 207f485f577..82053a8f364 100644
--- a/edb/pgsql/parser/exceptions.py
+++ b/edb/pgsql/parser/exceptions.py
@@ -16,7 +16,7 @@
 # limitations under the License.
 #
 
-
+import re
 from typing import Any, Optional
 
 
@@ -34,9 +34,20 @@ class PSqlUnsupportedError(Exception):
     def __init__(self, node: Optional[Any] = None, feat: Optional[str] = None):
         self.node = node
         self.location = None
-        self.message = "unsupported SQL feature"
+        self.message = "not supported"
         if feat:
-            self.message += f" `{feat}`"
+            self.message += f": {feat}"
 
     def __str__(self):
         return self.message
+
+
+def get_node_name(name: str) -> str:
+    """
+    Given a node name (CreateTableStmt), this function tries to guess the SQL
+    command text (CREATE TABLE).
+    """
+
+    name = name.removesuffix('Stmt').removesuffix('Expr')
+    name = re.sub(r'(?<!^)(?=[A-Z])', ' ', name)
+    return name.upper()
diff --git a/edb/pgsql/resolver/command.py b/edb/pgsql/resolver/command.py
index b50f7cbb38a..e4c6fe1d1a9 100644
--- a/edb/pgsql/resolver/command.py
+++ b/edb/pgsql/resolver/command.py
@@ -220,11 +220,7 @@ def _uncompile_dml_stmt(stmt: pgast.DMLQuery, *, ctx: Context):
     - ptr-s are (usually) pointers on the subject.
     """
 
-    raise errors.QueryError(
-        f'{stmt.__class__.__name__} are not supported',
-        span=stmt.span,
-        pgext_code=pgerror.ERROR_FEATURE_NOT_SUPPORTED,
-    )
+    raise dispatch._raise_unsupported(stmt)
 
 
 def _uncompile_dml_subject(
diff --git a/edb/pgsql/resolver/dispatch.py b/edb/pgsql/resolver/dispatch.py
index 8ca24edbc18..877fe07d6ce 100644
--- a/edb/pgsql/resolver/dispatch.py
+++ b/edb/pgsql/resolver/dispatch.py
@@ -21,8 +21,11 @@
 
 import functools
 import typing
+import re
 
+from edb.server.pgcon import errors as pgerror
 from edb.pgsql import ast as pgast
+from edb import errors
 
 from . import context
 
@@ -34,7 +37,7 @@
 def _resolve(
     expr: pgast.Base, *, ctx: context.ResolverContextLevel
 ) -> pgast.Base:
-    raise ValueError(f'no SQL resolve handler for {expr.__class__}')
+    _raise_unsupported(expr)
 
 
 def resolve(expr: Base_T, *, ctx: context.ResolverContextLevel) -> Base_T:
@@ -85,7 +88,7 @@ def _resolve_relation(
     include_inherited: bool,
     ctx: context.ResolverContextLevel,
 ) -> typing.Tuple[pgast.BaseRelation, context.Table]:
-    raise ValueError(f'no SQL resolve handler for {rel.__class__}')
+    _raise_unsupported(rel)
 
 
 @_resolve.register
@@ -96,3 +99,16 @@ def _resolve_BaseRelation(
 
     rel, _ = resolve_relation(rel, ctx=ctx)
     return rel
+
+
+def _raise_unsupported(expr: pgast.Base) -> typing.Never:
+    pretty_name = expr.__class__.__name__
+    pretty_name = pretty_name.removesuffix('Stmt')
+    # title case to spaces
+    pretty_name = re.sub(r'(?<!^)(?=[A-Z])', ' ', pretty_name).upper()
+
+    raise errors.QueryError(
+        f'not supported: {pretty_name}',
+        span=expr.span,
+        pgext_code=pgerror.ERROR_FEATURE_NOT_SUPPORTED,
+    )
diff --git a/tests/test_sql_dml.py b/tests/test_sql_dml.py
index 66d2e9656ad..f93c731f450 100644
--- a/tests/test_sql_dml.py
+++ b/tests/test_sql_dml.py
@@ -1069,7 +1069,7 @@ async def test_sql_dml_delete_05(self):
         # delete where current of
         with self.assertRaisesRegex(
             asyncpg.FeatureNotSupportedError,
-            'unsupported SQL feature `CurrentOfExpr`',
+            'not supported: CURRENT OF',
         ):
             await self.scon.execute(
                 '''
@@ -1480,7 +1480,7 @@ async def test_sql_dml_update_05(self):
         # update where current of
         with self.assertRaisesRegex(
             asyncpg.FeatureNotSupportedError,
-            'unsupported SQL feature `CurrentOfExpr`',
+            'not supported: CURRENT OF',
         ):
             await self.scon.execute(
                 '''
diff --git a/tests/test_sql_query.py b/tests/test_sql_query.py
index 7ae60577a40..ecf1be70cfd 100644
--- a/tests/test_sql_query.py
+++ b/tests/test_sql_query.py
@@ -2135,6 +2135,30 @@ async def test_sql_query_access_policy_04(self):
 
         await tran.rollback()
 
+    async def test_sql_query_unsupported_01(self):
+        # test error messages of unsupported queries
+
+        # we build AST for this not, but throw in resolver
+        with self.assertRaisesRegex(
+            asyncpg.FeatureNotSupportedError,
+            "not supported: CREATE",
+            position="14",  # TODO: this is confusing
+        ):
+            await self.squery_values('CREATE TABLE a();')
+
+        # we don't even have AST node for this
+        with self.assertRaisesRegex(
+            asyncpg.FeatureNotSupportedError,
+            "not supported: ALTER TABLE",
+        ):
+            await self.squery_values('ALTER TABLE a ADD COLUMN b INT;')
+
+        with self.assertRaisesRegex(
+            asyncpg.FeatureNotSupportedError,
+            "not supported: REINDEX",
+        ):
+            await self.squery_values('REINDEX TABLE a;')
+
     async def test_native_sql_query_00(self):
         await self.assert_sql_query_result(
             """

From c78a6366dc00ac5c8bb9ac68d4a247d33789f034 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?= <aljaz@edgedb.com>
Date: Tue, 19 Nov 2024 13:40:52 +0100
Subject: [PATCH 2/2] fix

---
 edb/pgsql/resolver/dispatch.py | 1 +
 edb/pgsql/resolver/static.py   | 5 ++++-
 edb/server/compiler/sql.py     | 5 ++---
 tests/test_sql_query.py        | 2 +-
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/edb/pgsql/resolver/dispatch.py b/edb/pgsql/resolver/dispatch.py
index 877fe07d6ce..d3ec583fcf5 100644
--- a/edb/pgsql/resolver/dispatch.py
+++ b/edb/pgsql/resolver/dispatch.py
@@ -37,6 +37,7 @@
 def _resolve(
     expr: pgast.Base, *, ctx: context.ResolverContextLevel
 ) -> pgast.Base:
+    expr.dump()
     _raise_unsupported(expr)
 
 
diff --git a/edb/pgsql/resolver/static.py b/edb/pgsql/resolver/static.py
index e221bc52c0f..ebfe711951e 100644
--- a/edb/pgsql/resolver/static.py
+++ b/edb/pgsql/resolver/static.py
@@ -312,7 +312,9 @@ def eval_FuncCall(
                     return value
 
         raise errors.QueryError(
-            "function set_config is not supported", span=expr.span
+            "function set_config is not supported",
+            span=expr.span,
+            pgext_code=pgerror.ERROR_FEATURE_NOT_SUPPORTED,
         )
 
     if fn_name == 'current_setting':
@@ -329,6 +331,7 @@ def eval_FuncCall(
         raise errors.QueryError(
             f"function pg_catalog.{fn_name} is not supported",
             span=expr.span,
+            pgext_code=pgerror.ERROR_FEATURE_NOT_SUPPORTED,
         )
 
     if fn_name == "pg_get_serial_sequence":
diff --git a/edb/server/compiler/sql.py b/edb/server/compiler/sql.py
index 9597a6b7a9e..0371874be53 100644
--- a/edb/server/compiler/sql.py
+++ b/edb/server/compiler/sql.py
@@ -289,9 +289,8 @@ def compile_sql(
             else:
                 unit.cardinality = enums.Cardinality.MANY
         else:
-            raise errors.UnsupportedFeatureError(
-                f"SQL {stmt.__class__.__name__} is not supported"
-            )
+            from edb.pgsql import resolver as pg_resolver
+            pg_resolver.dispatch._raise_unsupported(stmt)
 
         unit.stmt_name = compute_stmt_name(unit.query, tx_state).encode("utf-8")
 
diff --git a/tests/test_sql_query.py b/tests/test_sql_query.py
index ecf1be70cfd..0049da79857 100644
--- a/tests/test_sql_query.py
+++ b/tests/test_sql_query.py
@@ -2142,7 +2142,7 @@ async def test_sql_query_unsupported_01(self):
         with self.assertRaisesRegex(
             asyncpg.FeatureNotSupportedError,
             "not supported: CREATE",
-            position="14",  # TODO: this is confusing
+            # position="14",  # TODO: this is confusing
         ):
             await self.squery_values('CREATE TABLE a();')