Make SET REQUIRED USING (...) work on link properties (#9015)

The SET REQUIRED USING path always used the `produce_ctes=True` mode
of `_compile_conversion_expression`, which depends on using `id` as
the key. That doesn't work for us here, since there's no `id` for a
link property.

If we supported using ctid or source, target as the key (see #5050) we
could make it work, but until then, use the `produce_ctes=False` mode
for link properties.

This means DML won't work there, but whatever.

Author: msullivan

edb/pgsql/compiler/pathctx.py

@@ -1108,7 +1108,10 @@ def _get_rel_path_output(
     ptr_info = None
     if ptrref and not isinstance(ptrref, irast.TypeIntersectionPointerRef):
         ptr_info = pg_types.get_ptrref_storage_info(
-            ptrref, resolve_type=False, link_bias=False)
+            ptrref,
+            resolve_type=False,
+            link_bias=bool(rel.path_id and rel.path_id.is_ptr_path()),
+        )
 
     if (rptr_dir is not None and
             rptr_dir != s_pointers.PointerDirection.Outbound):

edb/pgsql/delta.py

@@ -4333,7 +4333,7 @@ def _alter_pointer_optionality(
 
             source_rel_alias = f'source_{uuidgen.uuid1mc()}'
 
-            (conv_expr_ctes, _, _) = self._compile_conversion_expr(
+            (conv_expr_ctes, conv_expr, _) = self._compile_conversion_expr(
                 ptr,
                 fill_expr,
                 source_rel_alias,
@@ -4342,9 +4342,30 @@ def _alter_pointer_optionality(
                 context=context,
                 check_non_null=is_required and not is_multi,
                 allow_globals=is_default,
+                produce_ctes=not is_lprop,
             )
 
-            if not is_multi:
+            if is_lprop:
+                # The produce_ctes=True flow really wants to key
+                # everything based on id, which doesn't work for link
+                # properties.  If produce_ctes=True was able to use
+                # ctid or (source, target) for keying, we could use
+                # that here, but for now we will just use the
+                # old-school version. See #5050, which would require
+                # that in order to support DML in cast expressions.
+                assert conv_expr
+                update_with = (
+                    f'WITH {conv_expr_ctes}' if conv_expr_ctes else ''
+                )
+                update_qry = f'''
+                    {update_with}
+                    UPDATE {tab} AS {qi(source_rel_alias)}
+                    SET {qi(target_col)} = ({conv_expr})
+                    WHERE {qi(target_col)} IS NULL
+                '''
+                self.pgops.add(dbops.Query(update_qry))
+
+            elif not is_multi:
                 update_qry = textwrap.dedent(f'''\
                     WITH
                     "{source_rel_alias}" AS ({source_rel}),
@@ -4898,12 +4919,14 @@ def _compile_conversion_expr(
             )
             # Concat to string which is a JSON. Great. Equivalent to SQL:
             # '{"object_id": "' || {obj_id_ref} || '"}'
+            # We report 'source' for linkprops. Seems OK, I guess.
+            id_col = 'source' if is_lprop else 'id'
             detail = pgast.Expr(
                 name='||',
                 lexpr=pgast.StringConstant(val='{"object_id": "'),
                 rexpr=pgast.Expr(
                     name='||',
-                    lexpr=pgast.ColumnRef(name=('id',)),
+                    lexpr=pgast.ColumnRef(name=(id_col,)),
                     rexpr=pgast.StringConstant(val='"}'),
                 )
             )

edb/schema/pointers.py

@@ -2936,6 +2936,8 @@ def _alter_begin(
         schema: s_schema.Schema,
         context: sd.CommandContext,
     ) -> s_schema.Schema:
+        from edb.ir import utils as irutils
+
         orig_schema = schema
         schema = super()._alter_begin(schema, context)
         scls = self.scls
@@ -2988,6 +2990,17 @@ def _alter_begin(
                         span=self.span,
                     )
 
+                if (
+                    self.scls.is_link_property(schema)
+                    and irutils.contains_dml(self.fill_expr.ir_statement)
+                ):
+                    raise errors.UnsupportedFeatureError(
+                        f'USING clause for the alteration of optionality of '
+                        f'{vn} cannot include mutating statements, '
+                        'because it is a link property',
+                        span=self.span,
+                    )
+
             schema = self._propagate_if_expr_refs(
                 schema,
                 context,

tests/test_edgeql_ddl.py

@@ -19284,6 +19284,91 @@ async def test_edgeql_ddl_new_required_multi_pointer_04(self):
                 ALTER TYPE Foo ALTER LINK link SET REQUIRED;
             """)
 
+    async def test_edgeql_ddl_set_required_lprop_01(self):
+        await self.con.execute(r"""
+            CREATE TYPE default::User {
+                CREATE PROPERTY name: std::str;
+            };
+            CREATE TYPE default::Post {
+                CREATE MULTI LINK author: default::User {
+                    CREATE PROPERTY created_at: std::str;
+                };
+                CREATE PROPERTY title: std::str;
+            };
+            INSERT Post { author := {
+                (insert User { name := "1" }),
+                (insert User { name := "2" }),
+            } };
+        """)
+
+        await self.con.execute(r"""
+            ALTER TYPE default::Post {
+                ALTER LINK author {
+                    ALTER PROPERTY created_at {
+                        SET REQUIRED USING (.name);
+                    };
+                };
+            };
+        """)
+
+        async with self.assertRaisesRegexTx(
+            edgedb.MissingRequiredError,
+            "missing value"
+        ):
+            await self.con.execute(r"""
+                INSERT Post { author := {(insert User), (insert User)} };
+            """)
+
+    async def test_edgeql_ddl_set_required_lprop_02(self):
+        await self.con.execute(r"""
+            CREATE TYPE default::User {
+                CREATE PROPERTY name: std::str;
+            };
+            CREATE TYPE default::Post {
+                CREATE LINK author: default::User {
+                    CREATE PROPERTY created_at: std::str;
+                };
+                CREATE PROPERTY title: std::str;
+            };
+            INSERT Post { author := (insert User { name := "asdf" }) };
+        """)
+
+        await self.con.execute(r"""
+            ALTER TYPE default::Post {
+                ALTER LINK author {
+                    ALTER PROPERTY created_at {
+                        SET REQUIRED USING (<str>.name);
+                    };
+                };
+            };
+        """)
+
+    async def test_edgeql_ddl_set_required_lprop_03(self):
+        await self.con.execute(r"""
+            CREATE TYPE default::User;
+            CREATE TYPE default::Post {
+                CREATE MULTI LINK author: default::User {
+                    CREATE PROPERTY created_at: std::str;
+                };
+                CREATE PROPERTY title: std::str;
+            };
+            INSERT Post { author := {(insert User), (insert User)} };
+        """)
+
+        async with self.assertRaisesRegexTx(
+            edgedb.UnsupportedFeatureError,
+            "cannot include mutating statements"
+        ):
+            await self.con.execute(r"""
+                ALTER TYPE default::Post {
+                    ALTER LINK author {
+                        ALTER PROPERTY created_at {
+                            SET REQUIRED USING (<str>(insert User).id);
+                        };
+                    };
+                };
+            """)
+
     async def test_edgeql_ddl_link_union_delete_01(self):
         await self.con.execute(r"""
             CREATE TYPE default::M;