Skip to content

Commit

Permalink
MDEV-26376 pars_info_bind_id() unnecessarily copies strings
Browse files Browse the repository at this point in the history
pars_info_bind_id(): Remove the parameter copy_name. It was always
being passed as constant TRUE or true. It turns out that copying
the string is completely unnecessary. In all calls except the one
in fts_get_select_columns_str() and fts_doc_fetch_by_doc_id(),
the parameter is being passed as a compile-time constant, and therefore
the pointer cannot become stale. In that special call, the string
that is being passed is allocated from the same memory heap that
pars_info_bind_id() would have been using.

pars_info_add_id(): Remove (unused declaration).
  • Loading branch information
dr-m committed Aug 16, 2021
1 parent 50428b3 commit 4cd063b
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 51 deletions.
8 changes: 4 additions & 4 deletions storage/innobase/fts/fts0config.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 2007, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, 2020, MariaDB Corporation.
Copyright (c) 2017, 2021, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -97,7 +97,7 @@ fts_config_get_value(

fts_table->suffix = "CONFIG";
fts_get_table_name(fts_table, table_name);
pars_info_bind_id(info, true, "table_name", table_name);
pars_info_bind_id(info, "table_name", table_name);

graph = fts_parse_sql(
fts_table,
Expand Down Expand Up @@ -217,7 +217,7 @@ fts_config_set_value(

fts_table->suffix = "CONFIG";
fts_get_table_name(fts_table, table_name, dict_locked);
pars_info_bind_id(info, true, "table_name", table_name);
pars_info_bind_id(info, "table_name", table_name);

graph = fts_parse_sql(
fts_table, info,
Expand Down Expand Up @@ -245,7 +245,7 @@ fts_config_set_value(
info, "value", value->f_str, value->f_len);

fts_get_table_name(fts_table, table_name, dict_locked);
pars_info_bind_id(info, true, "table_name", table_name);
pars_info_bind_id(info, "table_name", table_name);

graph = fts_parse_sql(
fts_table, info,
Expand Down
18 changes: 9 additions & 9 deletions storage/innobase/fts/fts0fts.cc
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ fts_load_user_stopword(

pars_info_t* info = pars_info_create();

pars_info_bind_id(info, TRUE, "table_stopword", stopword_table_name);
pars_info_bind_id(info, "table_stopword", stopword_table_name);

pars_info_bind_function(info, "my_func", fts_read_stopword,
stopword_info);
Expand Down Expand Up @@ -1912,7 +1912,7 @@ fts_create_common_tables(

fts_table.suffix = "CONFIG";
fts_get_table_name(&fts_table, fts_name, true);
pars_info_bind_id(info, true, "config_table", fts_name);
pars_info_bind_id(info, "config_table", fts_name);

graph = fts_parse_sql_no_dict_lock(
&fts_table, info, fts_config_table_insert_values_sql);
Expand Down Expand Up @@ -2668,7 +2668,7 @@ fts_cmp_set_sync_doc_id(
info, "my_func", fts_fetch_store_doc_id, doc_id);

fts_get_table_name(&fts_table, table_name);
pars_info_bind_id(info, true, "config_table", table_name);
pars_info_bind_id(info, "config_table", table_name);

graph = fts_parse_sql(
&fts_table, info,
Expand Down Expand Up @@ -2796,7 +2796,7 @@ fts_update_sync_doc_id(

fts_get_table_name(&fts_table, fts_name,
table->fts->dict_locked);
pars_info_bind_id(info, true, "table_name", fts_name);
pars_info_bind_id(info, "table_name", fts_name);

graph = fts_parse_sql(
&fts_table, info,
Expand Down Expand Up @@ -2939,7 +2939,7 @@ fts_delete(
fts_table.suffix = "DELETED";

fts_get_table_name(&fts_table, table_name);
pars_info_bind_id(info, true, "deleted", table_name);
pars_info_bind_id(info, "deleted", table_name);

graph = fts_parse_sql(
&fts_table,
Expand Down Expand Up @@ -3764,7 +3764,7 @@ fts_doc_fetch_by_doc_id(
pars_info_bind_function(info, "my_func", callback, arg);

select_str = fts_get_select_columns_str(index, info, info->heap);
pars_info_bind_id(info, TRUE, "table_name", index->table_name);
pars_info_bind_id(info, "table_name", index->table_name);

if (!get_doc || !get_doc->get_document_graph) {
if (option == FTS_FETCH_DOC_BY_ID_EQUAL) {
Expand Down Expand Up @@ -3871,7 +3871,7 @@ fts_write_node(
info = pars_info_create();

fts_get_table_name(fts_table, table_name);
pars_info_bind_id(info, true, "index_table_name", table_name);
pars_info_bind_id(info, "index_table_name", table_name);
}

pars_info_bind_varchar_literal(info, "token", word->f_str, word->f_len);
Expand Down Expand Up @@ -3946,7 +3946,7 @@ fts_sync_add_deleted_cache(
&fts_table, "DELETED_CACHE", FTS_COMMON_TABLE, sync->table);

fts_get_table_name(&fts_table, table_name);
pars_info_bind_id(info, true, "table_name", table_name);
pars_info_bind_id(info, "table_name", table_name);

graph = fts_parse_sql(
&fts_table,
Expand Down Expand Up @@ -4951,7 +4951,7 @@ fts_get_rows_count(
pars_info_bind_function(info, "my_func", fts_read_ulint, &count);

fts_get_table_name(fts_table, table_name);
pars_info_bind_id(info, true, "table_name", table_name);
pars_info_bind_id(info, "table_name", table_name);

graph = fts_parse_sql(
fts_table,
Expand Down
28 changes: 13 additions & 15 deletions storage/innobase/fts/fts0opt.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 2007, 2018, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2016, 2020, MariaDB Corporation.
Copyright (c) 2016, 2021, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -492,7 +492,7 @@ fts_index_fetch_nodes(

fts_get_table_name(fts_table, table_name);

pars_info_bind_id(info, true, "table_name", table_name);
pars_info_bind_id(info, "table_name", table_name);
}

pars_info_bind_function(info, "my_func", fetch->read_record, fetch);
Expand Down Expand Up @@ -821,7 +821,7 @@ fts_index_fetch_words(
info, "word", word->f_str, word->f_len);

fts_get_table_name(&optim->fts_index_table, table_name);
pars_info_bind_id(info, true, "table_name", table_name);
pars_info_bind_id(info, "table_name", table_name);

graph = fts_parse_sql(
&optim->fts_index_table,
Expand Down Expand Up @@ -977,7 +977,7 @@ fts_table_fetch_doc_ids(
pars_info_bind_function(info, "my_func", fts_fetch_doc_ids, doc_ids);

fts_get_table_name(fts_table, table_name);
pars_info_bind_id(info, true, "table_name", table_name);
pars_info_bind_id(info, "table_name", table_name);

graph = fts_parse_sql(
fts_table,
Expand Down Expand Up @@ -1441,7 +1441,7 @@ fts_optimize_write_word(

fts_table->suffix = fts_get_suffix(selected);
fts_get_table_name(fts_table, table_name);
pars_info_bind_id(info, true, "table_name", table_name);
pars_info_bind_id(info, "table_name", table_name);

graph = fts_parse_sql(
fts_table,
Expand Down Expand Up @@ -2032,11 +2032,11 @@ fts_optimize_purge_deleted_doc_ids(
used in the fts_delete_doc_ids_sql */
optim->fts_common_table.suffix = fts_common_tables[3];
fts_get_table_name(&optim->fts_common_table, deleted);
pars_info_bind_id(info, true, fts_common_tables[3], deleted);
pars_info_bind_id(info, fts_common_tables[3], deleted);

optim->fts_common_table.suffix = fts_common_tables[4];
fts_get_table_name(&optim->fts_common_table, deleted_cache);
pars_info_bind_id(info, true, fts_common_tables[4], deleted_cache);
pars_info_bind_id(info, fts_common_tables[4], deleted_cache);

graph = fts_parse_sql(NULL, info, fts_delete_doc_ids_sql);

Expand Down Expand Up @@ -2089,12 +2089,11 @@ fts_optimize_purge_deleted_doc_id_snapshot(
used in the fts_end_delete_sql */
optim->fts_common_table.suffix = fts_common_tables[0];
fts_get_table_name(&optim->fts_common_table, being_deleted);
pars_info_bind_id(info, true, fts_common_tables[0], being_deleted);
pars_info_bind_id(info, fts_common_tables[0], being_deleted);

optim->fts_common_table.suffix = fts_common_tables[1];
fts_get_table_name(&optim->fts_common_table, being_deleted_cache);
pars_info_bind_id(info, true, fts_common_tables[1],
being_deleted_cache);
pars_info_bind_id(info, fts_common_tables[1], being_deleted_cache);

/* Delete the doc ids that were copied to delete pending state at
the start of optimize. */
Expand Down Expand Up @@ -2150,20 +2149,19 @@ fts_optimize_create_deleted_doc_id_snapshot(
used in the fts_init_delete_sql */
optim->fts_common_table.suffix = fts_common_tables[0];
fts_get_table_name(&optim->fts_common_table, being_deleted);
pars_info_bind_id(info, true, fts_common_tables[0], being_deleted);
pars_info_bind_id(info, fts_common_tables[0], being_deleted);

optim->fts_common_table.suffix = fts_common_tables[3];
fts_get_table_name(&optim->fts_common_table, deleted);
pars_info_bind_id(info, true, fts_common_tables[3], deleted);
pars_info_bind_id(info, fts_common_tables[3], deleted);

optim->fts_common_table.suffix = fts_common_tables[1];
fts_get_table_name(&optim->fts_common_table, being_deleted_cache);
pars_info_bind_id(info, true, fts_common_tables[1],
being_deleted_cache);
pars_info_bind_id(info, fts_common_tables[1], being_deleted_cache);

optim->fts_common_table.suffix = fts_common_tables[4];
fts_get_table_name(&optim->fts_common_table, deleted_cache);
pars_info_bind_id(info, true, fts_common_tables[4], deleted_cache);
pars_info_bind_id(info, fts_common_tables[4], deleted_cache);

/* Move doc_ids that are to be deleted to state being deleted. */
graph = fts_parse_sql(NULL, info, fts_init_delete_sql);
Expand Down
8 changes: 4 additions & 4 deletions storage/innobase/fts/fts0que.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 2007, 2020, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, 2020, MariaDB Corporation.
Copyright (c) 2017, 2021, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -2146,7 +2146,7 @@ fts_query_find_term(
query->fts_index_table.suffix = fts_get_suffix(selected);

fts_get_table_name(&query->fts_index_table, table_name);
pars_info_bind_id(info, true, "index_table_name", table_name);
pars_info_bind_id(info, "index_table_name", table_name);
}

select.found = FALSE;
Expand Down Expand Up @@ -2286,7 +2286,7 @@ fts_query_total_docs_containing_term(

fts_get_table_name(&query->fts_index_table, table_name);

pars_info_bind_id(info, true, "index_table_name", table_name);
pars_info_bind_id(info, "index_table_name", table_name);

graph = fts_parse_sql(
&query->fts_index_table,
Expand Down Expand Up @@ -2369,7 +2369,7 @@ fts_query_terms_in_document(

fts_get_table_name(&query->fts_index_table, table_name);

pars_info_bind_id(info, true, "index_table_name", table_name);
pars_info_bind_id(info, "index_table_name", table_name);

graph = fts_parse_sql(
&query->fts_index_table,
Expand Down
4 changes: 2 additions & 2 deletions storage/innobase/fts/fts0sql.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 2007, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2019, MariaDB Corporation.
Copyright (c) 2019, 2021, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -249,7 +249,7 @@ fts_get_select_columns_str(
sel_str = mem_heap_printf(heap, "sel%lu", (ulong) i);

/* Set copy_name to TRUE since it's dynamic. */
pars_info_bind_id(info, TRUE, sel_str, field->name);
pars_info_bind_id(info, sel_str, field->name);

str = mem_heap_printf(
heap, "%s%s$%s", str, (*str) ? ", " : "", sel_str);
Expand Down
4 changes: 2 additions & 2 deletions storage/innobase/handler/i_s.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************

Copyright (c) 2007, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2014, 2020, MariaDB Corporation.
Copyright (c) 2014, 2021, MariaDB Corporation.

This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -3456,7 +3456,7 @@ i_s_fts_index_table_fill_selected(
FTS_INIT_INDEX_TABLE(&fts_table, fts_get_suffix(selected),
FTS_INDEX_TABLE, index);
fts_get_table_name(&fts_table, table_name);
pars_info_bind_id(info, true, "table_name", table_name);
pars_info_bind_id(info, "table_name", table_name);

graph = fts_parse_sql(
&fts_table, info,
Expand Down
12 changes: 1 addition & 11 deletions storage/innobase/include/pars0pars.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, 2019, MariaDB Corporation.
Copyright (c) 2017, 2021, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -491,7 +491,6 @@ void
pars_info_bind_id(
/*=============*/
pars_info_t* info, /*!< in: info struct */
ibool copy_name,/* in: make a copy of name if TRUE */
const char* name, /*!< in: name */
const char* id); /*!< in: id */
/****************************************************************//**
Expand Down Expand Up @@ -537,15 +536,6 @@ pars_info_bind_ull_literal(
const ib_uint64_t* val) /*!< in: value */
MY_ATTRIBUTE((nonnull));

/****************************************************************//**
Add bound id. */
void
pars_info_add_id(
/*=============*/
pars_info_t* info, /*!< in: info struct */
const char* name, /*!< in: name */
const char* id); /*!< in: id */

/****************************************************************//**
Get bound literal with the given name.
@return bound literal, or NULL if not found */
Expand Down
6 changes: 2 additions & 4 deletions storage/innobase/pars/pars0pars.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2019, MariaDB Corporation.
Copyright (c) 2019, 2021, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -2352,7 +2352,6 @@ void
pars_info_bind_id(
/*==============*/
pars_info_t* info, /*!< in: info struct */
ibool copy_name, /* in: copy name if TRUE */
const char* name, /*!< in: name */
const char* id) /*!< in: id */
{
Expand All @@ -2375,8 +2374,7 @@ pars_info_bind_id(
bid = static_cast<pars_bound_id_t*>(
ib_vector_push(info->bound_ids, NULL));

bid->name = (copy_name)
? mem_heap_strdup(info->heap, name) : name;
bid->name = name;
}

bid->id = id;
Expand Down

0 comments on commit 4cd063b

Please sign in to comment.