Repository navigation
fix(expression): support DELETE targeting a set in a list element (#812) - #890
Draft
Dodothereal wants to merge 4 commits into
Draft
Dodothereal wants to merge 4 commits into
Dodothereal wants to merge 4 commits into
Conversation
…d ADD out-of-bounds check
…#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.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes
When an
UpdateExpressionusesDELETEtargeting 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_deleteincrates/core/src/expression/update_evaluator.rsby introducing aDeleteTargetabstraction 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
DELETEto eliminate overlap with open maintainer PR #596 (UpdateExpression ADD creates no missing map on its path):apply_addand introducesnavigate_to_existing_parent_map. To avoid conflicting navigation logic and merge collisions, allADDchanges have been removed from this PR.apply_addandnavigate_to_parent_map_or_createremain completely untouched and byte-identical toupstream/main.DELETEthrough a missing or wrong-type parent on existing items withValidationException(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
ADD l[0] :vis refused andDELETE l[0] :vchanges nothing; the service applies both to the set in the list element #812 explicitly marks the case whereDELETEremoves all elements from a set inside a list as "not sent" / unmeasured against DynamoDB.DELETEremoves all members of a set at a list index,DeleteTarget::removeis 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.Measurements
As reported and measured against Amazon DynamoDB in
us-east-1on 2026-11-02 in issue #812 (tablepkS HASH, initial iteml2holdsl = [{"SS": ["a", "b"]}],ReturnValues: ALL_NEW):DELETE l[0] :vwith:v = {"SS": ["a"]}: DynamoDB returned 200, removing"a"to leavel = [{"SS": ["b"]}]. ExtendDB main returned 200 but leftl = [{"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
crates/core/src/expression/update_evaluator_tests.rs:delete_list_element_string_set_reporter_measurements: mirrors reporter measurement (DELETE l[0] :von["a", "b"]removing"a"->["b"]) with strictassert_eq!.delete_list_element_number_and_binary_sets: validatesNSandBSset operations targeting list indices with exactassert_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 withValidationException, out-of-bounds list indices (l[99]) returnOk(())no-op, and missing top-level parents returnOk(())no-op.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 incrates/core/tests/data_file_decode_peak.rs:83:9(unused variable: line_len). This failure is byte-identical and pre-existing onupstream/main(d35d860) and is unrelated to this PR's expression evaluator changes.Provenance & Disclosure
Checklist
cargo test -p extenddb-core --lib)cargo fmt --check)