[Tarantool-patches] [PATCH] sql: forbid data changes in read-only mode
Mergen Imeev
imeevma at tarantool.org
Fri Nov 20 16:47:37 MSK 2020
Hi! Thank you for the fixes. I have one unimportant comment: next time please
include link to branch
https://github.com/tarantool/tarantool/tree/sergos/gh-5231-box-execute-writes-in-ro
instead of link to compare
https://github.com/tarantool/tarantool/compare/sergos/gh-5231-box-execute-writes-in-ro
LGTM.
On Thu, Nov 19, 2020 at 10:41:23AM +0300, Sergey Ostanevich wrote:
> Hi!
>
> Thanks for review! Updated, force-pushed, new patch attached.
>
> Sergos
>
> > On 17 Nov 2020, at 00:09, Nikita Pettik <korablev at tarantool.org> wrote:
> >
> > On 16 Nov 23:36, Sergey Ostanevich wrote:
> >> diff --git a/test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua b/test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua
> >> new file mode 100755
> >> index 000000000..6d95adc2e
> >> --- /dev/null
> >> +++ b/test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua
> >> @@ -0,0 +1,105 @@
> >> +#!/usr/bin/env tarantool
> >> +
> >> +test = require("sqltester")
> >> +test:plan(9)
> >> +
> >> +local expected_err = "Can't modify data because this instance is in read-only mode."
> >> +
> >> +test:execsql([[
> >> + CREATE TABLE TEST (A INT, B INT, PRIMARY KEY (A));
> >> + INSERT INTO TEST (A, B) VALUES (3, 3);
> >> +]])
> >> +
> >> +box.cfg{read_only = true}
> >> +
> >> +test:do_catchsql_test(
> >> + "sql-errors-1.1",
> >> + [[
> >> + INSERT INTO TEST (A, B) VALUES (1, 1);
> >> + ]], {
> >> + -- <sql-errors-1.1>
> >
> > These comments are artifacts from porting SQLite test suite with
> > auto-tools, they are not really needed in new tests :)
> >
> =============
>
> From 4804ac485b38ddd280735970f6b9422bad1b30b6 Mon Sep 17 00:00:00 2001
> From: Sergey Ostanevich <sergos at tarantool.org>
> Date: Tue, 10 Nov 2020 13:52:10 +0300
> Subject: [PATCH] sql: forbid data changes in read-only mode
>
> A number of places in sql.c uses direct access to box_process_rw() that
> does not check read-only setting. Fixed by use of an intended interface
> of box_process1().
>
> Closes #5231
> ---
>
> Branch: https://github.com/tarantool/tarantool/compare/sergos/gh-5231-box-execute-writes-in-ro
> Issue: https://github.com/tarantool/tarantool/issues/5231
>
> @ChangeLog
> - Data changes are blocked for SQL if node is read-only (gh-5231).
>
> src/box/sql.c | 8 +-
> .../gh-5231-box-execute-writes-in-ro.test.lua | 87 +++++++++++++++++++
> 2 files changed, 91 insertions(+), 4 deletions(-)
> create mode 100755 test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua
>
> diff --git a/src/box/sql.c b/src/box/sql.c
> index a551bffc3..c55d0f4e5 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -453,7 +453,7 @@ insertOrReplace(struct space *space, const char *tuple, const char *tuple_end,
> request.space_id = space->def->id;
> request.type = type;
> mp_tuple_assert(request.tuple, request.tuple_end);
> - return box_process_rw(&request, space, NULL);
> + return box_process1(&request, NULL);
> }
>
> int tarantoolsqlInsert(struct space *space, const char *tuple,
> @@ -531,7 +531,7 @@ sql_delete_by_key(struct space *space, uint32_t iid, char *key,
> request.space_id = space->def->id;
> request.index_id = iid;
> assert(space_index(space, iid)->def->opts.is_unique);
> - return box_process_rw(&request, space, &unused);
> + return box_process1(&request, &unused);
> }
>
> /*
> @@ -597,7 +597,7 @@ int tarantoolsqlClearTable(struct space *space, uint32_t *tuple_count)
> request.key = tuple_extract_key(tuple, pk->def->key_def,
> MULTIKEY_NONE, &key_size);
> request.key_end = request.key + key_size;
> - rc = box_process_rw(&request, space, &unused);
> + rc = box_process1(&request, &unused);
> if (rc != 0) {
> iterator_delete(iter);
> return -1;
> @@ -834,7 +834,7 @@ tarantoolsqlIncrementMaxid(uint64_t *space_max_id)
> request.key_end = key + sizeof(key);
> request.type = IPROTO_UPDATE;
> request.space_id = space_schema->def->id;
> - if (box_process_rw(&request, space_schema, &res) != 0 || res == NULL ||
> + if (box_process1(&request, &res) != 0 || res == NULL ||
> tuple_field_u64(res, 1, space_max_id) != 0)
> return -1;
> return 0;
> diff --git a/test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua b/test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua
> new file mode 100755
> index 000000000..05c23c2f8
> --- /dev/null
> +++ b/test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua
> @@ -0,0 +1,87 @@
> +#!/usr/bin/env tarantool
> +
> +test = require("sqltester")
> +test:plan(9)
> +
> +local expected_err = "Can't modify data because this instance is in read-only mode."
> +
> +test:execsql([[
> + CREATE TABLE TEST (A INT, B INT, PRIMARY KEY (A));
> + INSERT INTO TEST (A, B) VALUES (3, 3);
> +]])
> +
> +box.cfg{read_only = true}
> +
> +test:do_catchsql_test(
> + "gh-5231-1",
> + [[
> + INSERT INTO TEST (A, B) VALUES (1, 1);
> + ]], {
> + 1, expected_err
> + })
> +
> +test:do_catchsql_test(
> + "gh-5231-2",
> + [[
> + DELETE FROM TEST;
> + ]], {
> + 1, expected_err
> + })
> +
> +test:do_catchsql_test(
> + "gh-5231-3",
> + [[
> + REPLACE INTO TEST VALUES (1, 2);
> + ]], {
> + 1, expected_err
> + })
> +
> +test:do_catchsql_test(
> + "gh-5231-4",
> + [[
> + UPDATE TEST SET B=4 WHERE A=3;
> + ]], {
> + 1, expected_err
> + })
> +
> +test:do_catchsql_test(
> + "gh-5231-5",
> + [[
> + TRUNCATE TABLE TEST;
> + ]], {
> + 1, expected_err
> + })
> +
> +test:do_catchsql_test(
> + "gh-5231-6",
> + [[
> + CREATE TABLE TEST2 (A INT, PRIMARY KEY (A));
> + ]], {
> + 1, expected_err
> + })
> +
> +test:do_catchsql_test(
> + "gh-5231-7",
> + [[
> + ALTER TABLE TEST ADD CONSTRAINT UK_CON UNIQUE (B);
> + ]], {
> + 1, expected_err
> + })
> +
> +test:do_catchsql_test(
> + "gh-5231-8",
> + [[
> + ALTER TABLE TEST RENAME TO TEST2;
> + ]], {
> + 1, expected_err
> + })
> +
> +test:do_catchsql_test(
> + "gh-5231-9",
> + [[
> + DROP TABLE TEST;
> + ]], {
> + 1, expected_err
> + })
> +
> +test:finish_test()
> --
> 2.24.3 (Apple Git-128)
>
>
More information about the Tarantool-patches
mailing list