From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 09/12] vinyl: use source tuple format when copying field map Date: Sun, 1 Apr 2018 12:05:36 +0300 Message-Id: <616d69802cc20179aad60f1ab3b79b6bcd498806.1522572161.git.vdavydov.dev@gmail.com> In-Reply-To: References: In-Reply-To: References: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: There are two functions in vy_stmt.c that blindly copy tuple field map, vy_stmt_dup() and vy_stmt_replace_from_upsert(). Both these functions take a tuple format to use for the new statement and require this format to be the same as the source tuple format in terms of fields definition, otherwise they'll just crash. The only reason why we did that is that back when these functions were written we used a separate format for UPSERT statements so we needed this extra argument for creating a REPLACE from UPSERT. Now it's not needed, and we can use the source tuple format instead. Moreover, passing the current tuple format to any of those functions is even harmful, because tuple format can be extended by ALTER, in which case these functions will crash if called on a statement created before ALTER. That being said, let's drop the tuple format argument. --- src/box/tuple_format.c | 18 ------------------ src/box/tuple_format.h | 8 -------- src/box/vy_lsm.c | 2 +- src/box/vy_mem.c | 2 +- src/box/vy_point_lookup.c | 3 +-- src/box/vy_stmt.c | 18 +++++------------- src/box/vy_stmt.h | 6 ++---- src/box/vy_upsert.c | 4 ++-- 8 files changed, 12 insertions(+), 49 deletions(-) diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c index e458f49a..d3a7702d 100644 --- a/src/box/tuple_format.c +++ b/src/box/tuple_format.c @@ -324,24 +324,6 @@ tuple_format1_can_store_format2_tuples(const struct tuple_format *format1, return true; } -bool -tuple_format_eq(const struct tuple_format *a, const struct tuple_format *b) -{ - if (a->field_map_size != b->field_map_size || - a->field_count != b->field_count) - return false; - for (uint32_t i = 0; i < a->field_count; ++i) { - if (a->fields[i].type != b->fields[i].type || - a->fields[i].offset_slot != b->fields[i].offset_slot) - return false; - if (a->fields[i].is_key_part != b->fields[i].is_key_part) - return false; - if (a->fields[i].is_nullable != b->fields[i].is_nullable) - return false; - } - return true; -} - struct tuple_format * tuple_format_dup(struct tuple_format *src) { diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h index d35182df..7678b6a1 100644 --- a/src/box/tuple_format.h +++ b/src/box/tuple_format.h @@ -215,14 +215,6 @@ tuple_format1_can_store_format2_tuples(const struct tuple_format *format1, const struct tuple_format *format2); /** - * Check that two tuple formats are identical. - * @param a format a - * @param b format b - */ -bool -tuple_format_eq(const struct tuple_format *a, const struct tuple_format *b); - -/** * Register the duplicate of the specified format. * @param src Original format. * diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c index 88de1e61..915ffaeb 100644 --- a/src/box/vy_lsm.c +++ b/src/box/vy_lsm.c @@ -822,7 +822,7 @@ vy_lsm_commit_upsert(struct vy_lsm *lsm, struct vy_mem *mem, return; } - struct tuple *dup = vy_stmt_dup(stmt, lsm->mem_format); + struct tuple *dup = vy_stmt_dup(stmt); if (dup != NULL) { lsm->env->upsert_thresh_cb(lsm, dup, lsm->env->upsert_thresh_arg); diff --git a/src/box/vy_mem.c b/src/box/vy_mem.c index 9aaabf0b..22cd33fa 100644 --- a/src/box/vy_mem.c +++ b/src/box/vy_mem.c @@ -279,7 +279,7 @@ vy_mem_iterator_copy_to(struct vy_mem_iterator *itr, struct tuple **ret) assert(itr->curr_stmt != NULL); if (itr->last_stmt) tuple_unref(itr->last_stmt); - itr->last_stmt = vy_stmt_dup(itr->curr_stmt, tuple_format(itr->curr_stmt)); + itr->last_stmt = vy_stmt_dup(itr->curr_stmt); *ret = itr->last_stmt; if (itr->last_stmt != NULL) { vy_stmt_counter_acct_tuple(&itr->stat->get, *ret); diff --git a/src/box/vy_point_lookup.c b/src/box/vy_point_lookup.c index 0f4d75f7..0618590a 100644 --- a/src/box/vy_point_lookup.c +++ b/src/box/vy_point_lookup.c @@ -347,8 +347,7 @@ vy_point_lookup_apply_history(struct vy_lsm *lsm, if (vy_stmt_type(node->stmt) == IPROTO_DELETE) { /* Ignore terminal delete */ } else if (node->src_type == ITER_SRC_MEM) { - curr_stmt = vy_stmt_dup(node->stmt, - tuple_format(node->stmt)); + curr_stmt = vy_stmt_dup(node->stmt); } else { curr_stmt = node->stmt; tuple_ref(curr_stmt); diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c index f65b63c5..2229eb2e 100644 --- a/src/box/vy_stmt.c +++ b/src/box/vy_stmt.c @@ -109,22 +109,20 @@ vy_stmt_alloc(struct tuple_format *format, uint32_t bsize) } struct tuple * -vy_stmt_dup(const struct tuple *stmt, struct tuple_format *format) +vy_stmt_dup(const struct tuple *stmt) { /* * We don't use tuple_new() to avoid the initializing of * tuple field map. This map can be simple memcopied from * the original tuple. */ - struct tuple *res = vy_stmt_alloc(format, stmt->bsize); + struct tuple *res = vy_stmt_alloc(tuple_format(stmt), stmt->bsize); if (res == NULL) return NULL; assert(tuple_size(res) == tuple_size(stmt)); assert(res->data_offset == stmt->data_offset); memcpy(res, stmt, tuple_size(stmt)); res->refs = 1; - res->format_id = tuple_format_id(format); - assert(tuple_size(res) == tuple_size(stmt)); return res; } @@ -294,8 +292,7 @@ vy_stmt_new_insert(struct tuple_format *format, const char *tuple_begin, } struct tuple * -vy_stmt_replace_from_upsert(struct tuple_format *replace_format, - const struct tuple *upsert) +vy_stmt_replace_from_upsert(const struct tuple *upsert) { assert(vy_stmt_type(upsert) == IPROTO_UPSERT); /* Get statement size without UPSERT operations */ @@ -304,13 +301,8 @@ vy_stmt_replace_from_upsert(struct tuple_format *replace_format, assert(bsize <= upsert->bsize); /* Copy statement data excluding UPSERT operations */ - struct tuple_format *format = tuple_format_by_id(upsert->format_id); - /* - * In other fields the REPLACE tuple format must equal to - * the UPSERT tuple format. - */ - assert(tuple_format_eq(format, replace_format)); - struct tuple *replace = vy_stmt_alloc(replace_format, bsize); + struct tuple_format *format = tuple_format(upsert); + struct tuple *replace = vy_stmt_alloc(format, bsize); if (replace == NULL) return NULL; /* Copy both data and field_map. */ diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h index 9e73b2ca..e53f98ce 100644 --- a/src/box/vy_stmt.h +++ b/src/box/vy_stmt.h @@ -209,7 +209,7 @@ vy_tuple_delete(struct tuple_format *format, struct tuple *tuple); * @return new statement of the same type with the same data. */ struct tuple * -vy_stmt_dup(const struct tuple *stmt, struct tuple_format *format); +vy_stmt_dup(const struct tuple *stmt); struct lsregion; @@ -504,14 +504,12 @@ vy_stmt_new_upsert(struct tuple_format *format, /** * Create REPLACE statement from UPSERT statement. * - * @param replace_format Format for new REPLACE statement. * @param upsert Upsert statement. * @retval not NULL Success. * @retval NULL Memory error. */ struct tuple * -vy_stmt_replace_from_upsert(struct tuple_format *replace_format, - const struct tuple *upsert); +vy_stmt_replace_from_upsert(const struct tuple *upsert); /** * Extract MessagePack data from the REPLACE/UPSERT statement. diff --git a/src/box/vy_upsert.c b/src/box/vy_upsert.c index 2b38139a..67f9ef09 100644 --- a/src/box/vy_upsert.c +++ b/src/box/vy_upsert.c @@ -140,7 +140,7 @@ vy_apply_upsert(const struct tuple *new_stmt, const struct tuple *old_stmt, /* * INSERT case: return new stmt. */ - return vy_stmt_replace_from_upsert(format, new_stmt); + return vy_stmt_replace_from_upsert(new_stmt); } /* @@ -244,7 +244,7 @@ check_key: * @retval the old stmt. */ tuple_unref(result_stmt); - result_stmt = vy_stmt_dup(old_stmt, format); + result_stmt = vy_stmt_dup(old_stmt); } return result_stmt; } -- 2.11.0