Add support for Put with secondary indices #13289
Conversation
|
This pull request was exported from Phabricator. Differential Revision: D68035089 |
Summary: The patch adds support for `Put` / `PutUntracked` to the secondary indexing logic. Similarly to `PutEntity` (see facebook#13180), calling these APIs automatically add or remove secondary index entries as needed in an atomic and transparent fashion. Differential Revision: D68035089
ltamasi
force-pushed
the
export-D68035089
branch
from
b106cf2 to
b6d9fdb
Compare
|
This pull request was exported from Phabricator. Differential Revision: D68035089 |
Summary: The patch adds support for `Put` / `PutUntracked` to the secondary indexing logic. Similarly to `PutEntity` (see facebook#13180), calling these APIs automatically add or remove secondary index entries as needed in an atomic and transparent fashion. Differential Revision: D68035089
ltamasi
force-pushed
the
export-D68035089
branch
from
b6d9fdb to
2b27598
Compare
|
This pull request was exported from Phabricator. Differential Revision: D68035089 |
Summary: The patch adds support for `Put` / `PutUntracked` to the secondary indexing logic. Similarly to `PutEntity` (see facebook#13180), calling these APIs automatically add or remove secondary index entries as needed in an atomic and transparent fashion. Differential Revision: D68035089
| typename AddPrimaryEntry> | ||
| Status PutlikeWithSecondaryIndices(ColumnFamilyHandle* column_family, | ||
| const Slice& key, | ||
| const Valuelike& valuelike, |
There was a problem hiding this comment.
nit: Not sure if there could be a better name. I've seenValueOrWideColumns in another place and wondering if we can be a bit more explicit like the other place.
Line 1278 in 6e97a81
(And name the function PutWithSecondaryIndicesImpl or something)
| @@ -403,11 +411,9 @@ class SecondaryIndexMixin : public Txn { | |||
| } | |||
| } | |||
|
|
|||
| auto primary_columns = get_primary_columns(valuelike); | |||
There was a problem hiding this comment.
I'm trying to understand the benefit of using template and combining code for plain value + wide columns.
If the value is a plain text slice, this will be guaranteed to a single column and FindPrimaryColumn won't be really necessary, right? We may just check if column's name is secondary_index->GetPrimaryColumnName().
I'm wondering if it's actually a bit simpler to have two different functions, one for slice value and one for WideColumns. And same for UpdatePrimaryColumnValues() that one takes a single column and the other multiple. Having Valuelike won't be necessary, too.
|
|
||
| constexpr bool assume_tracked = true; | ||
|
|
||
| return Txn::PutEntity(cfh, primary_key, primary_columns, |
There was a problem hiding this comment.
At this point, I believe that primary_columns are already copied, sorted and updated with the index. IIUC, Tx::PutEntity() uses WriteBatch::PutEntity() and we copy the columns and sort them again. Wondering if we can avoid doing that or I am misunderstanding something.
Summary: The patch adds support for
Put/PutUntrackedto the secondary indexing logic. Similarly toPutEntity(see #13180), calling these APIs automatically add or remove secondary index entries as needed in an atomic and transparent fashion.Differential Revision: D68035089