Tarantool development patches archive
 help / color / mirror / Atom feed
From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH v2 4/8] sql: get rid of SQL_FUNC_COUNT flag
Date: Tue, 13 Aug 2019 23:35:36 +0300	[thread overview]
Message-ID: <EAD66280-1CF2-4AE4-87BA-E81B3A067D6E@tarantool.org> (raw)
In-Reply-To: <2f18c9df1d818a222f2fb3005d13bd59228d4698.1565275469.git.kshcherbatov@tarantool.org>



> On 8 Aug 2019, at 17:50, Kirill Shcherbatov <kshcherbatov@tarantool.org> wrote:
> 
> Tarantool's SQL engine generates a different VDBE bytecode
> for ..COUNT(*).. and ..COUNT(fieldname).. operations:
> the first one produces a lightweight OP_Count operation that uses
> native mechanism to report the count of record in index while
> the second one pessimistically opens a space read iterator and
> uses Count aggregate function.
> 
> A helper routine is_simple_count decides whether such
> optimisation is correct. Using SQL_FUNC_COUNT flag to mark a
> dummy (non-functional)

Why do you consider it to be non-functional?

> function entry with 0 arguments is a bad practice

Why? Please, support this with arguments.

As I understand, the real reason is that we are striving
to remove function overloading, isn’t it?

> and should be reworked in relation with introducing
> a uniform functions cache in further patches.
> 
> Therefore SQL_FUNC_COUNT flag test in is_simple_count helper
> was replaced with more straightforward test with manually looking
> for count of function arguments passed.
> 
> Needed for #2200, #4113, #2233
> ---
> 
> -	if ((agg_info->aFunc[0].pFunc->funcFlags & SQL_FUNC_COUNT) == 0)
> +	if (agg_info->aFunc->pExpr->x.pList != NULL &&
> +	    agg_info->aFunc->pExpr->x.pList->nExpr > 0)
> 		return NULL;

Hm, this code works just by accident :) There’s no check
that function is really COUNT, but we are lucky enough
and there’s no other aggregate function which can take
empty list of arguments...

> 	if (expr->flags & EP_Distinct)
> 		return NULL;
> -- 
> 2.22.0

  parent reply	other threads:[~2019-08-13 20:35 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-08 14:50 [tarantool-patches] [PATCH v2 0/8] sql: uniform SQL and Lua functions subsystem Kirill Shcherbatov
2019-08-08 14:50 ` [tarantool-patches] [PATCH v2 1/8] sql: remove SQL_PreferBuiltin flag Kirill Shcherbatov
2019-08-09 16:07   ` [tarantool-patches] " n.pettik
2019-08-12 21:58   ` Konstantin Osipov
2019-08-08 14:50 ` [tarantool-patches] [PATCH v2 2/8] sql: GREATEST, LEAST instead of MIN/MAX overload Kirill Shcherbatov
2019-08-09 17:37   ` [tarantool-patches] " n.pettik
2019-08-13  8:26     ` Kirill Shcherbatov
2019-08-12 21:59   ` Konstantin Osipov
2019-08-08 14:50 ` [tarantool-patches] [PATCH v2 3/8] sql: wrap all trim functions in dispatcher Kirill Shcherbatov
2019-08-09 18:05   ` [tarantool-patches] " n.pettik
2019-08-13  8:28     ` Kirill Shcherbatov
2019-08-13 22:19       ` n.pettik
2019-08-12 22:00   ` Konstantin Osipov
2019-08-08 14:50 ` [tarantool-patches] [PATCH v2 4/8] sql: get rid of SQL_FUNC_COUNT flag Kirill Shcherbatov
2019-08-12 22:01   ` [tarantool-patches] " Konstantin Osipov
2019-08-13 20:35   ` n.pettik [this message]
2019-08-14  7:25     ` Kirill Shcherbatov
2019-08-08 14:50 ` [tarantool-patches] [PATCH v2 5/8] sql: introduce a signature_mask for functions Kirill Shcherbatov
2019-08-12 22:04   ` [tarantool-patches] " Konstantin Osipov
2019-08-13  8:32     ` Kirill Shcherbatov
2019-08-13  8:44       ` Konstantin Osipov
2019-08-13 20:38   ` n.pettik
2019-08-14  7:21     ` Kirill Shcherbatov
2019-08-08 14:50 ` [tarantool-patches] [PATCH v2 6/8] sql: rename OP_Function to OP_BuiltinFunction Kirill Shcherbatov
2019-08-12 22:04   ` [tarantool-patches] " Konstantin Osipov
2019-08-13 20:36   ` n.pettik
2019-08-08 14:50 ` [tarantool-patches] [PATCH v2 7/8] sql: get rid of FuncDef function hash Kirill Shcherbatov
2019-08-12 22:11   ` [tarantool-patches] " Konstantin Osipov
2019-08-13  7:29     ` Kirill Shcherbatov
2019-08-13  8:42       ` Konstantin Osipov
2019-08-13  9:45         ` Kirill Shcherbatov
2019-08-13 20:40   ` n.pettik
2019-08-16 12:57     ` Kirill Shcherbatov
2019-08-20 16:06       ` n.pettik
2019-08-08 14:50 ` [tarantool-patches] [PATCH v2 8/8] box: get rid of box.internal.sql_function_create Kirill Shcherbatov
2019-08-13 20:43   ` [tarantool-patches] " n.pettik
2019-08-16 12:57     ` Kirill Shcherbatov
2019-08-20 19:36       ` n.pettik

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=EAD66280-1CF2-4AE4-87BA-E81B3A067D6E@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=kshcherbatov@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH v2 4/8] sql: get rid of SQL_FUNC_COUNT flag' \
    /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