Re: [Tarantool-patches] [PATCH] sql: raise an error on attempt to use HASH index in SQL

[Nikita Pettik <korablev@tarantool.org>]

On 16 Jun 18:06, Kirill Yukhin wrote:
> Since currently query planner is unable to use HASH indexes
> and attempt to use it will likely lead to SEGFAULT, this
> patch raises an error on attempt to open VDBE cursor
> against HASH index.
> 
> Closes #4659
> ---
> 
> ChangeLog entry:
> ## Bugs fixed
> ### SQL
> * Block using HASH indexes in SQL since scheduler

scheduler -> planner.

> is unable  to use it properly (gh-4659).

Not only hash indexes, but any other except for tree indexes.
 
> Issue: https://github.com/tarantool/tarantool/issues/4659
> Branch: https://github.com/tarantool/tarantool/tree/kyukhin/gh-4659-block-hash-index

Tests fail on your branch. See https://travis-ci.org/github/tarantool/tarantool/jobs/698493716

>  src/box/sql/build.c                            |  6 +++++
>  test/sql-tap/gh-4659-block-hash-index.test.lua | 35 ++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
>  create mode 100755 test/sql-tap/gh-4659-block-hash-index.test.lua
> 
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 0b60d2e..b429a4e 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -722,6 +722,12 @@ vdbe_emit_open_cursor(struct Parse *parse_context, int cursor, int index_id,
>  		      struct space *space)
>  {
>  	assert(space != NULL);
> +	if (space->index[index_id]->def->type != TREE) {
> +		diag_set(ClientError, ER_UNSUPPORTED, "SQL",
> +			 "using of non-TREE index type");

Nit: I guess 'of' is reundant in this sentence. Also I'd include
hint in error message like: "... type. Please, use INDEXED BY clause
to force using proper index."

> diff --git a/test/sql-tap/gh-4659-block-hash-index.test.lua b/test/sql-tap/gh-4659-block-hash-index.test.lua
> new file mode 100755
> index 0000000..8c84e2d
> --- /dev/null
> +++ b/test/sql-tap/gh-4659-block-hash-index.test.lua
> @@ -0,0 +1,35 @@
> +#!/usr/bin/env tarantool
> +test = require("sqltester")
> +test:plan(2)
> +
> +-- gh-4782 - Raise an error in case space features HASH index.
> +-- Make sure that in case of attempt to use HASH index
> +-- from within SQL statement - an error is raised.
> +-- This is actually a stub until we are unable to employ
> +-- HASH index while planning a query.
> +
> +f = {
> +    {'1', 'unsigned'},
> +    {'2', 'string'},
> +}
> +
> +s = box.schema.create_space("T1", {format = f})
> +s:create_index('PK', {type = 'hash', parts = {'1'}})
> +
> +test:do_catchsql_test(
> +    "blocked-hash-index-1",
> +    "SELECT * FROM T1", {
> +        1, "SQL does not support using of non-TREE index type"
> +    })
> +
> +s = box.schema.create_space("T2", {format = f})
> +s:create_index('PK', {parts = {'2'}})
> +s:create_index('SK', {type = 'hash', parts = {'1'}})

Add a couple of tests covering other index types.

> +test:do_catchsql_test(
> +    "blocked-hash-index-1",
> +    "SELECT * FROM T2 INDEXED BY SK", {
> +        1, "SQL does not support using of non-TREE index type"
> +    })
> +
> +test:finish_test()
> -- 
> 1.8.3.1
>