[tarantool-patches] Re: [PATCH] sql: enable basic SELECTs for Lua created tables
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Wed Jun 27 15:23:48 MSK 2018
Hello. Thanks for the fixes! See 6 comments below.
All the comments except point 6 I have fixed and pushed as
a separate commit. Please, look and squash.
(I did not run tests, so maybe some of my fixes can break them).
>
> commit 93d8e00322a6822649d2358eceb3653632eb1d27
> Author: Kirill Yukhin <kyukhin at tarantool.org>
> Date: Tue Jun 26 09:58:17 2018 +0300
>
> sql: enable basic SELECTs for Lua created tables
>
> Update expander of SELECT statement to search for space if
> corresponding table wasn't found in table hash. Create dummy
> table if so and fill def field only.
>
> Also fix a leak in VIEW creation.
>
> Part of #3369
>
> diff --git a/src/box/sql/select.c b/src/box/sql/select.c
> index 368bcd6..755cb0c 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -4798,9 +4799,34 @@ selectExpander(Walker * pWalker, Select * p)
> /* An ordinary table or view name in the FROM clause */
> assert(pFrom->pTab == 0);
> pFrom->pTab = pTab =
> - sqlite3LocateTable(pParse, 0, pFrom->zName);
> - if (pTab == NULL)
> - return WRC_Abort;
> + sqlite3LocateTable(pParse, LOCATE_NOERR, pFrom->zName);
1. Multiple 80 symbols violation below.
> + if (pTab == NULL) {
> + const char *t_name = pFrom->zName;
> + int space_id = box_space_id_by_name(t_name,
> + strlen(t_name));
> + if (space_id == BOX_ID_NIL) {
> + sqlite3ErrorMsg(pParse,
> + "no such table: %s",
> + t_name);
> + return WRC_Abort;
> + }
> + struct space *space = space_by_id(space_id);
> +
> + if (space->def->field_count <= 0) {
> + sqlite3ErrorMsg(pParse,
> + "no format for space: %s",
> + t_name);
> + return WRC_Abort;
> + }
> + struct Table *tab = sqlite3DbMallocZero(db,
> + sizeof(Table));
> + tab->nTabRef = 1;
2. The tab == NULL is useless, if you use tab before the check.
> + if (tab == NULL)
> + return WRC_Abort;
> + tab->nTabRef = 1;
3. Second setting.
> + tab->def = space->def;
> + pFrom->pTab = pTab = tab;
> + }
> if (pTab->nTabRef >= 0xffff) {
4. This check makes no sense for just created dummy table.
5. Below the table is referenced again (it is necessary for non-dymmy tables),
so dummy tab leaks always: it has 1 ref on creation, 2 refs after ref below and
1 ref on parser finalization. Never 0.
> sqlite3ErrorMsg(pParse,
> "too many references to \"%s\": max 65535",
> diff --git a/test/sql-tap/lua-tables.test.lua b/test/sql-tap/lua-tables.test.lua
> new file mode 100755
> index 0000000..5ccb7bd
> --- /dev/null
> +++ b/test/sql-tap/lua-tables.test.lua
> @@ -0,0 +1,122 @@
> +#!/usr/bin/env tarantool
> +test = require("sqltester")
> +test:plan(8)
> +
> +test:do_test(
> + "lua-tables-prepare-1",
> + function()
> + format = {}
> + format[1] = { name = 'id', type = 'scalar'}
> + format[2] = { name = 'f2', type = 'scalar'}
> + s = box.schema.create_space('t', {format = format})
> + i = s:create_index('i', {parts={1, 'scalar'}})
> +
> + s:replace{1, 4}
> + s:replace{2, 2}
> + s:replace{3, 3}
> + s:replace{4, 3}
> +
> + s1 = box.schema.create_space('t1')
> + s1:create_index('i', {parts={1, 'scalar'}})
> + s1:replace{1, 1}
> + end,
> + {})
> +
> +test:do_execsql_test(
> + "lua-tables-2",
> + [[SELECT *, count(*)
> + FROM "t" as t1, "t" as t2
> + WHERE t1."id" = t2."f2"
> + ]],
> + {4, 3, 1, 4, 4})
> +
> +test:do_execsql_test(
> + "lua-tables-3",
> + [[create view v as SELECT * FROM "t";
> + select * from v;
> + ]],
> + {1, 4, 2, 2, 3, 3, 4, 3})
> +
> +test:do_catchsql_test(
> + "lua-tables-4",
> + [[SELECT * from t1]],
> + {1, "no such table: T1"}
> +)
> +
> +test:do_catchsql_test(
> + "lua-tables-5",
> + [[SELECT * from "t1"]],
> + {1, "no format for space: t1"}
> +)
> +-- Extract from tkt3527.test.lua
> +test:do_test(
> + "lua-tables-prepare-6",
> + function()
> + format = {{name = "CODE", type = "integer"},
> + {name = "NAME", type = "scalar"}}
> + s = box.schema.create_space("ELEMENT", {format = format})
> + s:create_index('pk', {parts = {1, 'scalar'}})
> + s:replace{1, 'Elem1'}
> + s:replace{2, 'Elem2'}
> + s:replace{3, 'Elem3'}
> + s:replace{4, 'Elem4'}
> + s:replace{5, 'Elem5'}
> +
> + format = {{name = "CODEOR", type = "scalar"},
> + {name = "CODE", type = "scalar"}}
> + s = box.schema.create_space("ELEMOR", {format = format})
> + s:create_index('pk', {parts = {1, 'scalar', 2, 'scalar'}})
> + s:replace{3, 4}
> + s:replace{4, 5}
> +
> + format = {{name = "CODEAND", type = "scalar"},
> + {name = "CODE", type = "scalar"},
> + {name = "ATTR1", type = "scalar"},
> + {name = "ATTR2", type = "scalar"},
> + {name = "ATTR3", type = "scalar"}}
> + s = box.schema.create_space("ELEMAND", {format = format})
> + s:create_index('pk', {parts = {1, "scalar", 2, "scalar"}})
> + s:replace{1, 3, 'a', 'b', 'c'}
> + s:replace{1, 2, 'x', 'y', 'z'}
> + end,
> + {})
> +
> +test:do_execsql_test(
> + "lua-tables-7",
> + [[CREATE VIEW ElemView1 AS
> + SELECT
> + CAST(Element.Code AS VARCHAR(50)) AS ElemId,
> + Element.Code AS ElemCode,
> + Element.Name AS ElemName,
> + ElemAnd.Code AS InnerCode,
> + ElemAnd.Attr1 AS Attr1,
> + ElemAnd.Attr2 AS Attr2,
> + ElemAnd.Attr3 AS Attr3,
> + 0 AS Level,
> + 0 AS IsOrElem
> + FROM Element JOIN ElemAnd ON ElemAnd.CodeAnd=Element.Code
> + WHERE ElemAnd.CodeAnd NOT IN (SELECT CodeOr FROM ElemOr)
> + UNION ALL
> + SELECT
> + CAST(ElemOr.CodeOr AS VARCHAR(50)) AS ElemId,
> + Element.Code AS ElemCode,
> + Element.Name AS ElemName,
> + ElemOr.Code AS InnerCode,
> + NULL AS Attr1,
> + NULL AS Attr2,
> + NULL AS Attr3,
> + 0 AS Level,
> + 1 AS IsOrElem
> + FROM ElemOr JOIN Element ON Element.Code=ElemOr.CodeOr
> + ORDER BY ElemId, InnerCode;]],
> + {})
> +
> +test:do_execsql_test(
> + "lua-tables-8",
> + [[SELECT * FROM ElemView1]],
> + {"1",1,"Elem1",2,"x","y","z",0,0,
> + "1",1,"Elem1",3,"a","b","c",0,0,
> + "3",3,"Elem3",4,"","","",0,1,
> + "4",4,"Elem4",5,"","","",0,1})
> +
> +test:finish_test()
>
6. So what about INDEXED BY? It leads to a crash? Or to an error?
Or what? A test is needed.
More information about the Tarantool-patches
mailing list