Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve unsupported error message over SQL adapter #7995

Merged
merged 2 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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