From 41afb84bfd56e7947b1227bb4b4b22280a9d1db1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alja=C5=BE=20Mur=20Er=C5=BEen?=
 <aljazerzen@users.noreply.github.com>
Date: Thu, 21 Nov 2024 14:13:02 +0100
Subject: [PATCH] Improve unsupported error message over SQL adapter (#7995)

---
 edb/pgsql/parser/ast_builder.py |  6 ++++--
 edb/pgsql/parser/exceptions.py  | 17 ++++++++++++++---
 edb/pgsql/resolver/command.py   |  6 +-----
 edb/pgsql/resolver/dispatch.py  | 21 +++++++++++++++++++--
 edb/pgsql/resolver/static.py    |  5 ++++-
 edb/server/compiler/sql.py      |  5 ++---
 tests/test_sql_dml.py           |  4 ++--
 tests/test_sql_query.py         | 24 ++++++++++++++++++++++++
 8 files changed, 70 insertions(+), 18 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..d3ec583fcf5 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,8 @@
 def _resolve(
     expr: pgast.Base, *, ctx: context.ResolverContextLevel
 ) -> pgast.Base:
-    raise ValueError(f'no SQL resolve handler for {expr.__class__}')
+    expr.dump()
+    _raise_unsupported(expr)
 
 
 def resolve(expr: Base_T, *, ctx: context.ResolverContextLevel) -> Base_T:
@@ -85,7 +89,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 +100,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/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_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 5fc5759828c..a83757ed22e 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(
             """