From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [94.100.177.96]) (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 13ACC46970E for ; Tue, 24 Dec 2019 18:34:43 +0300 (MSK) References: <0164c69c939fb61b7e6255c4cb695562d05fdef5.1576071711.git.korablev@tarantool.org> <20191224002656.GA37210@tarantool.org> From: Vladislav Shpilevoy Message-ID: <056f9213-5038-c096-2b0a-cf6c85a52a8a@tarantool.org> Date: Tue, 24 Dec 2019 16:34:41 +0100 MIME-Version: 1.0 In-Reply-To: <20191224002656.GA37210@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 6/6] sql: extend result set with alias List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org Thanks for the discussion! >>> diff --git a/test/sql/full_metadata.result b/test/sql/full_metadata.result >>> index 7c2982682..8f0bced36 100644 >>> --- a/test/sql/full_metadata.result >>> +++ b/test/sql/full_metadata.result >>> @@ -118,6 +121,35 @@ execute("SELECT * FROM t;") >>> | - [1, 1, 'aSd'] >>> | ... >>> >>> +-- Alias is always set in extended metadata. If column label in >>> +-- form of AS clause is set, then this alias is presented in >>> +-- metadata. Otherwise, alias is just the same as name. >> >> 4. But it is not true. The alias is not set always: >> >> ========================================================= >> >> tarantool> box.execute('PRAGMA full_metadata = true;') >> --- >> - row_count: 0 >> ... >> >> tarantool> box.execute('SELECT 1') >> --- >> - metadata: >> - name: '1' >> type: integer >> rows: >> - [1] >> ... >> >> ========================================================= >> >> As you can see, no AS = no alias. > > Always displaying alias (or label - call it whatever you want) > even in full_metadata mode doesn't seem to be reasonable for me. > Users can easily handle it in their code. Sending one map field > with string value duplicating the name and decoding it may affect > performance much more significant than one NULL check (IMHO). As I said already, I don't propose to send a duplicate name. I propose to handle it on the client side. In Lua you can push one string 2 times, and it will cost nothing - they will reference one string object. In IProto you still can send one name, when possible, and on the netbox side push both names. >> This is what I don't like >> in this patch the most. A user does not have a reliable always >> present column with a displayable name: either the column >> span or its alias after AS. He can't use name, because it >> changes its behavour depending on AS. And he can't use alias, >> because sometimes it is not present. > >> That leads to confusion, >> and necessity to branch in user code a lot. > > Yep, if user wants (for some reason; I don't have statistics how > often alias value can be useful or/and is used in production code) > to get alias value, one should make a small effort and add one > 'if' condition: > > if (is_full_metadata && alias == NULL) > alias = name; > > That's it. > Why, if we can remove that effort easily? I still don't see any real arguments why can't we always push a label. The argument about sending duplicate names is really weak - you can avoid sending two names, and push them on the client side, as I said above. No extra payload, not extra MessagePack decoding. Even the standard driver, to which you appeal so much, always returns a label. > Alternatively (I don't take into consideration option of swapping > name and alias as you suggest) we can change current default behaviour > and in 'casual metadata mode' return name as name (not alias): > > SELECT 1 AS x; -- would return '1' as name (now it returns 'x' as name). > > But I guess some users may turn out to be confused (in case they > haven't red about full_metadata pragma in docs). > >> The same example shows, that alias is a bad name. Here you need >> to return it, but it is not alias like AS. 'Label' would fit >> better here. > > I don't care much how to call it. If you prefer 'label', I will rename it. Yes, I prefer label. Not because this is my personal taste. I provided reasons, why it can't be called 'alias'. If you think alias is better, they say why. That is a discussion. > TBO I also don't care much which approach we should follow: always return > alias/label, fix current name/alias resolution, or swap name and alias > at all. If you insist on certain implementation, let's do it. If you > don't care much as well, let's ask smb else to take a final decision. > I insist on not breaking the current behaviour. The patch breaks it - 'name' behaves differently whether full_metadata is specified or not. Current clients may rely on the fact, that 'name' is always present, and that it behaves like now: returns either alias, or the original expression. Here are solutions with which I am good: - Keep the name as is, and add a new column meaning the original expression: 'orig_name', 'oname', 'span', 'column', 'expr', etc. In that case we don't break anything, we satisfy the driver, and 'full_metadata' only adds new fields. It does not change the behaviour of always present columns. - Or we could break the 'name' column a little, as you proposed - always return it as if AS is not specified, even in short metadata. In the full metadata it still will return the original expression, but we add a 'label' column, which returns either alias or name. In both solutions I want to rely on that both columns are always present on the client side, when full metadata is specified. Regardless of how are they sent in the binary protocol. Talking of asking somebody - I tried to get a final decision of what we should return from Alexander, because others just don't care. But the only thing he provides is samples of how other DBs behave. AFAIU, whatever we return and with whatever names, the driver will handle that. But how the driver will parse our protocol is not a public thing. I worry about public API, in Lua. Here the names and their behaviour matter.