From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <kshcherbatov@tarantool.org>
Subject: Re: [tarantool-patches] Re: [PATCH v5 08/12] box: introduce JSON
 indexes
References: <cover.1540795996.git.kshcherbatov@tarantool.org>
 <5c8cb44f2087518677e60cef442f1173e1d2e70a.1540795996.git.kshcherbatov@tarantool.org>
 <20181120165228.q3sur2j3lxf2m4fm@esperanza>
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Message-ID: <c855c67d-3527-59fe-3567-f5bdecb12cdf@tarantool.org>
Date: Mon, 26 Nov 2018 13:50:14 +0300
MIME-Version: 1.0
In-Reply-To: <20181120165228.q3sur2j3lxf2m4fm@esperanza>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
To: tarantool-patches@freelists.org, Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Kostya Osipov <kostja@tarantool.org>
List-ID: <tarantool-patches.dev.tarantool.org>

> You didn't patch this function, but you probably had to.
> Here's something for you to fix:
> 
> box.cfg{}
> s = box.schema.space.create('test')
> i = s:create_index('pk', {parts = {{1, 'integer', path = '[1]'}}})
> s:insert{{-1}}
> i:alter{parts = {{1, 'string', path = '[1]'}}} -- success
> s:insert{{'a'}} -- crash
Thank you for this case. I've introduced new checks in tuple_format1_can_store_format2_tuples
and have written few new tests in test sute.


bool
tuple_format1_can_store_format2_tuples(struct tuple_format *format1,
				       struct tuple_format *format2)
{
	if (format1->exact_field_count != format2->exact_field_count)
		return false;
	struct tuple_field *field1;
	struct json_token *field2_prev_token = NULL;
	struct json_token *skip_root_token = NULL;
	struct json_token *field1_prev_token = &format1->tree.root;
	json_tree_foreach_entry_preorder(field1, &format1->tree.root,
					 struct tuple_field, token) {
		/* Test if subtree skip is required. */
		if (skip_root_token != NULL) {
			struct json_token *tmp = &field1->token;
			while (tmp->parent != NULL &&
			       tmp->parent != skip_root_token)
				tmp = tmp->parent;
			if (tmp->parent == skip_root_token)
				continue;
		}
		skip_root_token = NULL;
		/* Lookup for a valid parent node in new tree. */
		while (field1_prev_token != field1->token.parent) {
			field1_prev_token = field1_prev_token->parent;
			field2_prev_token = field2_prev_token->parent;
			assert(field1_prev_token != NULL);
		}
		struct tuple_field *field2 =
			json_tree_lookup_entry(&format2->tree, field2_prev_token,
						&field1->token,
						struct tuple_field, token);
		/*
		 * The field has a data type in format1, but has
		 * no data type in format2.
		 */
		if (field2 == NULL) {
			/*
			 * The field can get a name added
			 * for it, and this doesn't require a data
			 * check.
			 * If the field is defined as not
			 * nullable, however, we need a data
			 * check, since old data may contain
			 * NULLs or miss the subject field.
			 */
			if (field1->type == FIELD_TYPE_ANY &&
			    tuple_field_is_nullable(field1)) {
				skip_root_token = &field1->token;
				continue;
			} else {
				return false;
			}
		}
		if (! field_type1_contains_type2(field1->type, field2->type))
			return false;
		/*
		 * Do not allow transition from nullable to non-nullable:
		 * it would require a check of all data in the space.
		 */
		if (tuple_field_is_nullable(field2) &&
		    !tuple_field_is_nullable(field1))
			return false;

		field2_prev_token = &field2->token;
		field1_prev_token = &field1->token;
	}
	return true;
}



-- incompatible format change
s = box.schema.space.create('test')
i = s:create_index('pk', {parts = {{1, 'integer', path = '[1]'}}})
s:insert{{-1}}
i:alter{parts = {{1, 'string', path = '[1]'}}}
s:insert{{'a'}}
i:drop()
i = s:create_index('pk', {parts = {{1, 'integer', path = '[1].FIO'}}})
s:insert{{{FIO=-1}}}
i:alter{parts = {{1, 'integer', path = '[1][1]'}}}
i:alter{parts = {{1, 'integer', path = '[1].FIO[1]'}}}
s:drop()