Make attributes non-inheritable by default

Automatic inheritance of attributes is sometimes inconvenient or
undesirable. A "description" attribute, for example, is unlikely to be
useful when inherited. Thus, we make all attributes non-inheritable by
default, with an option to opt-in to automatic inheritance with the new
`INHERITABLE` qualifier for `CREATE ATTRIBUTE`:

    CREATE INHERITABLE ATTRIBUTE foo;

Issue: #306

edb/lang/edgeql/ast.py

@@ -617,6 +617,7 @@ class DropRole(DropObject):
 
 class CreateAttribute(CreateExtendingObject):
     type: typing.Optional[TypeExpr]
+    inheritable: bool
 
 
 class DropAttribute(DropObject):

edb/lang/edgeql/codegen.py

@@ -839,8 +839,11 @@ def visit_AlterObjectProperty(self, node):
 
     def visit_CreateAttribute(self, node):
         after_name = lambda: self._ddl_visit_bases(node)
-        self._visit_CreateObject(node, 'ABSTRACT ATTRIBUTE',
-                                 after_name=after_name)
+        if node.inheritable:
+            tag = 'ABSTRACT INHERITABLE ATTRIBUTE'
+        else:
+            tag = 'ABSTRACT ATTRIBUTE'
+        self._visit_CreateObject(node, tag, after_name=after_name)
 
     def visit_DropAttribute(self, node):
         self._visit_DropObject(node, 'ABSTRACT ATTRIBUTE')

edb/lang/edgeql/parser/grammar/ddl.py

@@ -852,6 +852,17 @@ def reduce_CreateAttribute(self, *kids):
             name=kids[3].val,
             bases=kids[4].val,
             commands=kids[5].val,
+            inheritable=False,
+        )
+
+    def reduce_CreateInheritableAttribute(self, *kids):
+        r"""%reduce CREATE ABSTRACT INHERITABLE ATTRIBUTE
+                    NodeName OptExtending OptCreateCommandsBlock"""
+        self.val = qlast.CreateAttribute(
+            name=kids[4].val,
+            bases=kids[5].val,
+            commands=kids[6].val,
+            inheritable=True,
         )
 
 

edb/lang/edgeql/parser/grammar/keywords.py

@@ -55,6 +55,7 @@
     "implicit",
     "index",
     "infix",
+    "inheritable",
     "inherited",
     "into",
     "last",

edb/lang/schema/ast.py

@@ -109,7 +109,7 @@ class ScalarTypeDeclaration(Declaration):
 
 
 class AttributeDeclaration(Declaration):
-    type: qlast.TypeName
+    inheritable: bool = False
 
 
 class ObjectTypeDeclaration(Declaration):

edb/lang/schema/attributes.py

@@ -18,6 +18,8 @@
 #
 
 
+import typing
+
 from edb.lang.edgeql import ast as qlast
 from edb.lang.edgeql import errors as qlerrors
 
@@ -35,6 +37,9 @@ class Attribute(inheriting.InheritingObject):
     name = so.SchemaField(
         sn.Name, inheritable=False, compcoef=0.2)
 
+    inheritable = so.SchemaField(
+        bool, default=False, compcoef=0.2)
+
 
 class AttributeValue(inheriting.InheritingObject):
 
@@ -47,6 +52,9 @@ class AttributeValue(inheriting.InheritingObject):
     value = so.SchemaField(
         str, compcoef=0.909)
 
+    inheritable = so.SchemaField(
+        bool, default=False, compcoef=0.2)
+
     def __str__(self):
         return '<{}: at 0x{:x}>'.format(self.__class__.__name__, id(self))
 
@@ -57,6 +65,7 @@ class AttributeSubject(so.Object):
     attributes_refs = so.RefDict(
         attr='attributes',
         local_attr='own_attributes',
+        non_inheritable_attr='non_inheritable_attributes',
         ref_cls=AttributeValue)
 
     attributes = so.SchemaField(
@@ -69,6 +78,11 @@ class AttributeSubject(so.Object):
         inheritable=False, ephemeral=True, coerce=True,
         default=so.ObjectIndexByShortname)
 
+    non_inheritable_attributes = so.SchemaField(
+        so.ObjectIndexByShortname, compcoef=0.909,
+        inheritable=False, ephemeral=True, coerce=True,
+        default=so.ObjectIndexByShortname)
+
     def add_attribute(self, schema, attribute, replace=False):
         schema = self.add_classref(
             schema, 'attributes', attribute, replace=replace)
@@ -78,18 +92,24 @@ def del_attribute(self, schema, attribute_name):
         shortname = sn.shortname_from_fullname(attribute_name)
         return self.del_classref(schema, 'attributes', shortname)
 
-    def get_attribute(self, schema, name):
-        return self.get_attributes(schema).get(schema, name, None)
+    def get_attribute(self, schema, name: str) -> typing.Optional[str]:
+        attrval = self.get_attributes(schema).get(schema, name, None)
+        return attrval.get_value(schema) if attrval is not None else None
 
     def set_attribute(self, schema, attr: Attribute, value: str):
         attrname = attr.get_name(schema)
         existing = self.get_own_attributes(schema).get(schema, attrname, None)
+        if existing is None:
+            existing = self.get_non_inheritable_attributes(schema).get(
+                schema, attrname, None)
         if existing is None:
             my_name = self.get_name(schema)
             ann = sn.get_specialized_name(attrname, my_name)
             an = sn.Name(name=ann, module=my_name.module)
             schema, av = AttributeValue.create_in_schema(
-                schema, name=an, value=value)
+                schema, name=an, value=value,
+                subject=self, attribute=attr,
+                inheritable=attr.get_inheritable(schema))
             schema = self.add_attribute(schema, av)
         else:
             schema, updated = existing.set_field_value('value', value)
@@ -110,6 +130,24 @@ class AttributeCommand(sd.ObjectCommand, schema_metaclass=Attribute,
 class CreateAttribute(AttributeCommand, sd.CreateObject):
     astnode = qlast.CreateAttribute
 
+    @classmethod
+    def _cmd_tree_from_ast(cls, schema, astnode, context):
+        cmd = super()._cmd_tree_from_ast(schema, astnode, context)
+        cmd.update((
+            sd.AlterObjectProperty(
+                property='inheritable',
+                new_value=astnode.inheritable,
+            ),
+        ))
+
+        return cmd
+
+    def _apply_field_ast(self, schema, context, node, op):
+        if op.property == 'inheritable':
+            node.inheritable = op.new_value
+        else:
+            super()._apply_field_ast(schema, context, node, op)
+
 
 class AlterAttribute(AttributeCommand, sd.AlterObject):
     pass
@@ -195,6 +233,10 @@ def _cmd_tree_from_ast(cls, schema, astnode, context):
             sd.AlterObjectProperty(
                 property='value',
                 new_value=value
+            ),
+            sd.AlterObjectProperty(
+                property='inheritable',
+                new_value=attr.get_inheritable(schema),
             )
         ))
 
@@ -209,6 +251,8 @@ def _apply_field_ast(self, schema, context, node, op):
             pass
         elif op.property == 'subject':
             pass
+        elif op.property == 'inheritable':
+            pass
         else:
             super()._apply_field_ast(schema, context, node, op)
 
@@ -221,12 +265,15 @@ def apply(self, schema, context):
             name = sn.shortname_from_fullname(self.classname)
             attrs = attrsubj.scls.get_own_attributes(schema)
             attribute = attrs.get(schema, name, None)
+            if attribute is None:
+                attrs = attrsubj.scls.get_non_inheritable_attributes(schema)
+                attribute = attrs.get(schema, name, None)
+
             if attribute is None:
                 schema, attribute = super().apply(schema, context)
                 schema = self.add_attribute(schema, attribute, attrsubj.scls)
             else:
-                schema, attribute = sd.AlterObject.apply(
-                    self, schema, context)
+                schema, attribute = sd.AlterObject.apply(self, schema, context)
 
             return schema, attribute
 

edb/lang/schema/codegen.py

@@ -187,15 +187,11 @@ def visit_ScalarTypeDeclaration(self, node):
         self._visit_Declaration(node)
 
     def visit_AttributeDeclaration(self, node):
-        def after_name(node):
-            if node.type:
-                self.write(' ')
-                self.visit(node.type)
-                self.write(' ')
-
         if node.abstract:
             self.write('abstract ')
-        self._visit_Declaration(node, after_name=after_name)
+        if node.inheritable:
+            self.write('inheritable ')
+        self._visit_Declaration(node)
 
     def visit_ObjectTypeDeclaration(self, node):
         self._visit_qualifier(node)

edb/lang/schema/constraints.py

@@ -283,7 +283,7 @@ def format_error_message(self, schema):
             subjname = subject.get_shortname(schema)
             subjtitle = subjname.name
         else:
-            subjtitle = titleattr.value
+            subjtitle = titleattr
 
         formatted = errmsg.format(__subject__=subjtitle)
 

edb/lang/schema/declarative.py

@@ -105,6 +105,8 @@ def load_module(self, module_name, decl_ast):
                 objcls_kw['is_abstract'] = decl.delegated
             if hasattr(decl, 'final'):
                 objcls_kw['is_final'] = decl.final
+            if hasattr(decl, 'inheritable'):
+                objcls_kw['inheritable'] = decl.inheritable
 
             if objcls is s_constr.Constraint:
                 objcls_kw['return_type'] = self._schema.get('std::bool')

edb/lang/schema/inheriting.py

@@ -454,6 +454,19 @@ def merge(self, *objs, schema, dctx=None):
                 if other_coll is None:
                     continue
 
+                if refdict.non_inheritable_attr:
+                    non_inh_coll = obj.get_explicit_field_value(
+                        schema, refdict.non_inheritable_attr, None)
+
+                    if non_inh_coll:
+                        other_coll = type(other_coll).create(schema, {
+                            v for k, v in other_coll.items(schema)
+                            if not non_inh_coll.has(schema, k)
+                        })
+
+                if not other_coll:
+                    continue
+
                 if this_coll is None:
                     schema = self.set_field_value(
                         schema, refdict.attr, other_coll)

edb/lang/schema/objects.py

@@ -253,6 +253,7 @@ class RefDict(struct.Struct):
 
     local_attr = struct.Field(str)
     attr = struct.Field(str)
+    non_inheritable_attr = struct.Field(str, default=None)
     backref_attr = struct.Field(str, default='subject')
     requires_explicit_inherit = struct.Field(bool, default=False)
     ref_cls = struct.Field(type)
@@ -853,28 +854,11 @@ def delta(cls, old, new, *, context=None, old_schema, new_schema):
             else:
                 full_delta = alter_delta = delta
 
-            old_idx_key = lambda o: o.get_name(old_schema)
-            new_idx_key = lambda o: o.get_name(new_schema)
-
             for refdict in cls.get_refdicts():
-                local_attr = refdict.local_attr
-
-                if old:
-                    oldcoll = old.get_field_value(old_schema, local_attr)
-                    oldcoll_idx = ordered.OrderedIndex(
-                        oldcoll.objects(old_schema), key=old_idx_key)
-                else:
-                    oldcoll_idx = {}
-
-                if new:
-                    newcoll = new.get_field_value(new_schema, local_attr)
-                    newcoll_idx = ordered.OrderedIndex(
-                        newcoll.objects(new_schema), key=new_idx_key)
-                else:
-                    newcoll_idx = {}
-
-                cls.delta_sets(oldcoll_idx, newcoll_idx, alter_delta, context,
-                               old_schema=old_schema, new_schema=new_schema)
+                cls._delta_refdict(
+                    old, new, delta=alter_delta,
+                    refdict=refdict, context=context,
+                    old_schema=old_schema, new_schema=new_schema)
 
             if alter_delta is not full_delta:
                 if alter_delta.has_subcommands():
@@ -884,10 +868,46 @@ def delta(cls, old, new, *, context=None, old_schema, new_schema):
 
         return full_delta
 
+    @classmethod
+    def _delta_refdict(cls, old, new, *, delta, refdict, context,
+                       old_schema, new_schema):
+
+        old_idx_key = lambda o: o.get_name(old_schema)
+        new_idx_key = lambda o: o.get_name(new_schema)
+
+        def _delta_subdict(attr):
+            if old:
+                oldcoll = old.get_field_value(old_schema, attr)
+                oldcoll_idx = ordered.OrderedIndex(
+                    oldcoll.objects(old_schema), key=old_idx_key)
+            else:
+                oldcoll_idx = {}
+
+            if new:
+                newcoll = new.get_field_value(new_schema, attr)
+                newcoll_idx = ordered.OrderedIndex(
+                    newcoll.objects(new_schema), key=new_idx_key)
+            else:
+                newcoll_idx = {}
+
+            cls.delta_sets(oldcoll_idx, newcoll_idx, delta, context,
+                           old_schema=old_schema, new_schema=new_schema)
+
+        _delta_subdict(refdict.local_attr)
+        if refdict.non_inheritable_attr:
+            _delta_subdict(refdict.non_inheritable_attr)
+
     def add_classref(self, schema, collection, obj, replace=False):
         refdict = type(self).get_refdict(collection)
         attr = refdict.attr
-        local_attr = refdict.local_attr
+
+        if (refdict.non_inheritable_attr
+                and type(obj).get_field('inheritable') is not None
+                and not obj.get_inheritable(schema)):
+            local_attr = refdict.non_inheritable_attr
+        else:
+            local_attr = refdict.local_attr
+
         colltype = type(self).get_field(local_attr).type
 
         local_coll = self.get_explicit_field_value(schema, local_attr, None)
@@ -916,6 +936,12 @@ def del_classref(self, schema, collection, key):
         refdict = type(self).get_refdict(collection)
         attr = refdict.attr
         local_attr = refdict.local_attr
+        non_inh_attr = refdict.non_inheritable_attr
+
+        if non_inh_attr is not None:
+            non_inh_coll = self.get_field_value(schema, non_inh_attr)
+        else:
+            non_inh_coll = None
 
         local_coll = self.get_field_value(schema, local_attr)
         all_coll = self.get_field_value(schema, attr)
@@ -924,6 +950,10 @@ def del_classref(self, schema, collection, key):
             schema, local_coll = local_coll.delete(schema, [key])
             schema = self.set_field_value(schema, local_attr, local_coll)
 
+        if non_inh_coll and non_inh_coll.has(schema, key):
+            schema, non_inh_coll = non_inh_coll.delete(schema, [key])
+            schema = self.set_field_value(schema, non_inh_attr, non_inh_coll)
+
         if all_coll and all_coll.has(schema, key):
             schema, all_coll = all_coll.delete(schema, [key])
             schema = self.set_field_value(schema, attr, all_coll)