Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Add Document Deletion with a Key #150

Merged
merged 16 commits into from
Feb 2, 2022
Merged

feat: Add Document Deletion with a Key #150

merged 16 commits into from
Feb 2, 2022

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented Jan 28, 2022 •
edited
Loading

This enables the deletion of a single document using their dockey.

The query will look like something like this:

mutation {
    delete_user(id: "bae-6a6482a8-24e1-5c73-a237-ca569e41507d") {
        deletedDocKey: _key
    }
}

Note: deletedDocKey is an alias of a deleted key.

This PR resolves and closes #32 and #163

Future Work:

  • Implement the deletion of multiple documents using multiple keys.
  • Implement the deletion of documents using filters.

@shahzadlone shahzadlone marked this pull request as draft January 28, 2022 16:37
@shahzadlone shahzadlone mentioned this pull request Jan 28, 2022
Closed
@shahzadlone
Remove patch stuff that isn't needed for delete doc.
f53a392 
TODO:

Incorporate the version fetcher logic in a modular way.
@codecov
Copy link

codecov bot commented Jan 28, 2022 •
edited
Loading

Codecov Report

Merging #150 (d6216b9) into develop (b636a9e) will increase coverage by 3.04%.
The diff coverage is 41.66%.

❗ Current head d6216b9 differs from pull request most recent head 54dbfad. Consider uploading reports for the commit 54dbfad to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #150      +/-   ##
===========================================
+ Coverage    58.76%   61.81%   +3.04%     
===========================================
  Files           89       83       -6     
  Lines         8578     8094     -484     
===========================================
- Hits          5041     5003      -38     
+ Misses        2993     2552     -441     
+ Partials       544      539       -5
Impacted Files Coverage Δ
datastores/badger/v3/datastore.go 33.55% <ø> (+0.70%) ⬆️
db/base/descriptions.go 94.44% <ø> (-5.56%) ⬇️
db/db.go 50.47% <ø> (+0.94%) ⬆️
document/document.go 63.45% <ø> (-1.14%) ⬇️
query/graphql/planner/dagscan.go 72.28% <ø> (ø)
query/graphql/planner/datasource.go 55.88% <0.00%> (ø)
query/graphql/planner/update.go 72.91% <ø> (-0.09%) ⬇️
query/graphql/schema/descriptions.go 85.32% <ø> (-0.82%) ⬇️
db/collection_update.go 40.36% <25.00%> (+0.13%) ⬆️
db/collection_delete.go 28.07% <28.07%> (ø)
... and 22 more

@shahzadlone
start implementing the deletion method.
ada89af 
Todo: use the version fetcher.
@shahzadlone shahzadlone self-assigned this Jan 28, 2022
@shahzadlone shahzadlone added the feature New feature or request label Jan 28, 2022
@shahzadlone shahzadlone added this to the DefraDB v0.2 milestone Jan 28, 2022
@shahzadlone shahzadlone linked an issue Jan 28, 2022 that may be closed by this pull request
Delete single doc op #32
Closed
@shahzadlone
Rough implementation
d80f9fd
@shahzadlone
Deleting one document works sortof...
fc57bed 
Todo: There is a bug where it still shows me the document exists.
@shahzadlone
comment out
f87f622
jsimnz
jsimnz reviewed Feb 1, 2022
View reviewed changes
db/collection_delete.go Outdated Show resolved Hide resolved
jsimnz
jsimnz reviewed Feb 1, 2022
View reviewed changes
db/collection_delete.go Outdated Show resolved Hide resolved
@shahzadlone shahzadlone marked this pull request as ready for review February 1, 2022 16:30
@shahzadlone shahzadlone requested a review from jsimnz February 1, 2022 16:43
@jsimnz jsimnz added area/query Related to the query component area/collections Related to the collections system labels Feb 2, 2022
jsimnz
jsimnz requested changes Feb 2, 2022
View reviewed changes
Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments and changes for the the deleteNode stuff

Makefile Show resolved Hide resolved
db/collection_delete.go Outdated Show resolved Hide resolved
query/graphql/planner/delete.go Show resolved Hide resolved
query/graphql/planner/delete.go Outdated Show resolved Hide resolved
query/graphql/planner/delete.go Outdated Show resolved Hide resolved
query/graphql/planner/delete.go Outdated Show resolved Hide resolved
query/graphql/planner/delete.go Outdated Show resolved Hide resolved
query/graphql/planner/update.go Outdated Show resolved Hide resolved
db/tests/mutation/simple/delete/simple_test.go Show resolved Hide resolved
query/graphql/planner/delete.go Show resolved Hide resolved
@shahzadlone shahzadlone force-pushed the lone/feat/delete-doc-new-implementation branch 4 times, most recently from d6216b9 to 54dbfad Compare February 2, 2022 09:08
jsimnz
jsimnz approved these changes Feb 2, 2022
View reviewed changes
Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Great work!

@shahzadlone shahzadlone merged commit 9d822c3 into develop Feb 2, 2022
@shahzadlone shahzadlone deleted the lone/feat/delete-doc-new-implementation branch February 2, 2022 09:16
@shahzadlone shahzadlone linked an issue Feb 2, 2022 that may be closed by this pull request
Test single delete doc operation #163
Closed
jsimnz pushed a commit that referenced this pull request Feb 7, 2022
@shahzadlone @jsimnz
feat: Add Document Deletion with a Key (#150)
48fa15b 
This enables the deletion of a single document using their dockey.

The query will look like something like this:

mutation {
    delete_user(id: "bae-6a6482a8-24e1-5c73-a237-ca569e41507d") {
        deletedDocKey: _key
    }
}
Note: deletedDocKey is an alias of a deleted key.

This PR resolves and closes #32 and #163

Future Work:

Implement the deletion of multiple documents using multiple keys.
Implement the deletion of documents using filters.
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
@shahzadlone
feat: Add Document Deletion with a Key (sourcenetwork#150)
26a7833 
This enables the deletion of a single document using their dockey.

The query will look like something like this:

mutation {
    delete_user(id: "bae-6a6482a8-24e1-5c73-a237-ca569e41507d") {
        deletedDocKey: _key
    }
}
Note: deletedDocKey is an alias of a deleted key.

This PR resolves and closes sourcenetwork#32 and sourcenetwork#163

Future Work:

Implement the deletion of multiple documents using multiple keys.
Implement the deletion of documents using filters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsimnz jsimnz jsimnz approved these changes

Assignees

@shahzadlone shahzadlone

Labels
area/collections Related to the collections system area/query Related to the query component feature New feature or request
Projects
None yet
Milestone
DefraDB v0.2
Development

Successfully merging this pull request may close these issues.

Test single delete doc operation Delete single doc op
2 participants
@shahzadlone @jsimnz