[tarantool-patches] Re: [PATCH v2 3/7] box: create new methods for ephemeral spaces
- From: Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx>
- To: tarantool-patches@xxxxxxxxxxxxx, Mergen Imeev <imeevma@xxxxxxxxxxxxx>
- Date: Wed, 18 Jul 2018 14:52:52 +0300
/**
+ * Fill up request according to given
+ * operation type. For all cases but
+ * UPSERT argument name match names of
+ * fields of struct request. For UPSERT
+ * arguments key and key_end are fields
+ * ops and ops_end.
+ */
1. Please, write doxygen-style comment with right margin 66.
The other nitpicks are similar: you should be at least consistent with functions
comments: I mean, memtx_space_ephemeral_replace has _invalid_ comment, but at
the same time memtx_space_execute_upsert doesn't have comment at all.
Please, clean-up this by your own.
+memtx_space_ephemeral_replace(struct space *space, struct txn *txn,
+ struct request *request, struct tuple **result)
{
- struct memtx_space *memtx_space = (struct memtx_space *)space;
- struct tuple *new_tuple = memtx_tuple_new(space->format, tuple,
- tuple_end);
- if (new_tuple == NULL)
- return -1;
- tuple_ref(new_tuple);
+ assert(txn == NULL);
+ (void)txn;
+ struct tuple *new_tuple = NULL;
struct tuple *old_tuple = NULL;
- if (memtx_space->replace(space, old_tuple, new_tuple,
- DUP_REPLACE_OR_INSERT, &old_tuple) != 0) {
+ int rc = memtx_space_replace_impl(space, request, &new_tuple,
+ &old_tuple, result);
+ if (rc && new_tuple != NULL)
2. rc != 0
3. Looks like you introduce interesting functional changes. They deserve to be
tested, don't
you think? Or there is no way to test this new API yet? I don't insist on
C-module tests if this
the only way to test it.
Other related posts:
