From: Sergey Ostanevich <sergos@tarantool.org> To: Nikita Pettik <korablev@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH] sql: forbid data changes in read-only mode Date: Thu, 19 Nov 2020 10:41:23 +0300 [thread overview] Message-ID: <0174562A-F615-4DA8-B265-2124388131AE@tarantool.org> (raw) In-Reply-To: <20201116210931.GB13996@tarantool.org> Hi! Thanks for review! Updated, force-pushed, new patch attached. Sergos > On 17 Nov 2020, at 00:09, Nikita Pettik <korablev@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@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)
next prev parent reply other threads:[~2020-11-19 7:41 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-10 11:29 sergos 2020-11-13 14:01 ` Nikita Pettik 2020-11-16 10:31 ` Mergen Imeev 2020-11-16 20:36 ` Sergey Ostanevich 2020-11-16 21:09 ` Nikita Pettik 2020-11-19 7:41 ` Sergey Ostanevich [this message] 2020-11-20 13:47 ` Mergen Imeev 2020-11-22 14:55 ` Nikita 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=0174562A-F615-4DA8-B265-2124388131AE@tarantool.org \ --to=sergos@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH] sql: forbid data changes in read-only mode' \ /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