From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 751B82028B for ; Sat, 29 Dec 2018 10:28:31 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qPawWj84cYV4 for ; Sat, 29 Dec 2018 10:28:31 -0500 (EST) 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 turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 34B6A1FBDE for ; Sat, 29 Dec 2018 10:28:31 -0500 (EST) Subject: [tarantool-patches] Re: [PATCH v2 4/5] sql: encode tuples with mpstream on Vdbe run References: From: Kirill Shcherbatov Message-ID: <7a90ee7f-9180-a00e-8f99-129e2edf2735@tarantool.org> Date: Sat, 29 Dec 2018 18:28:28 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, Nikita Pettik Hi! Thank you for review. > Nit: ... and table has other records. > It is russian-like collocation, we can’t ‘make’ lookup. > "… Vdbe inspects parent table emitting OP_Found" (or smth like this) > Nit: must. > Nit: got rid of. > Nit: not NULL … or simply “Pointer to valid tuple." Fixed. Here and in previous patch. > Lets replace these if-else's with one switch. Unfortunately, it is not possible now, as we may have a flag combination (like OP_String+OP_Blob). You may take a look into Server chat for more details for this particular case, but it is not the one. Maybe we may improve it in future(when affinity would be reworked?) but not now and not as a part of this patch, I believe. > The rest is OK