From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 770B64696C3 for ; Fri, 10 Apr 2020 22:16:30 +0300 (MSK) References: <91166652f0b9cd151ca485538e6e895639c62acd.1585823952.git.imeevma@gmail.com> <20200410000445.GA14187@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Fri, 10 Apr 2020 21:16:28 +0200 MIME-Version: 1.0 In-Reply-To: <20200410000445.GA14187@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v1 1/1] sql: specify field types in ephemeral space format List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mergen Imeev Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the fixes! See 2 comments below. > diff --git a/src/box/sql.c b/src/box/sql.c > index 27089bd..288c4fa 100644 > --- a/src/box/sql.c > +++ b/src/box/sql.c > @@ -337,14 +337,15 @@ sql_ephemeral_space_create(uint32_t field_count, struct sql_key_info *key_info) > * length of each name is no more than strlen("_COLUMN_") > * + 5. > */ > - assert(SQL_MAX_COLUMN < 9999); > + assert(SQL_MAX_COLUMN <= 2000); > uint32_t name_len = sizeof("_COLUMN_") + 5; 1. sizeof("_COLUMN_") != strlen("_COLUMN_"). The latter is 8, the former is 9. Better use strlen(). > uint32_t size = field_count * (sizeof(struct field_def) + name_len + > sizeof(struct key_part_def)); > struct field_def *fields = region_alloc(region, size); > - struct key_part_def *parts = (void *)fields + > + struct key_part_def *ephemer_key_parts = (void *)fields + > field_count * sizeof(struct field_def); > - char *names = (char *)parts + field_count * sizeof(struct key_part_def); > + char *names = (char *)ephemer_key_parts + > + field_count * sizeof(struct key_part_def); > for (uint32_t i = 0; i < field_count; ++i) { > struct field_def *field = &fields[i]; > field->name = names; 2. It is worth extending the commit message with more details, how is it needed for 4256 and 4692, with all what we discussed in private. Please, do the things above and send to Nikita. Also, in case it will be pushed in this form to master, I propose you to create a ticket on ephemeral_space API rework. As you fairly noticed, we have issues with using sql_key_info as, in fact, a format definition. We basically *can't* index fields not added to the format.