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 BF2C846970E for ; Sun, 29 Dec 2019 15:58:27 +0300 (MSK) Date: Sun, 29 Dec 2019 15:58:25 +0300 From: Mergen Imeev Message-ID: <20191229125825.GC21920@tarantool.org> References: <9d98e29c55631d8fd3e817cd8bf44124303790e0.1577445318.git.imeevma@gmail.com> <20191227165522.GK18639@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20191227165522.GK18639@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v5 2/5] sql: remove PRAGMA "short_column_names" List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org Hi! Thank you for remarks. My answers below. On Fri, Dec 27, 2019 at 06:55:22PM +0200, Nikita Pettik wrote: > On 27 Dec 14:18, imeevma@tarantool.org wrote: > > The pragmas "short_column_names" and "full_column_names" allow us > > to use three ways to display the column name in metadata: > > 1) If both are turned off, then the column name was shown as it > > was written by the user. > > 2) If "short_column_names" = OFF and "full_column_names" = ON, > > then the column name is displayed as .. > > 3) If "short_column_names" = ON, then the column name is displayed > > as . This is the default option. > > Don't get the difference between first and last options. Also see > https://www.sqlite.org/pragma.html#pragma_full_column_names > Here you can see behaiour in the first case: tarantool> CREATE TABLE t(i INT PRIMARY KEY); --- - row_count: 1 ... tarantool> SELECT t . i from t; --- - metadata: - name: t . i type: integer rows: [] ... As we can see, the name is "t . i". In the third case: tarantool> SELECT t . i from t; --- - metadata: - name: I type: integer rows: [] ... > > But we need only two ways to show the column name: > > 1) Show the column name as . This should be the > > default option. > > 2) Show the column name as
.. > > > > In this regard, we need only one of these pragmas. > > > > @@ -1827,12 +1824,11 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList, > > assert(iCol >= 0 && iCol < (int)space_def->field_count); > > zCol = space_def->fields[iCol].name; > > const char *name = NULL; > > + > > > > Extra new line. Please, remove. > Fixed. > > if (pEList->a[i].zName != NULL) { > > name = pEList->a[i].zName; > > } else { > > - if (!shortNames && !fullNames) { > > - name = pEList->a[i].zSpan; > > - } else if (fullNames) { > > + if (pParse->sql_flags & SQL_FullColNames) { > > (pParse->sql_flags & SQL_FullColNames) != 0 > Fixed. > > name = tt_sprintf("%s.%s", > > space_def->name, > > zCol); > > @@ -2053,8 +2049,7 @@ sqlResultSetOfSelect(Parse * pParse, Select * pSelect) > > sql *db = pParse->db; > > > > uint32_t saved_flags = pParse->sql_flags; > > - pParse->sql_flags |= ~SQL_FullColNames; > > - pParse->sql_flags &= SQL_ShortColNames; > > + pParse->sql_flags = 0; > > Why do you set sql_flags to zero? > After "pParse->sql_flags &= SQL_ShortColNames;" all but one(SQL_ShortColNames) bits will be 0. Since we remove SQL_ShortColNames, all bits will be 0. > > sqlSelectPrep(pParse, pSelect, 0); > > if (pParse->is_aborted) > > return NULL; > > I comment your patch since I've faced the same necessity to remove > short_column_names. So to avoid doubling work, I'am going to cherry- > pick your patch (or push it to the master out of order). >