Skip to content

fix(expression): support DELETE targeting a set in a list element (#812) - #890

Draft
Dodothereal wants to merge 4 commits into
ExtendDB:mainfrom
Dodothereal:fix/812-add-delete-list-element-set
Draft

Dodothereal wants to merge 4 commits into
ExtendDB:mainfrom
Dodothereal:fix/812-add-delete-list-element-set

Conversation

@Dodothereal

@Dodothereal Dodothereal commented Nov 6, 2026 •

Copy link
Copy Markdown

What changes

When an UpdateExpression uses DELETE targeting a set inside a list element (e.g. DELETE l[0] :v), ExtendDB previously silently treated the path as absent and returned 200 without mutating the set.

This PR updates apply_delete in crates/core/src/expression/update_evaluator.rs by introducing a DeleteTarget abstraction that permits the target attribute to reside either in a Map (DeleteTarget::Map) or within a List element (DeleteTarget::List).

Partially addresses #812 (draft for maintainer review; strictly not ready for merge due to unresolved behaviors noted below).

Scope & Overlap Avoidance with PR #596

This draft is intentionally narrowed to DELETE to eliminate overlap with open maintainer PR #596 (UpdateExpression ADD creates no missing map on its path):

  1. Zero Overlap on ADD: PR UpdateExpression ADD creates no missing map on its path #596 extensively refactors apply_add and introduces navigate_to_existing_parent_map. To avoid conflicting navigation logic and merge collisions, all ADD changes have been removed from this PR. apply_add and navigate_to_parent_map_or_create remain completely untouched and byte-identical to upstream/main.
  2. List Element ADD / Append Semantics: PR UpdateExpression ADD creates no missing map on its path #596 notes under What is not changed here:

    - ADD into a list element (ADD #l[0] :one). The service adds to the element, or appends when the index is past the end; ExtendDB refuses it, before and after this change, now with the service’s refusal text for a map parent.
    Because ADD append mechanics when an index is past the end are unmeasured here, omitting ADD leaves this behavior for unified maintainer handling alongside PR UpdateExpression ADD creates no missing map on its path #596.

  3. DELETE Through Missing / Wrong-Type Parent: PR UpdateExpression ADD creates no missing map on its path #596 notes that Amazon DynamoDB refuses DELETE through a missing or wrong-type parent on existing items with ValidationException (while answering OK on missing keys). ExtendDB historically treats it as a no-op in both cases because the evaluator does not track item existence (fix(update): write nothing for a removal-only UpdateItem on a missing key #508). This pre-existing limitation is preserved as a no-op in ExtendDB and is explicitly NOT claimed fixed here.

Critical Known Defect & Unresolved Edge Cases: NOT READY FOR MERGE

  • DELETE-All Leaves Original Nonempty Set Unchanged (Return 200 defect):
    • Issue UpdateItem: ADD l[0] :v is refused and DELETE l[0] :v changes nothing; the service applies both to the set in the list element #812 explicitly marks the case where DELETE removes all elements from a set inside a list as "not sent" / unmeasured against DynamoDB.
    • In ExtendDB, when DELETE removes all members of a set at a list index, DeleteTarget::remove is a no-op on lists. As a result, ExtendDB returns HTTP 200 while leaving the original nonempty set values completely unchanged. It does not delete the set, truncate the list, convert the element, or remove the list element.
    • Simply noting "list length unchanged" obscures this active defect: returning 200 with the original nonempty set elements untouched is unverified against DynamoDB wire behavior and incorrect.
    • Because of this unmeasured edge case and defective behavior, this PR is strictly an exploratory draft and is NOT ready for merge.

Measurements

As reported and measured against Amazon DynamoDB in us-east-1 on 2026-11-02 in issue #812 (table pk S HASH, initial item l2 holds l = [{"SS": ["a", "b"]}], ReturnValues: ALL_NEW):

  • DELETE l[0] :v with :v = {"SS": ["a"]}: DynamoDB returned 200, removing "a" to leave l = [{"SS": ["b"]}]. ExtendDB main returned 200 but left l = [{"SS": ["a", "b"]}] unchanged.

(Note: Live AWS measurements were not performed in this workspace; they are cited directly from the reporter's measurements in #812).

Tests & Validation

  • Unit tests added in crates/core/src/expression/update_evaluator_tests.rs:
    • delete_list_element_string_set_reporter_measurements: mirrors reporter measurement (DELETE l[0] :v on ["a", "b"] removing "a" -> ["b"]) with strict assert_eq!.
    • delete_list_element_number_and_binary_sets: validates NS and BS set operations targeting list indices with exact assert_eq! assertions.
    • delete_nested_map_list_paths: tests nested paths (DELETE m.l[0].tags :v) with strict equality assertions.
    • delete_list_type_mismatches_and_missing_paths: validates type mismatches reject with ValidationException, out-of-bounds list indices (l[99]) return Ok(()) no-op, and missing top-level parents return Ok(()) no-op.
  • Test suites executed on exact head daea918eb57844ad8ecbda93091bda0d369df45b (git rev-parse HEAD):
    • cargo test -p extenddb-core --lib expression::update_evaluator: 38 passed, 0 failed.
    • cargo test -p extenddb-core --lib: 801 passed, 0 failed.
    • cargo fmt --check: passed cleanly.
    • cargo clippy -p extenddb-core --lib -- -D warnings: passed cleanly.

Build & Lint Status (Pre-existing Clippy Blocker)

  • cargo clippy -p extenddb-core --lib -- -D warnings: passed cleanly with zero warnings.
  • cargo clippy -p extenddb-core --all-targets -- -D warnings: fails due to a pre-existing error in crates/core/tests/data_file_decode_peak.rs:83:9 (unused variable: line_len). This failure is byte-identical and pre-existing on upstream/main (d35d860) and is unrelated to this PR's expression evaluator changes.

Provenance & Disclosure

  • Developed with AI assistance (Gemini 3.8 Flash via T3 Code harness).
  • Verification was conducted purely locally on test fixtures; no live cloud resources or AWS accounts were utilized.

Checklist

  • I have read CONTRIBUTING.md
  • All core unit tests pass (cargo test -p extenddb-core --lib)
  • Code is formatted (cargo fmt --check)
  • Added unit tests with strict equality assertions
  • No new dependencies or cost/credential requirements
  • Draft PR for maintainer review (NOT ready for merge)

…#812)

- Narrow scope to DELETE targeting list element sets (e.g. DELETE l[0] :v).
- Exclude ADD changes to eliminate architectural overlap with open PR ExtendDB#596
  (which refactors missing-parent navigation in apply_add).
- Implement DeleteTarget container supporting Map and List leaf targets.
- Add strict unit tests for String, Number, and Binary sets in list elements,
  nested list/map paths, operand type mismatches, and pre-existing no-op paths.
@Dodothereal Dodothereal changed the title fix(expression): support ADD and DELETE targeting a set in a list element fix(expression): support DELETE targeting a set in a list element (#812) Nov 6, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant