Tarantool development patches archive
 help / color / mirror / Atom feed
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

  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