Skip to content

Commit f19d7e8

Browse files
author
Andrzej Jarzabek
committed
Bug#34633727 fts_get_doc_t::cache not set in fts_reset_get_doc
Every object in fts_cache_t::get_docs vector should have its cache field set to point to the containing fts_cache_t object. When fts_reset_get_doc re-sets the get_docs vector, it does not set the field. Fix: Set fts_get_doc_t::cache in fts_reset_get_doc. Change-Id: I1947462a07ba53ce0e8d5155f220f2a78825e919
1 parent e877c7f commit f19d7e8

5 files changed

Lines changed: 243 additions & 45 deletions

File tree

storage/innobase/fts/fts0fts.cc

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -233,14 +233,15 @@ static void fts_tokenize_document_next(fts_doc_t *doc, ulint add_pos,
233233
fts_doc_t *result,
234234
st_mysql_ftparser *parser);
235235

236+
namespace detail {
236237
/** Create the vector of fts_get_doc_t instances.
237238
@param[in,out] cache fts cache
238239
@return vector of fts_get_doc_t instances */
239-
static ib_vector_t *fts_get_docs_create(fts_cache_t *cache);
240+
ib_vector_t *fts_get_docs_create(fts_cache_t *cache);
240241

241242
/** Free the FTS cache.
242243
@param[in,out] cache to be freed */
243-
static void fts_cache_destroy(fts_cache_t *cache) {
244+
void fts_cache_destroy(fts_cache_t *cache) {
244245
rw_lock_free(&cache->lock);
245246
rw_lock_free(&cache->init_lock);
246247
mutex_free(&cache->optimize_lock);
@@ -259,6 +260,10 @@ static void fts_cache_destroy(fts_cache_t *cache) {
259260
mem_heap_free(cache->cache_heap);
260261
}
261262

263+
} // namespace detail
264+
using detail::fts_cache_destroy;
265+
using detail::fts_get_docs_create;
266+
262267
/** Get a character set based on precise type.
263268
@param prtype precise type
264269
@return the corresponding character set */
@@ -600,33 +605,35 @@ void fts_add_index(dict_index_t *index, /*!< FTS index to be added */
600605
rw_lock_x_unlock(&cache->init_lock);
601606
}
602607

608+
namespace detail {
609+
603610
/** recalibrate get_doc structure after index_cache in cache->indexes changed */
604-
static void fts_reset_get_doc(fts_cache_t *cache) /*!< in: FTS index cache */
611+
void fts_reset_get_doc(fts_cache_t *cache) /*!< in: FTS index cache */
605612
{
606-
fts_get_doc_t *get_doc;
607-
ulint i;
608-
609613
ut_ad(rw_lock_own(&cache->init_lock, RW_LOCK_X));
610614

611615
ib_vector_reset(cache->get_docs);
612616

613-
for (i = 0; i < ib_vector_size(cache->indexes); i++) {
614-
fts_index_cache_t *ind_cache;
615-
616-
ind_cache =
617+
for (unsigned long i = 0; i < ib_vector_size(cache->indexes); i++) {
618+
fts_index_cache_t *ind_cache =
617619
static_cast<fts_index_cache_t *>(ib_vector_get(cache->indexes, i));
618620

619-
get_doc =
621+
fts_get_doc_t *get_doc =
620622
static_cast<fts_get_doc_t *>(ib_vector_push(cache->get_docs, nullptr));
621623

622624
memset(get_doc, 0x0, sizeof(*get_doc));
623625

624626
get_doc->index_cache = ind_cache;
627+
get_doc->cache = cache;
625628
}
626629

627630
ut_ad(ib_vector_size(cache->get_docs) == ib_vector_size(cache->indexes));
628631
}
629632

633+
} // namespace detail
634+
635+
using detail::fts_reset_get_doc;
636+
630637
/** Check an index is in the table->indexes list
631638
@return true if it exists */
632639
static bool fts_in_dict_index(
@@ -4859,10 +4866,11 @@ static void fts_tokenize_document_next(fts_doc_t *doc, ulint add_pos,
48594866
}
48604867
}
48614868

4869+
namespace detail {
48624870
/** Create the vector of fts_get_doc_t instances.
48634871
@param[in,out] cache fts cache
48644872
@return vector of fts_get_doc_t instances */
4865-
static ib_vector_t *fts_get_docs_create(fts_cache_t *cache) {
4873+
ib_vector_t *fts_get_docs_create(fts_cache_t *cache) {
48664874
ib_vector_t *get_docs;
48674875

48684876
ut_ad(rw_lock_own(&cache->init_lock, RW_LOCK_X));
@@ -4891,6 +4899,7 @@ static ib_vector_t *fts_get_docs_create(fts_cache_t *cache) {
48914899

48924900
return (get_docs);
48934901
}
4902+
} // namespace detail
48944903

48954904
/********************************************************************
48964905
Release any resources held by the fts_get_doc_t instances. */
@@ -6138,7 +6147,7 @@ static bool fts_init_recover_doc(void *row, /*!< in: sel_node_t* */
61386147
fts_doc_init(&doc);
61396148
doc.found = true;
61406149

6141-
ut_ad(cache);
6150+
ut_a(cache);
61426151

61436152
/* Copy each indexed column content into doc->text.f_str */
61446153
while (exp) {
@@ -6164,7 +6173,7 @@ static bool fts_init_recover_doc(void *row, /*!< in: sel_node_t* */
61646173
continue;
61656174
}
61666175

6167-
ut_ad(get_doc);
6176+
ut_a(get_doc);
61686177

61696178
if (!get_doc->index_cache->charset) {
61706179
get_doc->index_cache->charset = fts_get_charset(dfield->type.prtype);
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/* Copyright (c) 2024, Oracle and/or its affiliates.
2+
3+
This program is free software; you can redistribute it and/or modify
4+
it under the terms of the GNU General Public License, version 2.0,
5+
as published by the Free Software Foundation.
6+
7+
This program is designed to work with certain software (including
8+
but not limited to OpenSSL) that is licensed under separate terms,
9+
as designated in a particular file or component or in included license
10+
documentation. The authors of MySQL hereby grant you an additional
11+
permission to link the program and your derivative works with the
12+
separately licensed software that they have either included with
13+
the program or referenced in the documentation.
14+
15+
This program is distributed in the hope that it will be useful,
16+
but WITHOUT ANY WARRANTY; without even the implied warranty of
17+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
18+
GNU General Public License, version 2.0, for more details.
19+
20+
You should have received a copy of the GNU General Public License
21+
along with this program; if not, write to the Free Software
22+
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
23+
24+
#ifndef detail_fts_fts_h
25+
#define detail_fts_fts_h
26+
27+
#include "fts0types.h"
28+
29+
namespace detail {
30+
31+
ib_vector_t *fts_get_docs_create(fts_cache_t *cache);
32+
void fts_reset_get_doc(fts_cache_t *cache);
33+
void fts_cache_destroy(fts_cache_t *cache);
34+
35+
} // namespace detail
36+
37+
#endif // detail_fts_fts_h

unittest/gunit/innodb/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ SET(TESTS
4040
fil_path
4141
fts0vlc
4242
ha_innodb
43+
fts0fts
4344
log0log
4445
mem0mem
4546
os0file

unittest/gunit/innodb/fts0fts-t.cc

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
/* Copyright (c) 2024, Oracle and/or its affiliates.
2+
3+
This program is free software; you can redistribute it and/or modify
4+
it under the terms of the GNU General Public License, version 2.0,
5+
as published by the Free Software Foundation.
6+
7+
This program is designed to work with certain software (including
8+
but not limited to OpenSSL) that is licensed under separate terms,
9+
as designated in a particular file or component or in included license
10+
documentation. The authors of MySQL hereby grant you an additional
11+
permission to link the program and your derivative works with the
12+
separately licensed software that they have either included with
13+
the program or referenced in the documentation.
14+
15+
This program is distributed in the hope that it will be useful,
16+
but WITHOUT ANY WARRANTY; without even the implied warranty of
17+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
18+
GNU General Public License, version 2.0, for more details.
19+
20+
You should have received a copy of the GNU General Public License
21+
along with this program; if not, write to the Free Software
22+
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
23+
24+
#include <gtest/gtest.h>
25+
26+
#include "storage/innobase/include/detail/fts/fts.h"
27+
#include "storage/innobase/include/fts0fts.h"
28+
#include "storage/innobase/include/fts0types.h"
29+
#include "storage/innobase/include/os0event.h"
30+
#include "storage/innobase/include/sync0debug.h"
31+
#include "storage/innobase/include/univ.i"
32+
#include "storage/innobase/include/ut0rbt.h"
33+
34+
namespace innodb_fts0fts_unittest {
35+
36+
/* Doc id array for testing with values exceeding 32-bit integer limit */
37+
const doc_id_t doc_ids[] = {
38+
17574, 89783, 94755, 97537, 101358, 101361,
39+
102587, 103571, 104018, 106821, 108647, 109352,
40+
109379, 110325, 122868, 210682130, 231275441, 234172769,
41+
366236849, 526467159, 1675241735, 1675243405, 1947751899, 1949940363,
42+
2033691953, 2148227299, 2256289791, 2294223591, 2367501260, 2792700091,
43+
2792701220, 2817121627, 2820680352, 2821165664, 3253312130, 3404918378,
44+
3532599429, 3538712078, 3539373037, 3546479309, 3566641838, 3580209634,
45+
3580871267, 3693930556, 3693932734, 3693932983, 3781949558, 3839877411,
46+
3930968983, 4146309172, 4524715523, 4524715525, 4534911119, 4597818456};
47+
48+
const doc_id_t search_doc_id = 1675241735;
49+
50+
namespace {
51+
struct dummy {
52+
doc_id_t doc_id;
53+
};
54+
} // namespace
55+
56+
TEST(fts0fts, fts_doc_id_field_cmp) {
57+
// doc_ids are in ascending order
58+
auto doc_ids_size = sizeof(doc_ids) / sizeof(doc_ids[0]);
59+
for (size_t i = 1; i < doc_ids_size; ++i) {
60+
dummy obj1{doc_ids[i - 1]};
61+
dummy obj2{doc_ids[i]};
62+
EXPECT_LT(fts_doc_id_field_cmp<dummy>(&obj1, &obj2), 0);
63+
obj1.doc_id = doc_ids[i];
64+
obj2.doc_id = doc_ids[i - 1];
65+
EXPECT_GT(fts_doc_id_field_cmp<dummy>(&obj1, &obj2), 0);
66+
obj2.doc_id = doc_ids[i];
67+
EXPECT_EQ(fts_doc_id_field_cmp<dummy>(&obj1, &obj2), 0);
68+
}
69+
70+
/*Test in the context of RBT, where fts_doc_id_field_cmp is intended
71+
to be used */
72+
73+
ib_rbt_t *doc_id_rbt = rbt_create(sizeof(dummy), fts_doc_id_field_cmp<dummy>);
74+
75+
/* Insert doc ids into rbtree. */
76+
for (auto doc_id : doc_ids) {
77+
ib_rbt_bound_t parent;
78+
dummy obj;
79+
obj.doc_id = doc_id;
80+
81+
if (rbt_search(doc_id_rbt, &parent, &obj.doc_id) != 0) {
82+
rbt_add_node(doc_id_rbt, &parent, &obj);
83+
}
84+
}
85+
86+
/* Check if doc id exists in rbtree */
87+
ib_rbt_bound_t parent;
88+
EXPECT_EQ(rbt_search(doc_id_rbt, &parent, &search_doc_id), 0);
89+
90+
rbt_free(doc_id_rbt);
91+
}
92+
93+
// RAII wrapper for initialization needed to create and use RW locks
94+
struct os_support_t {
95+
os_support_t() {
96+
os_event_global_init();
97+
sync_check_init(1);
98+
}
99+
~os_support_t() {
100+
sync_check_close();
101+
os_event_global_destroy();
102+
}
103+
};
104+
105+
TEST(fts0fts, fts_reset_get_doc) {
106+
os_support_t dummy;
107+
dict_table_t *table =
108+
static_cast<dict_table_t *>(calloc(1, sizeof(dict_table_t)));
109+
110+
struct fts_cache_wrapper_t {
111+
fts_cache_t *const cache{};
112+
fts_cache_wrapper_t(dict_table_t *table) : cache{fts_cache_create(table)} {}
113+
~fts_cache_wrapper_t() { detail::fts_cache_destroy(cache); }
114+
} cache_wrapper(table);
115+
fts_cache_t *const cache = cache_wrapper.cache;
116+
117+
rw_lock_x_lock(&cache->init_lock, UT_LOCATION_HERE);
118+
cache->get_docs = detail::fts_get_docs_create(cache);
119+
// Provide some content in get_docs for fts_reset_get_doc to overwrite
120+
fts_get_doc_t *get_doc =
121+
static_cast<fts_get_doc_t *>(ib_vector_push(cache->get_docs, nullptr));
122+
memset(get_doc, 0x0, sizeof(*get_doc));
123+
124+
{
125+
fts_index_cache_t ic1{};
126+
fts_index_cache_t ic2{};
127+
ib_vector_push(cache->indexes, &ic1);
128+
ib_vector_push(cache->indexes, &ic2);
129+
}
130+
131+
detail::fts_reset_get_doc(cache);
132+
133+
rw_lock_x_unlock(&cache->init_lock);
134+
135+
auto index_cache1 =
136+
static_cast<fts_index_cache_t *>(ib_vector_get(cache->indexes, 0));
137+
auto index_cache2 =
138+
static_cast<fts_index_cache_t *>(ib_vector_get(cache->indexes, 1));
139+
140+
EXPECT_EQ(ib_vector_size(cache->get_docs), 2);
141+
142+
get_doc = static_cast<fts_get_doc_t *>(ib_vector_get(cache->get_docs, 0));
143+
EXPECT_EQ(get_doc->index_cache, index_cache1);
144+
EXPECT_EQ(get_doc->get_document_graph, nullptr);
145+
EXPECT_EQ(get_doc->cache, cache);
146+
147+
get_doc = static_cast<fts_get_doc_t *>(ib_vector_get(cache->get_docs, 1));
148+
EXPECT_EQ(get_doc->index_cache, index_cache2);
149+
EXPECT_EQ(get_doc->get_document_graph, nullptr);
150+
EXPECT_EQ(get_doc->cache, cache);
151+
152+
free(table);
153+
}
154+
} // namespace innodb_fts0fts_unittest

unittest/gunit/innodb/ut0rbt-t.cc

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -22,51 +22,48 @@
2222
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
2323

2424
#include <gtest/gtest.h>
25-
26-
#include "storage/innobase/include/fts0fts.h"
27-
#include "storage/innobase/include/fts0types.h"
28-
#include "storage/innobase/include/univ.i"
2925
#include "storage/innobase/include/ut0rbt.h"
3026

3127
namespace innodb_ut0rbt_unittest {
3228

33-
/* Doc id array for testing with values exceeding 32-bit integer limit */
34-
const doc_id_t doc_ids[] = {
35-
17574, 89783, 94755, 97537, 101358, 101361,
36-
102587, 103571, 104018, 106821, 108647, 109352,
37-
109379, 110325, 122868, 210682130, 231275441, 234172769,
38-
366236849, 526467159, 1675241735, 1675243405, 1947751899, 1949940363,
39-
2033691953, 2148227299, 2256289791, 2294223591, 2367501260, 2792700091,
40-
2792701220, 2817121627, 2820680352, 2821165664, 3253312130, 3404918378,
41-
3532599429, 3538712078, 3539373037, 3546479309, 3566641838, 3580209634,
42-
3580871267, 3693930556, 3693932734, 3693932983, 3781949558, 3839877411,
43-
3930968983, 4146309172, 4524715523, 4524715525, 4534911119, 4597818456};
29+
namespace {
4430

45-
const doc_id_t search_doc_id = 1675241735;
31+
/* RBT comparator function is only required to rturn value <0 if arg1 < arg2.
32+
For the test, we use a conventional comparator function */
33+
template <typename T>
34+
int compare(const void *p1, const void *p2) {
35+
const T *arg1 = static_cast<const T *>(p1);
36+
const T *arg2 = static_cast<const T *>(p2);
37+
38+
if (*arg1 < *arg2) {
39+
return -1;
40+
} else if (*arg2 < *arg1) {
41+
return 1;
42+
} else {
43+
return 0;
44+
}
45+
}
4646

47-
namespace {
48-
struct dummy {
49-
doc_id_t doc_id;
50-
};
5147
} // namespace
5248

53-
TEST(ut0rbt, fts_doc_id_cmp) {
54-
ib_rbt_t *doc_id_rbt = rbt_create(sizeof(dummy), fts_doc_id_field_cmp<dummy>);
49+
TEST(ut0rbt, create_add_search) {
50+
ib_rbt_t *doc_id_rbt =
51+
rbt_create(sizeof(unsigned int), compare<unsigned int>);
5552

56-
/* Insert doc ids into rbtree. */
57-
for (auto doc_id : doc_ids) {
53+
/* Add sorted values to rbtree. */
54+
for (unsigned int i = 0; i < 10; ++i) {
5855
ib_rbt_bound_t parent;
59-
dummy obj;
60-
obj.doc_id = doc_id;
56+
unsigned int obj{i};
6157

62-
if (rbt_search(doc_id_rbt, &parent, &obj.doc_id) != 0) {
63-
rbt_add_node(doc_id_rbt, &parent, &obj);
64-
}
58+
ASSERT_NE(rbt_search(doc_id_rbt, &parent, &obj), 0);
59+
rbt_add_node(doc_id_rbt, &parent, &obj);
6560
}
6661

67-
/* Check if doc id exists in rbtree */
62+
unsigned int search_key = 4;
63+
64+
/* Check if an added value exists in rbtree */
6865
ib_rbt_bound_t parent;
69-
EXPECT_EQ(rbt_search(doc_id_rbt, &parent, &search_doc_id), 0);
66+
EXPECT_EQ(rbt_search(doc_id_rbt, &parent, &search_key), 0);
7067

7168
rbt_free(doc_id_rbt);
7269
}

0 commit comments

Comments
 (0)