Repository: geldata/gel
Pull request: Forbid DML in non-scalar function args
Closes #5172

Problem statement:
Disallow passing insert/update objects to user-defined functions
(I thought there was an issue for this but I couldn't find it)

If a function is implemented as a SQL function, it won't be able to use overlays to see newly created data. This can cause a bunch of problems, including breaking cardinality.

I think that we don't have any stdlib functions that take object types that are implemented as functions, so this probably just affects user defined functions.

This will affect some access policy uses also

Hints from issue discussion:

Commit message: Forbid DML in non-scalar function args

Patch to review:
diff --git a/edb/edgeql/compiler/func.py b/edb/edgeql/compiler/func.py
index da19483554e..5c6a348af28 100644
--- a/edb/edgeql/compiler/func.py
+++ b/edb/edgeql/compiler/func.py
@@ -155,9 +155,10 @@ def compile_FunctionCall(
         matched_call = matched[0]
 
     func = matched_call.func
+    assert isinstance(func, s_func.Function)
 
     # Record this node in the list of potential DML expressions.
-    if isinstance(func, s_func.Function) and func.get_has_dml(env.schema):
+    if func.get_has_dml(env.schema):
         ctx.env.dml_exprs.append(expr)
 
         # This is some kind of mutation, so we need to check if it is
@@ -186,7 +187,6 @@ def compile_FunctionCall(
                 context=expr.context,
             )
 
-    assert isinstance(func, s_func.Function)
     func_name = func.get_shortname(env.schema)
 
     matched_func_params = func.get_params(env.schema)
@@ -214,6 +214,18 @@ def compile_FunctionCall(
         ctx=ctx,
     )
 
+    if func_name.module != 'std':
+        for arg in final_args:
+            if arg.expr.typeref.is_scalar:
+                continue
+            if not irutils.contains_dml(arg.expr):
+                continue
+            raise errors.UnsupportedFeatureError(
+                'newly created or updated objects cannot be passed to '
+                'functions',
+                context=arg.expr.context
+            )
+
     if not in_abstract_constraint:
         # We cannot add strong references to functions from
         # abstract constraints, since we cannot know which
diff --git a/tests/test_edgeql_calls.py b/tests/test_edgeql_calls.py
index bc2badb138b..d4c1b67b275 100644
--- a/tests/test_edgeql_calls.py
+++ b/tests/test_edgeql_calls.py
@@ -1673,6 +1673,39 @@ async def test_edgeql_calls_obj_04(self):
                 USING (s.name ++ <str>s.required);
         """)
 
+    async def test_edgeql_calls_obj_05(self):
+        await self.con.execute("""
+            CREATE TYPE Ghost {
+                CREATE PROPERTY name -> str;
+            };
+
+            CREATE FUNCTION boo(s: Ghost) -> set of str
+                USING ("oh my, " ++ s.name ++ " scared me!");
+
+            INSERT Ghost { name := 'Casper' };
+        """)
+
+        await self.assert_query_result(
+            "SELECT boo((SELECT Ghost))",
+            ["oh my, Casper scared me!"],
+        )
+
+        async with self.assertRaisesRegexTx(
+            edgedb.UnsupportedFeatureError,
+            r'newly created or updated objects cannot be passed to functions',
+        ):
+            await self.con.execute(
+                r"SELECT boo((UPDATE Ghost SET { name := 'Tom' }))",
+            )
+
+        async with self.assertRaisesRegexTx(
+            edgedb.UnsupportedFeatureError,
+            r'newly created or updated objects cannot be passed to functions',
+        ):
+            await self.con.execute(
+                r"SELECT boo((INSERT Ghost { name := 'Jack' }));",
+            )
+
     async def test_edgeql_call_builtin_obj(self):
         await self.con.execute(
             r"""


Code review comments:
diff_hunk: @@ -214,6 +214,18 @@ def compile_FunctionCall(
         ctx=ctx,
     )
 
+    if func_name.module != 'std':
path: edb/edgeql/compiler/func.py
text: @user1:
Let's check whether the function has a definition in edgeql instead; (`nativecode`)