Skip to content

Commit

Permalink
MDEV-33861 main.query_cache fails with embedded after enabling WITH_P…
Browse files Browse the repository at this point in the history
…ROTECT_STATEMENT_MEMROOT

Synopsis: If SELECT returned answer from Query Cache it is not really executed.

The reason for firing of assertion
  DBUG_ASSERT((mem_root->flags & ROOT_FLAG_READ_ONLY) == 0);
is that in case the query_cache is on and the same query run by different
stored routines the following use case can take place:
First, lets say that bodies of routines used by the test case are the same
and contains the only query 'SELECT * FROM t1';
  call p1() -- a result set is stored in query cache for further use.
  call p2() -- the same query is run against the table t1, that result in
               not running the actual query but using its cached result.
               On finishing execution of this routine, its memory root is
               marked for read only since every SP instruction that this
               routine contains has been executed.
  INSERT INT t1 VALUE (1); -- force following invalidation of query cache
  call p2() -- query the table t1 will result in assertion failure since its
               execution would require allocation on the memory root that
               has been already marked as read only memory root

The root cause of firing the assertion is that memory root of the stored
routine 'p2' was marked as read only although actual execution of the query
contained inside hadn't been performed.

To fix the issue, mark a SP instruction as not yet run in case its execution
doesn't result in real query processing and a result set got from query cache
instead.

Note that, this issue relates server built in debug mode AND with the protect
statement memory root feature turned on. It doesn't affect server built
in release mode.
  • Loading branch information
sanja-byelkin committed Apr 16, 2024
1 parent 051a1fa commit 50998a6
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 5 deletions.
27 changes: 27 additions & 0 deletions mysql-test/main/query_cache.result
Original file line number Diff line number Diff line change
Expand Up @@ -2207,6 +2207,33 @@ Variable_name Value
Qcache_queries_in_cache 0
DROP FUNCTION foo;
drop table t1;
#
# MDEV-33861: main.query_cache fails with embedded after
# enabling WITH_PROTECT_STATEMENT_MEMROOT
#
create table t1 (s1 int);
create procedure f3 () begin
select * from t1;
end;
//
create procedure f4 () begin
select * from t1;
end;
//
Call f4();
s1
cAll f3();
s1
insert into t1 values (2);
caLl f3();
s1
2
drop procedure f3;
drop procedure f4;
drop table t1;
#
# End of 10.4 tests
#
restore defaults
SET GLOBAL query_cache_type= default;
SET GLOBAL query_cache_size=@save_query_cache_size;
35 changes: 35 additions & 0 deletions mysql-test/main/query_cache.test
Original file line number Diff line number Diff line change
Expand Up @@ -1803,6 +1803,41 @@ show status like "Qcache_queries_in_cache";
DROP FUNCTION foo;
drop table t1;


--echo #
--echo # MDEV-33861: main.query_cache fails with embedded after
--echo # enabling WITH_PROTECT_STATEMENT_MEMROOT
--echo #

create table t1 (s1 int);
--delimiter //
create procedure f3 () begin
select * from t1;
end;
//
create procedure f4 () begin
select * from t1;
end;
//
--delimiter ;

Call f4();

cAll f3();

insert into t1 values (2);

caLl f3();

drop procedure f3;
drop procedure f4;
drop table t1;


--echo #
--echo # End of 10.4 tests
--echo #

--echo restore defaults
SET GLOBAL query_cache_type= default;
SET GLOBAL query_cache_size=@save_query_cache_size;
Expand Down
3 changes: 3 additions & 0 deletions sql/sp_head.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3702,6 +3702,9 @@ sp_instr_stmt::execute(THD *thd, uint *nextp)
thd->update_stats();
thd->lex->sql_command= save_sql_command;
*nextp= m_ip+1;
#ifdef PROTECT_STATEMENT_MEMROOT
mark_as_qc_used();
#endif
}
thd->set_query(query_backup);
thd->query_name_consts= 0;
Expand Down
18 changes: 13 additions & 5 deletions sql/sp_head.h
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,7 @@ class sp_instr :public Query_arena, public Sql_alloc
sp_instr(uint ip, sp_pcontext *ctx)
:Query_arena(0, STMT_INITIALIZED_FOR_SP), marked(0), m_ip(ip), m_ctx(ctx)
#ifdef PROTECT_STATEMENT_MEMROOT
, m_has_been_run(false)
, m_has_been_run(NON_RUN)
#endif
{}

Expand Down Expand Up @@ -1194,21 +1194,29 @@ class sp_instr :public Query_arena, public Sql_alloc
#ifdef PROTECT_STATEMENT_MEMROOT
bool has_been_run() const
{
return m_has_been_run;
return m_has_been_run == RUN;
}

void mark_as_qc_used()
{
m_has_been_run= QC;
}

void mark_as_run()
{
m_has_been_run= true;
if (m_has_been_run == QC)
m_has_been_run= NON_RUN; // answer was from WC => not really executed
else
m_has_been_run= RUN;
}

void mark_as_not_run()
{
m_has_been_run= false;
m_has_been_run= NON_RUN;
}

private:
bool m_has_been_run;
enum {NON_RUN, QC, RUN} m_has_been_run;
#endif
}; // class sp_instr : public Sql_alloc

Expand Down

0 comments on commit 50998a6

Please sign in to comment.