From: Vladimir Davydov <vdavydov.dev@gmail.com> To: kostja@tarantool.org Cc: tarantool-patches@freelists.org Subject: [PATCH 09/12] vinyl: use source tuple format when copying field map Date: Sun, 1 Apr 2018 12:05:36 +0300 [thread overview] Message-ID: <616d69802cc20179aad60f1ab3b79b6bcd498806.1522572161.git.vdavydov.dev@gmail.com> (raw) In-Reply-To: <cover.1522572160.git.vdavydov.dev@gmail.com> In-Reply-To: <cover.1522572160.git.vdavydov.dev@gmail.com> 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
next prev parent reply other threads:[~2018-04-01 9:05 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-04-01 9:05 [PATCH 00/12] vinyl: allow to extend key def of non-empty index Vladimir Davydov 2018-04-01 9:05 ` [PATCH 01/12] index: add commit_modify virtual method Vladimir Davydov 2018-04-01 9:05 ` [PATCH 02/12] alter: require rebuild of all secondary vinyl indexes if pk changes Vladimir Davydov 2018-04-01 9:05 ` [PATCH 03/12] alter: do not rebuild secondary indexes on compatible " Vladimir Davydov 2018-04-05 12:30 ` Konstantin Osipov 2018-04-01 9:05 ` [PATCH 04/12] space: make space_swap_index method virtual Vladimir Davydov 2018-04-01 9:05 ` [PATCH 05/12] vinyl: do not reallocate tuple formats on alter Vladimir Davydov 2018-04-01 11:12 ` [tarantool-patches] " v.shpilevoy 2018-04-01 11:24 ` Vladimir Davydov 2018-04-01 9:05 ` [PATCH 06/12] vinyl: zap vy_lsm_validate_formats Vladimir Davydov 2018-04-01 9:05 ` [PATCH 07/12] vinyl: zap vy_mem_update_formats Vladimir Davydov 2018-04-01 9:05 ` [PATCH 08/12] vinyl: remove pointless is_nullable initialization for disk_format Vladimir Davydov 2018-04-01 9:05 ` Vladimir Davydov [this message] 2018-04-01 9:05 ` [PATCH 10/12] vinyl: rename vy_log_record::commit_lsn to create_lsn Vladimir Davydov 2018-04-01 9:05 ` [PATCH 11/12] vinyl: do not write VY_LOG_DUMP_LSM record to snapshot Vladimir Davydov 2018-04-01 9:05 ` [PATCH 12/12] vinyl: allow to modify key definition if it does not require rebuild Vladimir Davydov 2018-04-02 8:46 ` Vladimir Davydov 2018-04-05 14:32 ` Konstantin Osipov 2018-04-05 14:45 ` Konstantin Osipov 2018-04-05 14:48 ` Konstantin Osipov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=616d69802cc20179aad60f1ab3b79b6bcd498806.1522572161.git.vdavydov.dev@gmail.com \ --to=vdavydov.dev@gmail.com \ --cc=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH 09/12] vinyl: use source tuple format when copying field map' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox