Skip to content

Commit

Permalink
Improve unsupported error message over SQL adapter (#7995)
Browse files Browse the repository at this point in the history
  • Loading branch information
aljazerzen authored Nov 21, 2024
1 parent d3a1839 commit 41afb84
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 18 deletions.
6 changes: 4 additions & 2 deletions edb/pgsql/parser/ast_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
17 changes: 14 additions & 3 deletions edb/pgsql/parser/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# limitations under the License.
#


import re
from typing import Any, Optional


Expand All @@ -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()
6 changes: 1 addition & 5 deletions edb/pgsql/resolver/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
21 changes: 19 additions & 2 deletions edb/pgsql/resolver/dispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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,
)
5 changes: 4 additions & 1 deletion edb/pgsql/resolver/static.py
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand All @@ -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":
Expand Down
5 changes: 2 additions & 3 deletions edb/server/compiler/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
4 changes: 2 additions & 2 deletions tests/test_sql_dml.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
'''
Expand Down Expand Up @@ -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(
'''
Expand Down
24 changes: 24 additions & 0 deletions tests/test_sql_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
"""
Expand Down

0 comments on commit 41afb84

Please sign in to comment.