Tarantool development patches archive
 help / color / mirror / Atom feed
From: Nikita Pettik <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: v.shpilevoy@tarantool.org, Nikita Pettik <korablev@tarantool.org>
Subject: [tarantool-patches] [PATCH 2/2] sql: don't change type of function's retval after codegen
Date: Mon, 11 Mar 2019 21:10:26 +0300	[thread overview]
Message-ID: <9a7a2794d68a949b801a3210231a35562fcf8847.1552327461.git.korablev@tarantool.org> (raw)
In-Reply-To: <cover.1552327461.git.korablev@tarantool.org>
In-Reply-To: <cover.1552327461.git.korablev@tarantool.org>

Proper type of function's returning value is set during names resolution
(i.e. promotion from struct FuncDef to struct Expr, see
resolveExprStep()). Accidentally, it was set again during byte-code
generation for VDBE. What is more, in some cases it was set to a wrong
type. For instance, built-in function randomblob() returns value to be
encoded as MP_BIN, so its returning type is SCALAR. However, it was
reset to INTEGER (as its first argument). This patch simply removes this
second type promotion.
---
 src/box/sql/expr.c       | 11 -----------
 test/sql/iproto.result   | 41 +++++++++++++++++++++++++++++++++++++++++
 test/sql/iproto.test.lua | 15 +++++++++++++++
 3 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index a75f23756..8efd0631a 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -4000,17 +4000,6 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 				sql_parser_error(pParse);
 				break;
 			}
-
-			if (pDef->ret_type != FIELD_TYPE_SCALAR) {
-				pExpr->type = pDef->ret_type;
-			} else {
-				/*
-				 * Otherwise, use first arg as
-				 * expression type.
-				 */
-				if (pFarg && pFarg->nExpr > 0)
-					pExpr->type = pFarg->a[0].pExpr->type;
-			}
 			/* Attempt a direct implementation of the built-in COALESCE() and
 			 * IFNULL() functions.  This avoids unnecessary evaluation of
 			 * arguments past the first non-NULL argument.
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index 3a77c8e93..26d55b60c 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -942,6 +942,47 @@ res.metadata
   - name: detail
     type: TEXT
 ...
+-- Make sure that built-in functions have a right returning type.
+--
+cn:execute("SELECT zeroblob(1);")
+---
+- metadata:
+  - name: zeroblob(1)
+    type: scalar
+  rows:
+  - ["\0"]
+...
+-- randomblob() returns different results each time, so check only
+-- type in meta.
+--
+res = cn:execute("SELECT randomblob(1);")
+---
+...
+res.metadata
+---
+- - name: randomblob(1)
+    type: scalar
+...
+-- Type set during compilation stage, and since min/max are accept
+-- arguments of all scalar type, we can't say nothing more than
+-- SCALAR.
+--
+cn:execute("SELECT min(1, 2, 3);")
+---
+- metadata:
+  - name: min(1, 2, 3)
+    type: scalar
+  rows:
+  - [1]
+...
+cn:execute("SELECT max(1, 2, 3);")
+---
+- metadata:
+  - name: max(1, 2, 3)
+    type: scalar
+  rows:
+  - [3]
+...
 cn:close()
 ---
 ...
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index fbdc5a2ae..89d04b638 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -289,6 +289,21 @@ res.metadata
 res = cn:execute("EXPLAIN QUERY PLAN SELECT COUNT(*) FROM t1")
 res.metadata
 
+-- Make sure that built-in functions have a right returning type.
+--
+cn:execute("SELECT zeroblob(1);")
+-- randomblob() returns different results each time, so check only
+-- type in meta.
+--
+res = cn:execute("SELECT randomblob(1);")
+res.metadata
+-- Type set during compilation stage, and since min/max are accept
+-- arguments of all scalar type, we can't say nothing more than
+-- SCALAR.
+--
+cn:execute("SELECT min(1, 2, 3);")
+cn:execute("SELECT max(1, 2, 3);")
+
 cn:close()
 box.sql.execute('DROP TABLE t1')
 
-- 
2.15.1

  parent reply	other threads:[~2019-03-11 18:10 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-11 18:10 [tarantool-patches] [PATCH 0/2] SQL types fixes Nikita Pettik
2019-03-11 18:10 ` [tarantool-patches] [PATCH 1/2] sql: make type in column-meta be consistent with NoSQL names Nikita Pettik
2019-03-21  9:03   ` [tarantool-patches] " Vladislav Shpilevoy
2019-03-21 14:00   ` Konstantin Osipov
2019-03-11 18:10 ` Nikita Pettik [this message]
2019-03-21  9:03   ` [tarantool-patches] Re: [PATCH 2/2] sql: don't change type of function's retval after codegen Vladislav Shpilevoy
2019-03-21 12:51     ` n.pettik
2019-03-21 14:46       ` Vladislav Shpilevoy
2019-04-03  7:58 ` [tarantool-patches] Re: [PATCH 0/2] SQL types fixes Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9a7a2794d68a949b801a3210231a35562fcf8847.1552327461.git.korablev@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [tarantool-patches] [PATCH 2/2] sql: don'\''t change type of function'\''s retval after codegen' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox