Merge pull request #69 from sirixdb/feature/sdb-select-item-delete-support

Fix the UpdateList

Author: JohannesLichtenberger

src/main/java/io/brackit/query/update/UpdateList.java

@@ -53,6 +53,16 @@ public final class UpdateList {
                                                              OpType.REPLACE_VALUE,
                                                              OpType.REPLACE_ELEMENT_CONTENT);
 
+  // Property update operations that should be skipped for deleted nodes
+  // Per XQuery Update Facility 1.0 section 3.2.1:
+  // "If a node is marked for deletion, updates to its properties have no effect."
+  // Note: Insert operations should NOT be skipped - they create new sibling/child
+  // nodes that persist after the target is deleted.
+  private static final EnumSet<OpType> propertyUpdateOps = EnumSet.of(OpType.RENAME,
+                                                                      OpType.REPLACE_NODE,
+                                                                      OpType.REPLACE_VALUE,
+                                                                      OpType.REPLACE_ELEMENT_CONTENT);
+
   private final List<UpdateOp> ops;
 
   public UpdateList() {
@@ -87,21 +97,20 @@ public void apply() throws QueryException {
     }
 
     // Collect all nodes marked for deletion
-    // Per XQuery Update Facility 1.0 section 3.2.1:
-    // "If a node is marked for deletion, updates to its properties have no effect."
     final Set<Item> deletedNodes = new HashSet<>();
     for (final UpdateOp op : ops) {
       if (op.getType() == OpType.DELETE) {
         deletedNodes.add(op.getTarget());
       }
     }
 
-    // Apply all updates, skipping those targeting deleted nodes
+    // Apply all updates, skipping only property updates to deleted nodes
     for (final UpdateOp op : ops) {
-      // Skip non-delete updates to nodes that will be deleted
-      if (op.getType() != OpType.DELETE && deletedNodes.contains(op.getTarget())) {
+      // Skip property updates to nodes that will be deleted
+      // Insert operations should NOT be skipped - they create new nodes
+      if (propertyUpdateOps.contains(op.getType()) && deletedNodes.contains(op.getTarget())) {
         if (log.isDebugEnabled()) {
-          log.debug(String.format("Skipping update %s - target node is marked for deletion", op));
+          log.debug(String.format("Skipping property update %s - target node is marked for deletion", op));
         }
         continue;
       }
@@ -167,4 +176,4 @@ private void checkCompatibility(final UpdateOp op1, final UpdateOp op2) throws Q
   public List<UpdateOp> list() {
     return ops;
   }
-}
+}

src/test/java/io/brackit/query/update/UpdateListTest.java

@@ -125,6 +125,81 @@ public void updateAndDeleteDifferentNodesAppliesBoth() {
     assertEquals("Both operations should be applied", 2, appliedOps.size());
   }
 
+  /**
+   * Per XQuery Update Facility 1.0 section 3.2.2:
+   * Insert operations are applied before delete operations.
+   * An INSERT_BEFORE targeting a node that will be deleted should NOT be skipped.
+   * The inserted content becomes a sibling and persists after the delete.
+   */
+  @Test
+  public void insertBeforeDeletedNodeIsNotSkipped() {
+    final List<String> appliedOps = new ArrayList<>();
+    final StructuredItem targetItem = new ArrayObject(new QNm[] { new QNm("name") },
+                                                      new Sequence[] { new Str("test") });
+
+    final UpdateList updateList = new UpdateList();
+
+    // Add an INSERT_BEFORE operation targeting a node
+    updateList.append(new TestUpdateOp(OpType.INSERT_BEFORE, targetItem, () -> appliedOps.add("INSERT_BEFORE")));
+
+    // Add a DELETE operation for the same target
+    updateList.append(new TestUpdateOp(OpType.DELETE, targetItem, () -> appliedOps.add("DELETE")));
+
+    // Apply updates
+    updateList.apply();
+
+    // Both should be applied - INSERT_BEFORE creates a sibling, then DELETE removes the target
+    assertEquals("Both INSERT_BEFORE and DELETE should be applied", List.of("INSERT_BEFORE", "DELETE"), appliedOps);
+  }
+
+  /**
+   * INSERT_INTO targeting a deleted node should also be applied.
+   */
+  @Test
+  public void insertIntoDeletedNodeIsNotSkipped() {
+    final List<String> appliedOps = new ArrayList<>();
+    final StructuredItem targetItem = new ArrayObject(new QNm[] { new QNm("name") },
+                                                      new Sequence[] { new Str("test") });
+
+    final UpdateList updateList = new UpdateList();
+
+    // Add an INSERT_INTO operation
+    updateList.append(new TestUpdateOp(OpType.INSERT_INTO, targetItem, () -> appliedOps.add("INSERT_INTO")));
+
+    // Add a DELETE operation for the same target
+    updateList.append(new TestUpdateOp(OpType.DELETE, targetItem, () -> appliedOps.add("DELETE")));
+
+    // Apply updates
+    updateList.apply();
+
+    // Both should be applied
+    assertEquals("Both INSERT_INTO and DELETE should be applied", List.of("INSERT_INTO", "DELETE"), appliedOps);
+  }
+
+  /**
+   * INSERT_AFTER targeting a deleted node should also be applied.
+   */
+  @Test
+  public void insertAfterDeletedNodeIsNotSkipped() {
+    final List<String> appliedOps = new ArrayList<>();
+    final StructuredItem targetItem = new ArrayObject(new QNm[] { new QNm("name") },
+                                                      new Sequence[] { new Str("test") });
+
+    final UpdateList updateList = new UpdateList();
+
+    // Add an INSERT_AFTER operation
+    updateList.append(new TestUpdateOp(OpType.INSERT_AFTER, targetItem, () -> appliedOps.add("INSERT_AFTER")));
+
+    // Add a DELETE operation for the same target
+    updateList.append(new TestUpdateOp(OpType.DELETE, targetItem, () -> appliedOps.add("DELETE")));
+
+    // Apply updates
+    updateList.apply();
+
+    // Both should be applied
+    assertEquals("Both INSERT_AFTER and DELETE should be applied", List.of("INSERT_AFTER", "DELETE"), appliedOps);
+  }
+
   /**
    * Simple test implementation of UpdateOp for testing purposes.
    */