Use ON CONFLICT to implement UNLESS CONFLICT more often (#7472)

We were being too strict in detecting whether a single pointer
specified in an INSERT contained DML (it was picking up DML in other
fields of the insert through the source).

Also drop an old and overly conservative check that uses a conflict
CTE whenever DML statements have already been compiled. We now already
do the more fine-grained stuff that the comment was talking about.

edb/ir/utils.py

@@ -31,6 +31,7 @@
     Sequence,
     Dict,
     List,
+    Iterable,
     Set,
     cast,
     TYPE_CHECKING,
@@ -321,10 +322,17 @@ def visit_Set(self, node: irast.Set) -> bool:
         return bool(self.generic_visit(node))
 
 
-def contains_dml(stmt: irast.Base, *, skip_bindings: bool = False) -> bool:
+def contains_dml(
+    stmt: irast.Base,
+    *,
+    skip_bindings: bool = False,
+    skip_nodes: Iterable[irast.Base] = (),
+) -> bool:
     """Check whether a statement contains any DML in a subtree."""
     # TODO: Make this caching.
     visitor = ContainsDMLVisitor(skip_bindings=skip_bindings)
+    for node in skip_nodes:
+        visitor._memo[node] = False
     res = visitor.visit(stmt) is True
     return res
 

edb/pgsql/compiler/dml.py

@@ -1319,17 +1319,9 @@ def insert_needs_conflict_cte(
 ) -> bool:
     # We need to generate a conflict CTE if it is possible that
     # the query might generate two conflicting objects.
-    # For now, we calculate that conservatively and generate a conflict
-    # CTE if there are iterators or other DML statements that we have
-    # seen already.
-    # A more fine-grained scheme would check if there are enclosing
-    # iterators or INSERT/UPDATEs to types that could conflict.
     if on_conflict.else_fail:
         return False
 
-    if ctx.dml_stmts:
-        return True
-
     if on_conflict.always_check or ir_stmt.conflict_checks:
         return True
 
@@ -1359,7 +1351,11 @@ def insert_needs_conflict_cte(
         if (
             ptr_info.table_type == 'ObjectType'
             and shape_el.expr.expr
-            and irutils.contains_dml(shape_el.expr.expr, skip_bindings=True)
+            and irutils.contains_dml(
+                shape_el.expr.expr,
+                skip_bindings=True,
+                skip_nodes=(ir_stmt.subject,),
+            )
         ):
             return True
 

tests/test_edgeql_sql_codegen.py

@@ -410,3 +410,54 @@ def test_codegen_materialized_02(self):
             count,
             1,
             f"filter not materialized")
+
+    def test_codegen_unless_conflict_01(self):
+        # Should have no conflict check because it has no subtypes
+        sql = self._compile('''
+            insert User { name := "test" }
+            unless conflict
+        ''')
+
+        self.assertIn(
+            "ON CONFLICT", sql,
+            "insert unless conflict not using ON CONFLICT"
+        )
+
+    def test_codegen_unless_conflict_02(self):
+        # Should have no conflict check because it has no subtypes
+        sql = self._compile('''
+            insert User { name := "test" }
+            unless conflict on (.name)
+            else (User)
+        ''')
+
+        self.assertIn(
+            "ON CONFLICT", sql,
+            "insert unless conflict not using ON CONFLICT"
+        )
+
+    SCHEMA_asdf = r'''
+        type Tgt;
+        type Tgt2;
+        type Src {
+            name: str { constraint exclusive; }
+            tgt: Tgt;
+            multi tgts: Tgt2;
+        };
+    '''
+
+    def test_codegen_unless_conflict_03(self):
+        # Should have no conflict check because it has no subtypes
+        sql = self._compile('''
+        WITH MODULE asdf
+        INSERT Src {
+            name := 'asdf',
+            tgt := (select Tgt limit 1),
+            tgts := (insert Tgt2),
+        } UNLESS CONFLICT
+        ''')
+
+        self.assertIn(
+            "ON CONFLICT", sql,
+            "insert unless conflict not using ON CONFLICT"
+        )