Skip to content
Permalink
Browse files
Fix inplace ALTER TABLE to not register tmp table 
Do not register intermediate tables created by inplace ALTER TABLE in
THD::temporary_tables.

Regular ALTER TABLE doesn't create .frm for temporary and discoverable
tables anymore. For inplace ALTER TABLE moved .frm creation to
create_table_for_inplace_alter().

Removed open_in_engine argument of create_and_open_tmp_table() and
open_temporary_table(): it became unused after this patch.

Part of MDEV-17805 - Remove InnoDB cache for temporary tables.
  • Loading branch information
Sergey Vojtovich committed Apr 3, 2019
1 parent 914bb53 commit 38e151d
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 57 deletions.
4 sql/sql_class.h
View file
@@ -4742,7 +4742,6 @@ class THD: public THD_count, /* this must be first */
const char *path,
const char *db,
const char *table_name,
bool open_in_engine,
bool open_internal_tables);

TABLE *find_temporary_table(const char *db, const char *table_name,
@@ -4784,8 +4783,7 @@ class THD: public THD_count, /* this must be first */
const char *table_name);
TABLE *find_temporary_table(const char *key, uint key_length,
Temporary_table_state state);
TABLE *open_temporary_table(TMP_TABLE_SHARE *share, const char *alias,
bool open_in_engine);
TABLE *open_temporary_table(TMP_TABLE_SHARE *share, const char *alias);
bool find_and_use_tmp_table(const TABLE_LIST *tl, TABLE **out_table);
bool use_temporary_table(TABLE *table, TABLE **out_table);
void close_temporary_table(TABLE *table);
106 sql/sql_table.cc
View file
@@ -5040,7 +5040,7 @@ int create_table_impl(THD *thd, const LEX_CSTRING &orig_db,
if (!frm_only && create_info->tmp_table())
{
TABLE *table= thd->create_and_open_tmp_table(frm, path, db.str,
table_name.str, true,
table_name.str,
false);

if (!table)
@@ -7455,7 +7455,6 @@ static bool mysql_inplace_alter_table(THD *thd,
Open_table_context ot_ctx(thd, MYSQL_OPEN_REOPEN | MYSQL_OPEN_IGNORE_KILLED);
handlerton *db_type= table->s->db_type();
MDL_ticket *mdl_ticket= table->mdl_ticket;
HA_CREATE_INFO *create_info= ha_alter_info->create_info;
Alter_info *alter_info= ha_alter_info->alter_info;
bool reopen_tables= false;
bool res;
@@ -7663,8 +7662,6 @@ static bool mysql_inplace_alter_table(THD *thd,
{
goto rollback;
}

thd->drop_temporary_table(altered_table, NULL, false);
}

close_all_tables_for_name(thd, table->s,
@@ -7688,9 +7685,6 @@ static bool mysql_inplace_alter_table(THD *thd,
thd->is_error())
{
// Since changes were done in-place, we can't revert them.
(void) quick_rm_table(thd, db_type,
&alter_ctx->new_db, &alter_ctx->tmp_name,
FN_IS_TMP | NO_HA_TABLE);
DBUG_RETURN(true);
}

@@ -7767,10 +7761,6 @@ static bool mysql_inplace_alter_table(THD *thd,
thd->locked_tables_list.unlink_all_closed_tables(thd, NULL, 0);
/* QQ; do something about metadata locks ? */
}
thd->drop_temporary_table(altered_table, NULL, false);
// Delete temporary .frm/.par
(void) quick_rm_table(thd, create_info->db_type, &alter_ctx->new_db,
&alter_ctx->tmp_name, FN_IS_TMP | NO_HA_TABLE);
DBUG_RETURN(true);
}

@@ -9125,6 +9115,49 @@ simple_rename_or_index_change(THD *thd, TABLE_LIST *table_list,
}


static void cleanup_table_after_inplace_alter_keep_files(TABLE *table)
{
TABLE_SHARE *share= table->s;
closefrm(table);
free_table_share(share);
}


static void cleanup_table_after_inplace_alter(TABLE *table)
{
table->file->ha_create_partitioning_metadata(table->s->normalized_path.str, 0,
CHF_DELETE_FLAG);
deletefrm(table->s->normalized_path.str);
cleanup_table_after_inplace_alter_keep_files(table);
}


static int create_table_for_inplace_alter(THD *thd,
const Alter_table_ctx &alter_ctx,
LEX_CUSTRING *frm,
TABLE_SHARE *share,
TABLE *table)
{
init_tmp_table_share(thd, share, alter_ctx.new_db.str, 0,
alter_ctx.new_name.str, alter_ctx.get_tmp_path());
if (share->init_from_binary_frm_image(thd, true, frm->str, frm->length) ||
open_table_from_share(thd, share, &alter_ctx.new_name, 0,
EXTRA_RECORD, thd->open_options,
table, false))
{
free_table_share(share);
deletefrm(alter_ctx.get_tmp_path());
return 1;
}
if (table->internal_tables && open_and_lock_internal_tables(table, false))
{
cleanup_table_after_inplace_alter(table);
return 1;
}
return 0;
}


/**
Alter table

@@ -9785,7 +9818,8 @@ do_continue:;
key_info, key_count,
IF_PARTITIONING(thd->work_part_info, NULL),
ignore);
TABLE *altered_table;
TABLE_SHARE altered_share;
TABLE altered_table;
bool use_inplace= true;

/* Fill the Alter_inplace_info structure. */
@@ -9814,56 +9848,54 @@ do_continue:;

Also note that we ignore the LOCK clause here.

TODO don't create the frm in the first place
TODO don't create partitioning metadata in the first place
*/
const char *path= alter_ctx.get_tmp_path();
table->file->ha_create_partitioning_metadata(path, NULL, CHF_DELETE_FLAG);
deletefrm(path);
table->file->ha_create_partitioning_metadata(alter_ctx.get_tmp_path(),
NULL, CHF_DELETE_FLAG);
my_free(const_cast<uchar*>(frm.str));
goto end_inplace;
}

// We assume that the table is non-temporary.
DBUG_ASSERT(!table->s->tmp_table);

if (!(altered_table=
thd->create_and_open_tmp_table(&frm,
alter_ctx.get_tmp_path(),
alter_ctx.new_db.str,
alter_ctx.new_name.str,
false, true)))
if (create_table_for_inplace_alter(thd, alter_ctx, &frm, &altered_share,
&altered_table))
goto err_new_table_cleanup;

/* Set markers for fields in TABLE object for altered table. */
update_altered_table(ha_alter_info, altered_table);
update_altered_table(ha_alter_info, &altered_table);

/*
Mark all columns in 'altered_table' as used to allow usage
of its record[0] buffer and Field objects during in-place
ALTER TABLE.
*/
altered_table->column_bitmaps_set_no_signal(&altered_table->s->all_set,
&altered_table->s->all_set);
restore_record(altered_table, s->default_values); // Create empty record
altered_table.column_bitmaps_set_no_signal(&altered_table.s->all_set,
&altered_table.s->all_set);
restore_record(&altered_table, s->default_values); // Create empty record
/* Check that we can call default functions with default field values */
thd->count_cuted_fields= CHECK_FIELD_EXPRESSION;
altered_table->reset_default_fields();
if (altered_table->default_field &&
altered_table->update_default_fields(0, 1))
altered_table.reset_default_fields();
if (altered_table.default_field &&
altered_table.update_default_fields(0, 1))
{
cleanup_table_after_inplace_alter(&altered_table);
goto err_new_table_cleanup;
}
thd->count_cuted_fields= CHECK_FIELD_IGNORE;

if (alter_info->requested_lock == Alter_info::ALTER_TABLE_LOCK_NONE)
ha_alter_info.online= true;
// Ask storage engine whether to use copy or in-place
enum_alter_inplace_result inplace_supported=
table->file->check_if_supported_inplace_alter(altered_table,
table->file->check_if_supported_inplace_alter(&altered_table,
&ha_alter_info);

if (alter_info->supports_algorithm(thd, inplace_supported, &ha_alter_info) ||
alter_info->supports_lock(thd, inplace_supported, &ha_alter_info))
{
thd->drop_temporary_table(altered_table, NULL, false);
cleanup_table_after_inplace_alter(&altered_table);
goto err_new_table_cleanup;
}

@@ -9888,21 +9920,23 @@ do_continue:;
for alter table.
*/
thd->count_cuted_fields = CHECK_FIELD_WARN;
int res= mysql_inplace_alter_table(thd, table_list, table, altered_table,
int res= mysql_inplace_alter_table(thd, table_list, table, &altered_table,
&ha_alter_info, inplace_supported,
&target_mdl_request, &alter_ctx);
thd->count_cuted_fields= save_count_cuted_fields;
my_free(const_cast<uchar*>(frm.str));

if (res)
{
cleanup_table_after_inplace_alter(&altered_table);
DBUG_RETURN(true);
}
cleanup_table_after_inplace_alter_keep_files(&altered_table);

goto end_inplace;
}
else
{
thd->drop_temporary_table(altered_table, NULL, false);
}
cleanup_table_after_inplace_alter_keep_files(&altered_table);
}

/* ALTER TABLE using copy algorithm. */
@@ -9958,7 +9992,7 @@ do_continue:;
alter_ctx.get_tmp_path(),
alter_ctx.new_db.str,
alter_ctx.new_name.str,
true, true);
true);
if (!new_table)
goto err_new_table_cleanup;

17 sql/temporary_tables.cc
View file
@@ -53,8 +53,6 @@ bool THD::has_thd_temporary_tables()
@param path [IN] File path (without extension)
@param db [IN] Schema name
@param table_name [IN] Table name
@param open_in_engine [IN] Whether open table in SE
@return Success A pointer to table object
Failure NULL
@@ -63,7 +61,6 @@ TABLE *THD::create_and_open_tmp_table(LEX_CUSTRING *frm,
const char *path,
const char *db,
const char *table_name,
bool open_in_engine,
bool open_internal_tables)
{
DBUG_ENTER("THD::create_and_open_tmp_table");
@@ -74,7 +71,7 @@ TABLE *THD::create_and_open_tmp_table(LEX_CUSTRING *frm,
if ((share= create_temporary_table(frm, path, db, table_name)))
{
open_options|= HA_OPEN_FOR_CREATE;
table= open_temporary_table(share, table_name, open_in_engine);
table= open_temporary_table(share, table_name);
open_options&= ~HA_OPEN_FOR_CREATE;

/*
@@ -94,7 +91,7 @@ TABLE *THD::create_and_open_tmp_table(LEX_CUSTRING *frm,

/* Open any related tables */
if (open_internal_tables && table->internal_tables &&
open_and_lock_internal_tables(table, open_in_engine))
open_and_lock_internal_tables(table, true))
{
drop_temporary_table(table, NULL, false);
DBUG_RETURN(0);
@@ -379,7 +376,7 @@ bool THD::open_temporary_table(TABLE_LIST *tl)
*/
if (!table && (share= find_tmp_table_share(tl)))
{
table= open_temporary_table(share, tl->get_table_name(), true);
table= open_temporary_table(share, tl->get_table_name());
}

if (!table)
@@ -1075,14 +1072,12 @@ TABLE *THD::find_temporary_table(const char *key, uint key_length,
@param share [IN] Table share
@param alias [IN] Table alias
@param open_in_engine [IN] Whether open table in SE
@return Success A pointer to table object
Failure NULL
*/
TABLE *THD::open_temporary_table(TMP_TABLE_SHARE *share,
const char *alias_arg,
bool open_in_engine)
const char *alias_arg)
{
TABLE *table;
LEX_CSTRING alias= {alias_arg, strlen(alias_arg) };
@@ -1095,11 +1090,11 @@ TABLE *THD::open_temporary_table(TMP_TABLE_SHARE *share,
}

if (open_table_from_share(this, share, &alias,
open_in_engine ? (uint)HA_OPEN_KEYFILE : 0,
(uint) HA_OPEN_KEYFILE,
EXTRA_RECORD,
(ha_open_options |
(open_options & HA_OPEN_FOR_CREATE)),
table, open_in_engine ? false : true))
table, false))
{
my_free(table);
DBUG_RETURN(NULL);
7 sql/unireg.cc
View file
@@ -531,12 +531,6 @@ int rea_create_table(THD *thd, LEX_CUSTRING *frm,
{
DBUG_ENTER("rea_create_table");

if (no_ha_create_table)
{
if (writefrm(path, db, table_name, true, frm->str, frm->length))
goto err_frm;
}

if (thd->variables.keep_files_on_create)
create_info->options|= HA_CREATE_KEEP_FILES;

@@ -553,7 +547,6 @@ int rea_create_table(THD *thd, LEX_CUSTRING *frm,

err_part:
file->ha_create_partitioning_metadata(path, NULL, CHF_DELETE_FLAG);
err_frm:
deletefrm(path);
DBUG_RETURN(1);
} /* rea_create_table */

0 comments on commit 38e151d

Please sign in to comment.