Add Delete functionality #15
Conversation
| batchQueries.addAll(vssPutRecords.stream() | ||
| .map(vssRecord -> buildPutObjectQuery(dsl, vssRecord)).toList()); | ||
| batchQueries.addAll(vssDeleteRecords.stream() | ||
| .map(vssRecord -> buildDeleteObjectQuery(dsl, vssRecord)).toList()); |
There was a problem hiding this comment.
Does there need to be a check for distinct keys across both lists?
There was a problem hiding this comment.
We will implement request validation as part of api class and not db-implementation.
Currently we assume all requests are valid and there are couple of pending request validations to be added.
For example, we are also not checking if keys in put_items are unique currently or size or number of keys. (will work on this separately)
|
Ready for review ! :D |
| // Multiple items in the `delete_items` field, along with the `transaction_items`, are written in a | ||
| // database transaction in an all-or-nothing fashion. | ||
| // | ||
| // All items within a single `PutObjectRequest` must have distinct keys. |
There was a problem hiding this comment.
No need to do so in this PR, but we should be consistent with using ticks for identifiers. Or possibly not use them at all, for that matter. In Rust, they are used to format the markdown. But that's not the case for proto docs, presumably.
There was a problem hiding this comment.
Opening another PR to consistently add ticks around all identifiers.
| .map(kv -> buildVssRecord(storeId, kv)).toList()); | ||
|
|
||
| if (request.hasGlobalVersion()) { | ||
| VssDbRecord globalVersionRecord = buildVssRecord(storeId, | ||
| KeyValue.newBuilder() | ||
| .setKey(GLOBAL_VERSION_KEY) | ||
| .setVersion(request.getGlobalVersion()) | ||
| .setValue(ByteString.EMPTY) |
There was a problem hiding this comment.
Is there different behavior if this is omitted?
There was a problem hiding this comment.
Not really,
it was just some inconcstency that i noticed and just to make it more explicit.
We add delete functionality as both:
This follows similar pattern followed in popular KVStore Databases, such as aws-dynamodb which allows for both delete api and transaction write api.
For now, ignore the naming for "PutObject" API, will rename it to WriteObject in separate rename-commit.