mirror of
https://github.com/maxkratz/edgedb.git
synced 2024-09-16 18:59:05 +00:00
Get rid of computeds subtyping other pointers in same object (#6568)
Currently if we have something like:
```
type T;
type S {
s: T;
s2 := .s;
};
```
`s2` will actually be derived from *`s`*. The point of this is that that if `s` has linkprops, they get inherited by `s2`, but this implementation strategy has caused a *lot* of bugs.
Ditch that and basically use the same approach we use for getting linkprops on backlinks
Fixes #6292
This commit is contained in:
Michael J. Sullivan
GitHub
parent
f0b262e1f5
commit
4f4afbcc37
8 changed files with 100 additions and 101 deletions
|
|
@ -44,7 +44,7 @@ from edb.common import verutils
|
|||
|
||||
|
||||
# Increment this whenever the database layout or stdlib changes.
|
||||
EDGEDB_CATALOG_VERSION = 2023_10_25_00_00
|
||||
EDGEDB_CATALOG_VERSION = 2023_12_05_00_00
|
||||
EDGEDB_MAJOR_VERSION = 5
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -309,7 +309,8 @@ def derive_ptr(
|
|||
# actually deriving from it.
|
||||
if derive_backlink:
|
||||
attrs = attrs.copy() if attrs else {}
|
||||
attrs['computed_backlink'] = ptr
|
||||
attrs['computed_link_alias'] = ptr
|
||||
attrs['computed_link_alias_is_backward'] = True
|
||||
ptr = ctx.env.schema.get('std::link', type=s_pointers.Pointer)
|
||||
|
||||
ctx.env.schema, derived = ptr.derive_ref(
|
||||
|
|
|
|||
|
|
@ -756,14 +756,15 @@ def resolve_ptr_with_intersections(
|
|||
s_name.UnqualName(pointer_name),
|
||||
)
|
||||
|
||||
# If we couldn't anything, but the source is a computed backlink,
|
||||
# look for a link property on the reverse side of it. This allows
|
||||
# us to access link properties in both directions on links, including
|
||||
# when the backlink has been stuck in a computed.
|
||||
# If we couldn't anything, but the source is a computed link
|
||||
# that aliases some other link, look for a link property on
|
||||
# it. This allows us to access link properties in both
|
||||
# directions on links, including when the backlink has been
|
||||
# stuck in a computed.
|
||||
if (
|
||||
ptr is None
|
||||
and isinstance(near_endpoint, s_pointers.Pointer)
|
||||
and (back := near_endpoint.get_computed_backlink(ctx.env.schema))
|
||||
and isinstance(near_endpoint, s_links.Link)
|
||||
and (back := near_endpoint.get_computed_link_alias(ctx.env.schema))
|
||||
and isinstance(back, s_links.Link)
|
||||
and (nptr := back.maybe_get_ptr(
|
||||
ctx.env.schema,
|
||||
|
|
@ -773,8 +774,24 @@ def resolve_ptr_with_intersections(
|
|||
# around a bunch of stuff inside them.
|
||||
and not nptr.is_pure_computable(ctx.env.schema)
|
||||
):
|
||||
ptr = schemactx.derive_ptr(nptr, near_endpoint, ctx=ctx)
|
||||
path_id_ptr = ptr
|
||||
src_type = downcast(
|
||||
s_types.Type, near_endpoint.get_source(ctx.env.schema)
|
||||
)
|
||||
if not src_type.is_view(ctx.env.schema):
|
||||
# HACK: If the source is in the standard library, and
|
||||
# not a view, we can't add a derived pointer. For
|
||||
# consistency, just always require it be a view.
|
||||
new_source = downcast(
|
||||
s_objtypes.ObjectType,
|
||||
schemactx.derive_view(src_type, ctx=ctx),
|
||||
)
|
||||
new_endpoint = downcast(s_links.Link, schemactx.derive_ptr(
|
||||
near_endpoint, new_source, ctx=ctx))
|
||||
else:
|
||||
new_endpoint = near_endpoint
|
||||
|
||||
ptr = schemactx.derive_ptr(nptr, new_endpoint, ctx=ctx)
|
||||
path_id_ptr = nptr
|
||||
|
||||
if ptr is not None:
|
||||
ref = ptr.get_nearest_non_derived_parent(ctx.env.schema)
|
||||
|
|
|
|||
|
|
@ -248,7 +248,8 @@ class BasePointerRef(ImmutableBase):
|
|||
# Inbound cardinality of the pointer.
|
||||
in_cardinality: qltypes.Cardinality = qltypes.Cardinality.MANY
|
||||
defined_here: bool = False
|
||||
computed_backlink: typing.Optional[BasePointerRef] = None
|
||||
computed_link_alias: typing.Optional[BasePointerRef] = None
|
||||
computed_link_alias_is_backward: typing.Optional[bool] = None
|
||||
|
||||
def dir_target(self, direction: s_pointers.PointerDirection) -> TypeRef:
|
||||
if direction is s_pointers.PointerDirection.Outbound:
|
||||
|
|
|
|||
|
|
@ -583,12 +583,14 @@ def ptrref_from_ptrcls(
|
|||
ircls = irast.PointerRef
|
||||
kwargs['id'] = ptrcls.id
|
||||
kwargs['defined_here'] = ptrcls.get_defined_here(schema)
|
||||
if backlink := ptrcls.get_computed_backlink(schema):
|
||||
if backlink := ptrcls.get_computed_link_alias(schema):
|
||||
assert isinstance(backlink, s_pointers.Pointer)
|
||||
kwargs['computed_backlink'] = ptrref_from_ptrcls(
|
||||
kwargs['computed_link_alias'] = ptrref_from_ptrcls(
|
||||
ptrcls=backlink, schema=schema,
|
||||
cache=cache, typeref_cache=typeref_cache,
|
||||
)
|
||||
kwargs['computed_link_alias_is_backward'] = (
|
||||
ptrcls.get_computed_link_alias_is_backward(schema))
|
||||
|
||||
else:
|
||||
raise AssertionError(f'unexpected pointer class: {ptrcls}')
|
||||
|
|
|
|||
|
|
@ -621,10 +621,9 @@ def _new_mapped_pointer_rvar(
|
|||
name=[tgt_col],
|
||||
nullable=not ptrref.required)
|
||||
|
||||
# Set up references according to the link direction.
|
||||
if (
|
||||
ir_ptr.direction == s_pointers.PointerDirection.Inbound
|
||||
or ptrref.computed_backlink
|
||||
or ptrref.computed_link_alias_is_backward
|
||||
):
|
||||
near_ref = target_ref
|
||||
far_ref = source_ref
|
||||
|
|
@ -1854,8 +1853,8 @@ def range_for_ptrref(
|
|||
# needs to appear in *all* of the tables, so we just pick any
|
||||
# one of them.
|
||||
refs = {next(iter((ptrref.intersection_components)))}
|
||||
elif ptrref.computed_backlink:
|
||||
refs = {ptrref.computed_backlink}
|
||||
elif ptrref.computed_link_alias:
|
||||
refs = {ptrref.computed_link_alias}
|
||||
else:
|
||||
refs = {ptrref}
|
||||
assert isinstance(ptrref, irast.PointerRef), \
|
||||
|
|
|
|||
|
|
@ -535,7 +535,12 @@ class Pointer(referencing.NamedReferencedInheritingObject,
|
|||
type_is_generic_self=True,
|
||||
)
|
||||
|
||||
computed_backlink = so.SchemaField(
|
||||
computed_link_alias_is_backward = so.SchemaField(
|
||||
bool,
|
||||
default=None,
|
||||
compcoef=0.99,
|
||||
)
|
||||
computed_link_alias = so.SchemaField(
|
||||
so.Object,
|
||||
default=None,
|
||||
compcoef=0.99,
|
||||
|
|
@ -1172,55 +1177,15 @@ class PointerCommandOrFragment(
|
|||
)
|
||||
|
||||
if isinstance(target_ref, ComputableRef):
|
||||
schema, inf_target_ref, base = self._parse_computable(
|
||||
schema, inf_target_ref = self._parse_computable(
|
||||
target_ref.expr, schema, context)
|
||||
elif (expr := self.get_local_attribute_value('expr')) is not None:
|
||||
schema = s_types.materialize_type_in_attribute(
|
||||
schema, context, self, 'target')
|
||||
schema, inf_target_ref, base = self._parse_computable(
|
||||
schema, inf_target_ref = self._parse_computable(
|
||||
expr.qlast, schema, context)
|
||||
else:
|
||||
inf_target_ref = None
|
||||
base = None
|
||||
|
||||
if base is not None:
|
||||
self.set_attribute_value(
|
||||
'bases', so.ObjectList.create(schema, [base]),
|
||||
)
|
||||
|
||||
self.set_attribute_value(
|
||||
'is_derived', True
|
||||
)
|
||||
if isinstance(self, inheriting.AlterInheritingObject):
|
||||
self._propagate_is_derived(schema, context, True)
|
||||
|
||||
if context.declarative:
|
||||
self.set_attribute_value(
|
||||
'declared_overloaded', True
|
||||
)
|
||||
|
||||
# If we *were* an aliased computed but are changing to not be
|
||||
# one, we need to update our base and is_derived to not be.
|
||||
elif (
|
||||
isinstance(self, inheriting.AlterInheritingObject)
|
||||
and self.has_attribute_value('expr')
|
||||
and (
|
||||
(cur_base := self.scls.get_bases(schema).objects(schema)[0])
|
||||
and (subj := cur_base.get_subject(schema))
|
||||
and subj == self.scls.get_subject(schema)
|
||||
)
|
||||
):
|
||||
mcls = self.get_schema_metaclass()
|
||||
base = schema.get(
|
||||
not_none(mcls.get_default_base_name()),
|
||||
type=mcls,
|
||||
)
|
||||
|
||||
self.set_attribute_value(
|
||||
'bases', so.ObjectList.create(schema, [base]),
|
||||
)
|
||||
|
||||
self._propagate_is_derived(schema, context, None)
|
||||
|
||||
if inf_target_ref is not None:
|
||||
srcctx = self.get_attribute_source_context('target')
|
||||
|
|
@ -1239,19 +1204,6 @@ class PointerCommandOrFragment(
|
|||
# There is an expression, therefore it is a computable.
|
||||
self.set_attribute_value('computable', True)
|
||||
|
||||
# If a change to the computable definition might be changing
|
||||
# the bases, we need to create a rebase.
|
||||
if base is not None and isinstance(self, sd.AlterObject):
|
||||
assert isinstance(self, inheriting.InheritingObjectCommand)
|
||||
assert isinstance(base, Pointer)
|
||||
_, cmd = self._rebase_ref_cmd(
|
||||
schema, context, self.scls,
|
||||
self.scls.get_bases(schema).objects(schema),
|
||||
[base],
|
||||
)
|
||||
if cmd:
|
||||
self.add(cmd)
|
||||
|
||||
return schema
|
||||
|
||||
def _parse_computable(
|
||||
|
|
@ -1262,7 +1214,6 @@ class PointerCommandOrFragment(
|
|||
) -> Tuple[
|
||||
s_schema.Schema,
|
||||
s_types.TypeShell[s_types.Type],
|
||||
Optional[PointerLike],
|
||||
]:
|
||||
from edb.ir import ast as irast
|
||||
from edb.ir import typeutils as irtyputils
|
||||
|
|
@ -1283,7 +1234,6 @@ class PointerCommandOrFragment(
|
|||
value=s_expr.Expression.from_ast(expr, schema, context.modaliases),
|
||||
)
|
||||
|
||||
base = None
|
||||
target = expression.irast.stype
|
||||
target_shell = target.as_shell(expression.irast.schema)
|
||||
if (
|
||||
|
|
@ -1304,39 +1254,42 @@ class PointerCommandOrFragment(
|
|||
|
||||
# Process a computable pointer which potentially could be an
|
||||
# aliased link that should inherit link properties.
|
||||
if isinstance(result_expr, irast.Set) and result_expr.rptr is not None:
|
||||
expr_rptr = result_expr.rptr
|
||||
computed_link_alias = None
|
||||
computed_link_alias_is_backward = None
|
||||
if (
|
||||
isinstance(result_expr, irast.Set)
|
||||
and (expr_rptr := result_expr.rptr) is not None
|
||||
and expr_rptr.direction is PointerDirection.Outbound
|
||||
and expr_rptr.source.rptr is None
|
||||
and isinstance(expr_rptr.ptrref, irast.PointerRef)
|
||||
and schema.has_object(expr_rptr.ptrref.id)
|
||||
):
|
||||
new_schema, aliased_ptr = irtyputils.ptrcls_from_ptrref(
|
||||
expr_rptr.ptrref, schema=schema
|
||||
)
|
||||
# Only pointers coming from the same source as the
|
||||
# alias should be "inherited" (in order to preserve
|
||||
# link props). Random paths coming from other sources
|
||||
# get treated same as any other arbitrary expression
|
||||
# in a computable.
|
||||
if (
|
||||
expr_rptr.direction is PointerDirection.Outbound
|
||||
and expr_rptr.source.rptr is None
|
||||
and isinstance(expr_rptr.ptrref, irast.PointerRef)
|
||||
and schema.has_object(expr_rptr.ptrref.id)
|
||||
aliased_ptr.get_source(new_schema) == source
|
||||
and isinstance(aliased_ptr, self.get_schema_metaclass())
|
||||
):
|
||||
new_schema, aliased_ptr = irtyputils.ptrcls_from_ptrref(
|
||||
expr_rptr.ptrref, schema=schema
|
||||
)
|
||||
# Only pointers coming from the same source as the
|
||||
# alias should be "inherited" (in order to preserve
|
||||
# link props). Random paths coming from other sources
|
||||
# get treated same as any other arbitrary expression
|
||||
# in a computable.
|
||||
if (
|
||||
aliased_ptr.get_source(new_schema) == source
|
||||
and isinstance(aliased_ptr, self.get_schema_metaclass())
|
||||
):
|
||||
base = aliased_ptr
|
||||
schema = new_schema
|
||||
schema = new_schema
|
||||
computed_link_alias = aliased_ptr
|
||||
computed_link_alias_is_backward = False
|
||||
|
||||
# Do similar logic, but in reverse, to see if the computed pointer
|
||||
# is a computed backlink that we need to keep track of.
|
||||
computed_backlink = None
|
||||
if (
|
||||
base is None
|
||||
computed_link_alias is None
|
||||
and isinstance(orig_expr, irast.Set)
|
||||
and orig_expr.rptr
|
||||
and isinstance(
|
||||
orig_expr.rptr.ptrref, irast.TypeIntersectionPointerRef)
|
||||
and len(orig_expr.rptr.ptrref.rptr_specialization) == 1
|
||||
and expr_rptr
|
||||
and expr_rptr.direction is not PointerDirection.Outbound
|
||||
):
|
||||
ptrref = list(orig_expr.rptr.ptrref.rptr_specialization)[0]
|
||||
|
|
@ -1348,10 +1301,13 @@ class PointerCommandOrFragment(
|
|||
and not ptrref.out_source.is_opaque_union
|
||||
and isinstance(aliased_ptr, self.get_schema_metaclass())
|
||||
):
|
||||
computed_backlink = aliased_ptr
|
||||
computed_link_alias_is_backward = True
|
||||
computed_link_alias = aliased_ptr
|
||||
schema = new_schema
|
||||
|
||||
self.set_attribute_value('computed_backlink', computed_backlink)
|
||||
self.set_attribute_value('computed_link_alias', computed_link_alias)
|
||||
self.set_attribute_value(
|
||||
'computed_link_alias_is_backward', computed_link_alias_is_backward)
|
||||
|
||||
self.set_attribute_value('expr', expression)
|
||||
required, card = expression.irast.cardinality.to_schema_value()
|
||||
|
|
@ -1461,7 +1417,7 @@ class PointerCommandOrFragment(
|
|||
|
||||
self.set_attribute_value('computable', True)
|
||||
|
||||
return schema, target_shell, base
|
||||
return schema, target_shell
|
||||
|
||||
def _compile_expr(
|
||||
self,
|
||||
|
|
@ -2232,7 +2188,9 @@ class AlterPointer(
|
|||
and not self.has_attribute_value('cardinality')
|
||||
):
|
||||
self.set_attribute_value('cardinality', None)
|
||||
self.set_attribute_value('computed_backlink', None)
|
||||
self.set_attribute_value(
|
||||
'computed_link_alias_is_backward', None)
|
||||
self.set_attribute_value('computed_link_alias', None)
|
||||
|
||||
# Clear the placeholder value for 'expr'.
|
||||
self.set_attribute_value('expr', None)
|
||||
|
|
|
|||
|
|
@ -1300,6 +1300,27 @@ class TestEdgeQLLinkproperties(tb.QueryTestCase):
|
|||
]
|
||||
)
|
||||
|
||||
@test.xerror('Stack overflow!')
|
||||
async def test_edgeql_props_back_09(self):
|
||||
await self.assert_query_result(
|
||||
r'''
|
||||
select assert_exists((
|
||||
select Card { name, z := .<deck[IS User] {
|
||||
name, @count := @count }}
|
||||
filter .name = 'Dragon'
|
||||
));
|
||||
''',
|
||||
[
|
||||
{
|
||||
"name": "Dragon",
|
||||
"z": tb.bag([
|
||||
{"x": 2, "name": "Alice"},
|
||||
{"x": 1, "name": "Dave"},
|
||||
])
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
async def test_edgeql_props_schema_back_00(self):
|
||||
with self.assertRaisesRegex(
|
||||
edgedb.QueryError,
|
||||
|
|
|
|||
Loading…
Reference in a new issue