[tarantool-patches] Re: [PATCH 1/4] sql: optimize compilation of SELECT COUNT(*)

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

See my 1 comment below. And please, rebase on the latest 2.0 - maybe some parts of your
patchset will become simpler.

On 23/04/2018 23:29, Nikita Pettik wrote:
> Originally, SQLite chooses the best index to perform COUNT operation.
> In Tarantool there is no any considerable difference between them,
> so lets don't spend time on this routine and choose always primary index,
> in case of simple query 'SELECT COUNT(*) FROM <tab>'.
> Also, patch contains codestyle fixes.
> ---
>   src/box/sql/select.c      | 172 ++++++++++++++++++----------------------------
>   src/box/sql/vdbe.c        |   2 -
>   test/sql-tap/eqp.test.lua |   2 +-
>   3 files changed, 68 insertions(+), 108 deletions(-)
> 
> diff --git a/src/box/sql/select.c b/src/box/sql/select.c
> index d97e466b5..0df8a71d4 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -5294,25 +5296,23 @@ updateAccumulator(Parse * pParse, AggInfo * pAggInfo)
>   	}
>   }
>   
> -/*
> - * Add a single OP_Explain instruction to the VDBE to explain a simple
> - * count(*) query ("SELECT count(*) FROM pTab").
> +/**
> + * Add a single OP_Explain instruction to the VDBE to explain
> + * a simple count(*) query ("SELECT count(*) FROM <tab>").
> + *
> + * @param parse_context Current parsing context.
> + * @param table_name Name of table being queried.
>    */
>   #ifndef SQLITE_OMIT_EXPLAIN>   static void
> -explainSimpleCount(Parse * pParse,	/* Parse context */
> -		   Table * pTab,	/* Table being queried */
> -		   Index * pIdx)	/* Index used to optimize scan, or NULL */
> +explain_simple_count(struct Parse *parse_context, const char *table_name)
>   {
> -	if (pParse->explain == 2) {
> -		int bCover = (pIdx != 0 && !IsPrimaryKeyIndex(pIdx));
> -		char *zEqp = sqlite3MPrintf(pParse->db, "SCAN TABLE %s%s%s",

1. In Tarantool 'memtx' count() is not a scan - it is O(1) operation, that simply
gets current size of the primary index B+ tree. Maybe worth show it in the
explanation.