Skip to content

Commit

Permalink
Support linkprops on backlinks (#3841)
Browse files Browse the repository at this point in the history
There are some hacks in this to make it cherry-pickable which can be
removed later.

Fixes #2663
  • Loading branch information
msullivan committed May 10, 2022
1 parent aa1c102 commit f06bec5
Show file tree
Hide file tree
Showing 12 changed files with 222 additions and 29 deletions.
2 changes: 2 additions & 0 deletions edb/edgeql/compiler/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def __init__(
ptrcls_is_alias: bool = False,
rptr: Optional[irast.Pointer] = None,
exprtype: s_types.ExprType = s_types.ExprType.Select,
rptr_dir: Optional[s_pointers.PointerDirection] = None,
) -> None:
self.source = source
self.ptrcls = ptrcls
Expand All @@ -90,6 +91,7 @@ def __init__(
self.ptrcls_is_alias = ptrcls_is_alias
self.rptr = rptr
self.exprtype = exprtype
self.rptr_dir = rptr_dir


@dataclasses.dataclass
Expand Down
2 changes: 1 addition & 1 deletion edb/edgeql/compiler/inference/cardinality.py
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ def _infer_set_inner(
card = cartesian_cardinality([source_card, rptr_spec_card])

else:
if rptrref.union_components:
if rptrref.union_components and not rptrref.is_computed_backlink:
# We use cartesian cardinality instead of union cardinality
# because the union of pointers in this context is disjoint
# in a sense that for any specific source only a given union
Expand Down
14 changes: 14 additions & 0 deletions edb/edgeql/compiler/schemactx.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ def derive_ptr(
derived_name_quals: Optional[Sequence[str]] = (),
preserve_shape: bool = False,
preserve_path_id: bool = False,
derive_backlink: bool = False,
inheritance_merge: bool = True,
attrs: Optional[Dict[str, Any]] = None,
ctx: context.ContextLevel,
Expand All @@ -289,6 +290,11 @@ def derive_ptr(
if ptr.get_name(ctx.env.schema) == derived_name:
qualifiers = qualifiers + (ctx.aliases.get('d'),)

if derive_backlink:
attrs = attrs.copy() if attrs else {}
attrs['union_of'] = [ptr]
attrs['intersection_of'] = [ptr]

ctx.env.schema, derived = ptr.derive_ref(
ctx.env.schema,
source,
Expand All @@ -302,6 +308,14 @@ def derive_ptr(
preserve_path_id=preserve_path_id,
attrs=attrs)

# Delete the bogus parents from a derived computed backlink.
if derive_backlink:
link = [ctx.env.schema.get('std::link')]
ctx.env.schema = derived.set_field_value(
ctx.env.schema, 'bases', link)
ctx.env.schema = derived.set_field_value(
ctx.env.schema, 'ancestors', link)

if not ptr.generic(ctx.env.schema):
if isinstance(derived, s_sources.Source):
ptr = cast(s_links.Link, ptr)
Expand Down
4 changes: 1 addition & 3 deletions edb/edgeql/compiler/setgen.py
Original file line number Diff line number Diff line change
Expand Up @@ -1196,11 +1196,9 @@ def fixup_computable_source_set(
source_rptrref = source_set.rptr.ptrref
if source_rptrref.base_ptr is not None:
source_rptrref = source_rptrref.base_ptr
source_set.rptr = irast.Pointer(
source=source_set.rptr.source,
source_set.rptr = source_set.rptr.replace(
target=source_set,
ptrref=source_rptrref,
direction=source_set.rptr.direction,
is_definition=True,
)
return source_set
Expand Down
23 changes: 21 additions & 2 deletions edb/edgeql/compiler/stmt.py
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,6 @@ def compile_query_subject(
and view_rptr.ptrcls is None
and view_rptr.ptrcls_name is not None
and expr_rptr is not None
and expr_rptr.direction is s_pointers.PointerDirection.Outbound
and expr_rptr.source.rptr is None
and (
view_rptr.source.get_bases(ctx.env.schema).first(ctx.env.schema).id
Expand All @@ -1143,10 +1142,30 @@ def compile_query_subject(
# the parent shape, ie. Spam { alias := Spam.bar }, so
# `Spam.alias` should be a subclass of `Spam.bar` inheriting
# its properties.
base_ptrcls = typegen.ptrcls_from_ptrref(expr_rptr.ptrref, ctx=ctx)
#
# We also try to detect reverse aliases like `.<bar[IS Spam]`
# and arange to inherit the linkprops from those if it resolves
# to a unique type.
ptrref = expr_rptr.ptrref
if (
expr.rptr
and isinstance(expr.rptr.ptrref, irast.TypeIntersectionPointerRef)
and len(expr.rptr.ptrref.rptr_specialization) == 1
):
ptrref = list(expr.rptr.ptrref.rptr_specialization)[0]

if (
expr_rptr.direction is not s_pointers.PointerDirection.Outbound
and ptrref.out_source.is_opaque_union
):
base_ptrcls = None
else:
base_ptrcls = typegen.ptrcls_from_ptrref(ptrref, ctx=ctx)

if isinstance(base_ptrcls, s_pointers.Pointer):
view_rptr.base_ptrcls = base_ptrcls
view_rptr.ptrcls_is_alias = True
view_rptr.rptr_dir = expr_rptr.direction

if (
(
Expand Down
28 changes: 28 additions & 0 deletions edb/edgeql/compiler/viewgen.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,16 @@ def _process_view(
ctx=ctx,
)

# HACK?: when we see linkprops being used on an intersection,
# attach the flattened source path to make linkprops on
# computed backlinks work
if (
isinstance(path_id.rptr(), irast.TypeIntersectionPointerRef)
and ptrcls.is_link_property(ctx.env.schema)
):
ctx.path_scope.attach_path(
path_id, flatten_intersection=True, context=None)

set_shape.append((ptr_set, shape_op))

ir_set.shape = tuple(set_shape)
Expand Down Expand Up @@ -741,9 +751,12 @@ def _normalize_view_ptr_expr(

irexpr.context = compexpr.context

is_inbound_alias = False
if base_ptrcls is None:
base_ptrcls = sub_view_rptr.base_ptrcls
base_ptrcls_is_alias = sub_view_rptr.ptrcls_is_alias
is_inbound_alias = (
sub_view_rptr.rptr_dir is s_pointers.PointerDirection.Inbound)

if ptrcls is not None:
ctx.env.schema = ptrcls.set_field_value(
Expand Down Expand Up @@ -898,6 +911,7 @@ def _normalize_view_ptr_expr(
else:
ptrcls = schemactx.derive_ptr(
derive_from, src_scls, ptr_target,
derive_backlink=is_inbound_alias,
derived_name=derived_name,
ctx=ctx)

Expand Down Expand Up @@ -1092,9 +1106,12 @@ def derive_ptrcls(
attrs['path_id_name'] = view_rptr.base_ptrcls.get_name(
ctx.env.schema)

is_inbound_alias = (
view_rptr.rptr_dir is s_pointers.PointerDirection.Inbound)
view_rptr.ptrcls = schemactx.derive_ptr(
view_rptr.base_ptrcls, view_rptr.source, target_scls,
derived_name=derived_name,
derive_backlink=is_inbound_alias,
attrs=attrs,
ctx=ctx
)
Expand Down Expand Up @@ -1485,6 +1502,17 @@ def _late_compile_view_shapes_in_set(
ctx=ctx,
)

# HACK?: when we see linkprops being used on an intersection,
# attach the flattened source path to make linkprops on
# computed backlinks work
if (
isinstance(
path_tip.path_id.rptr(), irast.TypeIntersectionPointerRef)
and ptr.is_link_property(ctx.env.schema)
):
ctx.path_scope.attach_path(
path_tip.path_id, flatten_intersection=True, context=None)

element_scope = pathctx.get_set_scope(element, ctx=ctx)

if element_scope is None:
Expand Down
12 changes: 12 additions & 0 deletions edb/ir/ast.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,18 @@ def descendants(self) -> typing.Set[BasePointerRef]:
res.update(child.descendants())
return res

@property
def real_material_ptr(self) -> BasePointerRef:
return self.material_ptr or self

@property
def real_base_ptr(self) -> BasePointerRef:
return self.base_ptr or self

@property
def is_computed_backlink(self) -> bool:
return bool(self.intersection_components and self.union_components)

def __repr__(self) -> str:
return f'<ir.{type(self).__name__} \'{self.name}\' at 0x{id(self):x}>'

Expand Down
3 changes: 2 additions & 1 deletion edb/ir/scopetree.py
Original file line number Diff line number Diff line change
Expand Up @@ -411,13 +411,14 @@ def attach_path(
self,
path_id: pathid.PathId,
*,
flatten_intersection: bool=False,
optional: bool=False,
context: Optional[pctx.ParserContext],
) -> List[ScopeTreeNode]:
"""Attach a scope subtree representing *path_id*."""

subtree = parent = ScopeTreeNode(fenced=True)
is_lprop = False
is_lprop = flatten_intersection
fences = []

for prefix in reversed(list(path_id.iter_prefixes(include_ptr=True))):
Expand Down
5 changes: 4 additions & 1 deletion edb/pgsql/compiler/relctx.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,10 @@ def _new_mapped_pointer_rvar(
nullable=not ptrref.required)

# Set up references according to the link direction.
if ir_ptr.direction == s_pointers.PointerDirection.Inbound:
if (
ir_ptr.direction == s_pointers.PointerDirection.Inbound
or ptrref.is_computed_backlink
):
near_ref = target_ref
far_ref = source_ref
else:
Expand Down
16 changes: 15 additions & 1 deletion edb/schema/pointers.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,13 @@ def _merge_types(
# When two pointers are merged, check target compatibility
# and return a target that satisfies both specified targets.

if (isinstance(t1, s_abc.ScalarType) !=
# This is a hack: when deriving computed backlink pointers, we
# (ab)use inheritance to populate them with the appropriate
# linkprops. We fix it up to remove the bogus parents later.
if ptr.is_computed_backlink(schema):
return schema, t2

elif (isinstance(t1, s_abc.ScalarType) !=
isinstance(t2, s_abc.ScalarType)):
# Mixing a property with a link.
vnp = ptr.get_verbosename(schema, with_parent=True)
Expand Down Expand Up @@ -490,6 +496,14 @@ def is_type_intersection(self) -> bool:
def is_generated(self, schema: s_schema.Schema) -> bool:
return bool(self.get_from_alias(schema))

def is_computed_backlink(self, schema: s_schema.Schema) -> bool:
# HACK: To avoid needing a schema change for a change we want to
# cherry-pick, we indicate that a pointer is a computed backlink
# by making it both a union and an intersection.
return bool(
self.get_union_of(schema) and self.get_intersection_of(schema)
)

@classmethod
def get_displayname_static(cls, name: sn.Name) -> str:
sn = cls.get_shortname_static(name)
Expand Down
122 changes: 122 additions & 0 deletions tests/test_edgeql_linkprops.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@

import os.path

import edgedb

from edb.testbase import server as tb
from edb.tools import test


class TestEdgeQLLinkproperties(tb.QueryTestCase):
Expand Down Expand Up @@ -1161,3 +1164,122 @@ async def test_edgeql_props_link_union_03(self):
''',
["test"],
)

async def test_edgeql_props_back_01(self):
await self.assert_query_result(
"""
with X1 := (Card { z := (.<deck[IS User], .<deck[IS User]@count)}),
X2 := X1 { owners2 := assert_distinct(
.z.0 { count := X1.z.1 }) },
select X2 { name, owners2: {name, count} order BY .name }
filter .name = 'Dwarf';
""",
[
{
"name": "Dwarf",
"owners2": [
{"count": 3, "name": "Bob"},
{"count": 4, "name": "Carol"}
]
}
],
)

async def test_edgeql_props_back_02(self):
await self.assert_query_result(
r'''
select Card { name, z := .<deck[IS User] { name, @count }}
filter .name = 'Dragon';
''',
[
{
"name": "Dragon",
"z": tb.bag([
{"@count": 2, "name": "Alice"},
{"@count": 1, "name": "Dave"},
])
}
]
)

async def test_edgeql_props_back_03(self):
await self.assert_query_result(
r'''
select Card { name, z := .<deck[IS User] { name, x := @count }}
filter .name = 'Dragon';
''',
[
{
"name": "Dragon",
"z": tb.bag([
{"x": 2, "name": "Alice"},
{"x": 1, "name": "Dave"},
])
}
]
)

async def test_edgeql_props_back_04(self):
await self.assert_query_result(
r'''
select assert_exists((
select Card { name, z := .<deck[IS User] { name, @count }}
filter .name = 'Dragon'
));
''',
[
{
"name": "Dragon",
"z": tb.bag([
{"@count": 2, "name": "Alice"},
{"@count": 1, "name": "Dave"},
])
}
]
)

async def test_edgeql_props_back_05(self):
await self.assert_query_result(
r'''
select assert_exists((
select Card { name, z := .<deck[IS User] { name, x := @count }}
filter .name = 'Dragon'
));
''',
[
{
"name": "Dragon",
"z": tb.bag([
{"x": 2, "name": "Alice"},
{"x": 1, "name": "Dave"},
])
}
]
)

async def test_edgeql_props_back_06(self):
# This should not work
with self.assertRaisesRegex(
edgedb.QueryError,
r"has no property 'count'"):

await self.con.query(
r'''
select Card { name, z := .<deck { @count }}
filter .name = 'Dragon'
'''
)

@test.xfail('We are too permissive with intersections on supertypes')
async def test_edgeql_props_back_07(self):
# This should not work
with self.assertRaisesRegex(
edgedb.QueryError,
r"has no property 'count'"):

await self.con.query(
r'''
select Card { name, z := .<deck[IS Object] { @count }}
filter .name = 'Dragon'
'''
)

0 comments on commit f06bec5

Please sign in to comment.