Skip to content

Commit

Permalink
Improve error message when creating union with Incompatible types. (#…
Browse files Browse the repository at this point in the history
…7278)

Given a schema:
```
type X {
  property a -> str;
}
type Y {
  property a -> int64;
}
type Z {
  link xy -> X | Y;
}
```

Produces the error:
```
error: cannot create union (default::X | default::Y) with property 'a' using incompatible types std::int64, std::str
10 │   link xy -> X | Y;
   │              ^^^^^ error
```

Similar errors are produced for:
- `select X union Y`
- `select Object is X | Y`
  • Loading branch information
dnwpark authored May 1, 2024
1 parent 6373d6b commit 66b1991
Show file tree
Hide file tree
Showing 8 changed files with 220 additions and 20 deletions.
6 changes: 5 additions & 1 deletion edb/edgeql/compiler/func.py
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,11 @@ def compile_operator(
left_type = setgen.get_set_type(larg, ctx=ctx)
right_type = setgen.get_set_type(rarg, ctx=ctx)
rtype = schemactx.get_union_type(
[left_type, right_type], preserve_derived=True, ctx=ctx)
[left_type, right_type],
preserve_derived=True,
ctx=ctx,
span=qlexpr.span
)

from_op = oper.get_from_operator(env.schema)
sql_operator = None
Expand Down
21 changes: 18 additions & 3 deletions edb/edgeql/compiler/schemactx.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ def get_union_type(
opaque: bool = False,
preserve_derived: bool = False,
ctx: context.ContextLevel,
span: Optional[parsing.Span] = None,
) -> s_types.TypeT:

targets: Sequence[s_types.Type]
Expand All @@ -413,9 +414,23 @@ def get_union_type(
ctx.env.schema, types
)

ctx.env.schema, union, created = s_utils.ensure_union_type(
ctx.env.schema, targets,
opaque=opaque, transient=True)
try:
ctx.env.schema, union, created = s_utils.ensure_union_type(
ctx.env.schema, targets,
opaque=opaque, transient=True)
except errors.SchemaError as e:
union_name = (
'(' + ' | '.join(sorted(
t.get_displayname(ctx.env.schema)
for t in types
)) + ')'
)
e.args = (
(f'cannot create union {union_name} {e.args[0]}',)
+ e.args[1:]
)
e.set_span(span)
raise e

if created:
ctx.env.created_schema_objects.add(union)
Expand Down
2 changes: 1 addition & 1 deletion edb/edgeql/compiler/typegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def ql_typeexpr_to_type(

types = _ql_typeexpr_to_type(ql_t, ctx=ctx)
if len(types) > 1:
return schemactx.get_union_type(types, ctx=ctx)
return schemactx.get_union_type(types, ctx=ctx, span=ql_t.span)
else:
return types[0]

Expand Down
28 changes: 20 additions & 8 deletions edb/schema/sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
)


from edb import errors

from . import delta as sd
from . import indexes
from . import name as sn
Expand Down Expand Up @@ -243,14 +245,24 @@ def populate_pointer_set_for_source_union(
if len(ptrs) == 1:
ptr = ptrs[0]
else:
schema, ptr = s_pointers.get_or_create_union_pointer(
schema,
ptrname=pn,
source=union,
direction=s_pointers.PointerDirection.Outbound,
components=set(ptrs),
modname=modname,
)
try:
schema, ptr = s_pointers.get_or_create_union_pointer(
schema,
ptrname=pn,
source=union,
direction=s_pointers.PointerDirection.Outbound,
components=set(ptrs),
modname=modname,
)
except errors.SchemaError as e:
# ptrs may have different verbose names
# ensure the same one is always chosen
vn = sorted(p.get_verbosename(schema) for p in ptrs)[0]
e.args = (
(f'with {vn} {e.args[0]}',)
+ e.args[1:]
)
raise e

union_pointers[pn] = ptr

Expand Down
27 changes: 21 additions & 6 deletions edb/schema/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,7 @@ def as_create_delta(
cmd.set_attribute_value('name', self.name)
cmd.set_attribute_value('components', tuple(self.components))
cmd.set_attribute_value('is_opaque_union', self.opaque)
cmd.set_attribute_value('span', self.sourcectx)
return cmd

def __repr__(self) -> str:
Expand Down Expand Up @@ -884,12 +885,26 @@ def apply(
for c in self.get_attribute_value('components')
]

new_schema, union_type, created = utils.ensure_union_type(
schema,
components,
opaque=self.get_attribute_value('is_opaque_union') or False,
module=self.classname.module,
)
try:
new_schema, union_type, created = utils.ensure_union_type(
schema,
components,
opaque=self.get_attribute_value('is_opaque_union') or False,
module=self.classname.module,
)
except errors.SchemaError as e:
union_name = (
'(' + ' | '.join(sorted(
c.get_displayname(schema)
for c in components
)) + ')'
)
e.args = (
(f'cannot create union {union_name} {e.args[0]}',)
+ e.args[1:]
)
e.set_span(self.get_attribute_value('span'))
raise e

if created:
delta = union_type.as_create_delta(
Expand Down
6 changes: 5 additions & 1 deletion edb/schema/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,25 +451,29 @@ def type_op_ast_to_type_shell(
components=left.components + right.components,
module=module,
schemaclass=metaclass,
sourcectx=node.span,
)
else:
return s_types.UnionTypeShell(
components=left.components + (right,),
module=module,
schemaclass=metaclass,
sourcectx=node.span,
)
else:
if isinstance(right, s_types.UnionTypeShell):
return s_types.UnionTypeShell(
components=(left,) + right.components,
schemaclass=metaclass,
module=module,
sourcectx=node.span,
)
else:
return s_types.UnionTypeShell(
components=(left, right),
module=module,
schemaclass=metaclass,
sourcectx=node.span,
)


Expand Down Expand Up @@ -1171,7 +1175,7 @@ def _union_error(
schema: s_schema.Schema, components: Iterable[s_types.Type]
) -> errors.SchemaError:
names = ', '.join(sorted(c.get_displayname(schema) for c in components))
return errors.SchemaError(f'cannot create a union of {names}')
return errors.SchemaError(f'using incompatible types {names}')


def ensure_intersection_type(
Expand Down
90 changes: 90 additions & 0 deletions tests/test_edgeql_select.py
Original file line number Diff line number Diff line change
Expand Up @@ -7072,6 +7072,27 @@ async def test_edgeql_select_is_13(self):
[True],
)

async def test_edgeql_select_is_incompatible_union_01(self):
await self.con.execute('''
CREATE TYPE Dummy1 {
CREATE PROPERTY foo -> int64;
};
CREATE TYPE Dummy2 {
CREATE PROPERTY foo -> str;
};
''')

with self.assertRaisesRegex(
edgedb.SchemaError,
r"cannot create union \(default::Dummy1 \| default::Dummy2\) "
r"with property 'foo' using incompatible types std::int64, "
r"std::str"):
await self.con.query(
r'''
SELECT Object is Dummy1 | Dummy2;
''',
)

async def test_edgeql_select_duplicate_definition_01(self):
with self.assertRaisesRegex(
edgedb.QueryError,
Expand Down Expand Up @@ -7532,6 +7553,75 @@ async def test_edgeql_select_reverse_overload_03(self):
''',
)

async def test_edgeql_select_incompatible_union_01(self):
await self.con.execute('''
CREATE TYPE Dummy1 {
CREATE PROPERTY foo -> int64;
};
CREATE TYPE Dummy2 {
CREATE PROPERTY foo -> str;
};
''')

with self.assertRaisesRegex(
edgedb.SchemaError,
r"cannot create union \(default::Dummy1 \| default::Dummy2\) "
r"with property 'foo' using incompatible types std::int64, "
r"std::str"):
await self.con.query(
r'''
SELECT Dummy1 union Dummy2;
''',
)

async def test_edgeql_select_incompatible_union_02(self):
await self.con.execute('''
CREATE TYPE Bar;
CREATE TYPE Dummy1 {
CREATE PROPERTY foo -> int64;
};
CREATE TYPE Dummy2 {
CREATE LINK foo -> Bar;
};
''')

with self.assertRaisesRegex(
edgedb.SchemaError,
r"cannot create union \(default::Dummy1 \| default::Dummy2\) "
r"with link 'foo' using incompatible types default::Bar, "
r"std::int64"):
await self.con.query(
r'''
SELECT Dummy1 union Dummy2;
''',
)

async def test_edgeql_select_incompatible_union_03(self):
await self.con.execute('''
CREATE TYPE Bar;
CREATE TYPE Dummy1 {
CREATE LINK foo -> Bar {
CREATE PROPERTY baz -> int64
}
};
CREATE TYPE Dummy2 {
CREATE LINK foo -> Bar {
CREATE PROPERTY baz -> str
}
};
''')

with self.assertRaisesRegex(
edgedb.SchemaError,
r"cannot create union \(default::Dummy1 \| default::Dummy2\) "
r"with link 'foo' with property 'baz' using incompatible types "
r"std::int64, std::str"):
await self.con.query(
r'''
SELECT Dummy1 union Dummy2;
''',
)

async def test_edgeql_function_source_01a(self):
# TODO: I think we might want to eliminate this sort of shape
# propagation out of array_unpack instead?
Expand Down
60 changes: 60 additions & 0 deletions tests/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -2619,6 +2619,66 @@ def test_schema_index_non_singleton_01(self):
}
"""

@tb.must_fail(
errors.SchemaError,
"cannot create union \\(test::X | test::Y\\) with property 'a' using "
"incompatible types std::int64, std::str",
)
def test_schema_incompatible_union_01(self):
"""
type X {
property a -> int64;
}
type Y {
property a -> str;
}
type Z {
link xy -> X | Y;
}
"""

@tb.must_fail(
errors.SchemaError,
"cannot create union \\(test::X | test::Y\\) with link 'a' using "
"incompatible types std::int64, test::A",
)
def test_schema_incompatible_union_02(self):
"""
type A;
type X {
property a -> int64;
}
type Y {
link a -> A;
}
type Z {
link xy -> X | Y;
}
"""

@tb.must_fail(
errors.SchemaError,
"cannot create union \\(test::X | test::Y\\) with link 'a' with "
"property 'b' using incompatible types std::int64, std::str",
)
def test_schema_incompatible_union_03(self):
"""
type A;
type X {
link a -> A {
b -> int64
};
}
type Y {
link a -> A {
b -> str
}
}
type Z {
link xy -> X | Y;
}
"""


class TestGetMigration(tb.BaseSchemaLoadTest):
"""Test migration deparse consistency.
Expand Down

0 comments on commit 66b1991

Please sign in to comment.