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

ISE with aliases and with bindings #6561

Open
aljazerzen opened this issue Dec 4, 2023 · 1 comment
Open

ISE with aliases and with bindings #6561

aljazerzen opened this issue Dec 4, 2023 · 1 comment
Assignees

Comments

@aljazerzen
Copy link
Contributor

Schema:

module default {
    type Hello {
        multi link worlds := .<hello[IS World];
    };

    type World {
        required single hello: Hello;

        optional foo: int16;
    };

    alias WorldAlias := World {
        bar := (
            with boo := 5
            select boo
        )
    };
}
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/ddl.py", line 775, in statements_from_delta
          stmts = ddlast_from_delta(
                  ^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/ddl.py", line 754, in ddlast_from_delta
          schema = command.apply(schema, context)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/delta.py", line 3150, in apply
          schema = self.apply_caused(schema, context)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/delta.py", line 985, in apply_caused
          schema = op.apply(schema, context)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/delta.py", line 3145, in apply
          schema = self._create_begin(schema, context)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/referencing.py", line 1094, in _create_begin
          return super()._create_begin(schema, context)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/referencing.py", line 660, in _create_begin
          schema = super()._create_begin(schema, context)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/inheriting.py", line 755, in _create_begin
          schema = super()._create_begin(schema, context)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/delta.py", line 3033, in _create_begin
          props = self.get_resolved_attributes(schema, context)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/delta.py", line 2666, in get_resolved_attributes
          result[attr] = self.get_resolved_attribute_value(
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/delta.py", line 2609, in get_resolved_attribute_value
          value = self.resolve_attribute_value(
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/delta.py", line 2634, in resolve_attribute_value
          value = self._resolve_attr_value(
                  ^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/delta.py", line 570, in _resolve_attr_value
          value = value.resolve(schema)
                  ^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/types.py", line 609, in resolve
          return schema.get(
                 ^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/schema.py", line 416, in get
          return self._get(
                 ^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/schema.py", line 2026, in _get
          return self._base_schema._get(
                 ^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/schema.py", line 1396, in _get
          self._raise_bad_reference(
        File "/home/aljaz/EdgeDB/edgedb/edb/schema/schema.py", line 1437, in _raise_bad_reference
          raise errors.InvalidReferenceError(
      edb.errors.InvalidReferenceError: scalar type 'default::boo' does not exist
@aljazerzen
Copy link
Contributor Author

When alias is created, its shape might contain references to other shapes generated during compilation.

For example:

alias WorldAlias := World {
    bar := 3,
    baz := (select Hello { a := 4 }),
};

Compilation will generate two shapes: one for World and one for baz.
These shapes are then added into the schema, since they are actually needed.
This happens here:

for vt in ir.views.values():
if not _is_view_from_alias(classname, vt, new_schema):
continue
if isinstance(vt, s_types.Collection):
coll_expr_aliases.append(vt)
elif prev_expr is not None or not schema.has_object(vt.id):
new_schema = vt.set_field_value(
new_schema, 'alias_is_persistent', True)
new_schema = vt.set_field_value(
new_schema, 'from_global', is_global)
expr_aliases.append(vt)

To decide which shapes are actually needed, we use this criteria:

def _is_view_from_alias(
our_name: sn.QualName,
obj: s_types.Type,
schema: s_schema.Schema,
) -> bool:
name = obj.get_name(schema)
if our_name == name:
return True
name_prefix = f'__{our_name.name}__'
name = obj.get_name(schema)
return (
isinstance(name, sn.QualName)
and name.module == our_name.module
and name.name.startswith(name_prefix)
)

We basically check if the name of the shape starts with dunder name of the alias.

This works because we generate "pretty" names when compiling aliases:

if needs_real_name and view_rptr is not None:
# Make sure persistent schema expression aliases have properly formed
# names as opposed to the usual mangled form of the ephemeral
# aliases. This is needed for introspection readability, as well
# as helps in maintaining proper type names for schema
# representations that require alphanumeric names, such as
# GraphQL.
#
# We use the name of the source together with the name
# of the inbound link to form the name, so in e.g.
# CREATE ALIAS V := (SELECT Foo { bar: { baz: { ... } })
# The name of the innermost alias would be "__V__bar__baz".
source_name = view_rptr.source.get_name(ctx.env.schema).name
if not source_name.startswith('__'):
source_name = f'__{source_name}'
if view_rptr.ptrcls_name is not None:
ptr_name = view_rptr.ptrcls_name.name
elif view_rptr.ptrcls is not None:
ptr_name = view_rptr.ptrcls.get_shortname(ctx.env.schema).name
else:
raise errors.InternalServerError(
'_process_view in schema mode received view_rptr with '
'neither ptrcls_name, not ptrcls'
)

The problem of this issue stems from that fact that when we compile with blocks, we don't use the same code path, but explicitly set the toplevel_result_view_name:

subctx.toplevel_result_view_name = view_name

So the name of the name of the view for our World.bar is default::boo@w~5.
So it does not get added into the schema, and subsequent lookups into the type of the alias fail with InvalidReferenceError.

A few observations:

  • we should also generate nicer names for with binding names.
  • we should include all needed types in the schema.
  • we should not use just their names to determine which types are needed and with are not.

So I've tried removing the check for _is_view_from_alias to add all the shapes from compilation into the schema.

This does not fix the issue, it fails with the same error, because the new shape is removed during computing of the delta between current and target schema:

edgedb/edb/schema/ddl.py

Lines 314 to 327 in 4865706

if issubclass(sclass, so.DerivableObject):
def _only_generic(
schema: s_schema.Schema,
obj: so.Object,
) -> bool:
assert isinstance(obj, so.DerivableObject)
return (
obj.generic(schema)
or (
isinstance(obj, s_types.Type)
and obj.get_from_global(schema)
)
)
filters.append(_only_generic)

My questions are:

  • when we create auxiliary objects for aliases (types in this case) should they appear in the delta tree?
  • if they should, why don't we ever generate DDL in migrations for them? Where does the "delta_to_ddl" compilation say "hey, you were generated for an alias, you are a prerequisite for some other object, so I won't generate DDL for you"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant