From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (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 8C018469710 for ; Wed, 18 Nov 2020 19:15:20 +0300 (MSK) Date: Wed, 18 Nov 2020 16:15:19 +0000 From: Nikita Pettik Message-ID: <20201118161519.GA23479@tarantool.org> References: <20201009134529.13212-1-roman.habibov@tarantool.org> <20201009134529.13212-6-roman.habibov@tarantool.org> <20201106155719.GA13328@tarantool.org> <8AFFA6D9-CCF7-4134-9018-0C6389542ADE@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <8AFFA6D9-CCF7-4134-9018-0C6389542ADE@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4 5/5] sql: support column addition List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Khabibov Cc: tarantool-patches@dev.tarantool.org On 10 Nov 15:18, Roman Khabibov wrote: > >> diff --git a/src/box/sql/build.c b/src/box/sql/build.c > >> index fd8e7ed1e..597608bea 100644 > >> --- a/src/box/sql/build.c > >> +++ b/src/box/sql/build.c > >> @@ -285,48 +285,110 @@ sql_field_retrieve(Parse *parser, struct space_def *space_def, uint32_t id) > >> return field; > >> } > >> > >> +static struct space * > >> +sql_shallow_space_copy(struct Parse *parse, struct space *space) > >> +{ > >> + assert(space->def != NULL); > >> + struct space *ret = sql_ephemeral_space_new(parse, space->def->name); > >> + if (ret == NULL) > >> + goto error; > >> + ret->index_count = space->index_count; > >> + ret->index_id_max = space->index_id_max; > >> + size_t size = 0; > >> + ret->index = region_alloc_array(&parse->region, typeof(struct index *), > >> + ret->index_count, &size); > >> + if (ret->index == NULL) { > >> + diag_set(OutOfMemory, size, "region_alloc_array", "ret->index"); > >> + goto error; > >> + } > > > > > > You don't need all indexes from space..What you only need from space - > > count of indexes (to generate name if it is required). The same for > > field defs. Mb it is worth avoiding copying space at all - just > > store in create_column_def list of ck/fk constraints, single > > field_def, index/field count - these objects seem to be enough. > > Please consider this suggestion. > Yes, I understand that. But unfortunately, the current code, namely > the functions for creating indexes, fk, and ck constraints, are written > in such a way that they cannot be called without struct space (ephemeral > or not). Vlad and I discussed this and came to the conclusion that it is > better to go in the direction of reusing the code, rather than rewriting > a significant part of build.c. > > https://lists.tarantool.org/pipermail/tarantool-patches/2020-July/018460.html > https://lists.tarantool.org/pipermail/tarantool-patches/2020-August/019003.html Oh, ok, I won't insist on this refactoring. But could you please copy-paste referenced anwers from mailing list? Those letters quite large so navigating through them takes a lot of time. > >> + /* > >> + * If it is an , then we have to > >> + * create all indexes added by this statement. These > > > > AFAIK only one index (corresponding to unique constraint). > I can write any number of named unique constraints. They should be > created. > ALTER TABLE t ADD COLUMN a INT CONSTRAINT c_1 UNIQUE, CONSTRAINT c_2 UNIQUE … Well, ok. Just wondering why somebody might want to create several unique constraints over the same field.. Otherwise LGTM