Skip to content

Commit 0274c94

Browse files
author
Mattias Jonsson
committed
WL#4305 changes according to review of partitioning code.
renamed _storage to _ref, since it was references. moved allocations from Parts_share_refs constructor to separate init function. Changed handler::ha_share to be private Not doing any locking in get/set_ha_share_ptr, but only asserting lock exists in debug. updated copyright year on touched files.
1 parent a61e82c commit 0274c94

20 files changed

Lines changed: 110 additions & 118 deletions

‎sql/ha_partition.cc‎

Lines changed: 37 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright (c) 2005, 2011, Oracle and/or its affiliates. All rights reserved.
2+
Copyright (c) 2005, 2012, Oracle and/or its affiliates. All rights reserved.
33
44
This program is free software; you can redistribute it and/or modify
55
it under the terms of the GNU General Public License as published by
@@ -135,9 +135,14 @@ bool Partition_share::init(uint num_parts)
135135
auto_inc_initialized= false;
136136
partition_name_hash_initialized= false;
137137
next_auto_inc_val= 0;
138-
partitions_shares= new Parts_share_storage(num_parts);
139-
if (!partitions_shares)
138+
partitions_share_refs= new Parts_share_refs;
139+
if (!partitions_share_refs)
140140
DBUG_RETURN(true);
141+
if (partitions_share_refs->init(num_parts))
142+
{
143+
delete partitions_share_refs;
144+
DBUG_RETURN(true);
145+
}
141146
DBUG_RETURN(false);
142147
}
143148

@@ -334,7 +339,7 @@ void ha_partition::init_handler_variables()
334339
m_is_clone_of= NULL;
335340
m_clone_mem_root= NULL;
336341
part_share= NULL;
337-
m_new_partitions_shares.empty();
342+
m_new_partitions_share_refs.empty();
338343

339344
#ifdef DONT_HAVE_TO_BE_INITALIZED
340345
m_start_key.flag= 0;
@@ -363,8 +368,8 @@ const char *ha_partition::table_type() const
363368
ha_partition::~ha_partition()
364369
{
365370
DBUG_ENTER("ha_partition::~ha_partition()");
366-
if (m_new_partitions_shares.elements)
367-
m_new_partitions_shares.delete_elements();
371+
if (m_new_partitions_share_refs.elements)
372+
m_new_partitions_share_refs.delete_elements();
368373
if (m_file != NULL)
369374
{
370375
uint i;
@@ -1707,16 +1712,18 @@ int ha_partition::change_partitions(HA_CREATE_INFO *create_info,
17071712
part_elem->part_state == PART_TO_BE_ADDED)
17081713
{
17091714
uint j= 0;
1710-
Parts_share_storage *p_share_storage;
1715+
Parts_share_refs *p_share_refs;
17111716
/*
17121717
The Handler_shares for each partition's handler can be allocated
17131718
within this handler, since there will not be any more instances of the
17141719
new partitions, until the table is reopened after the ALTER succeeded.
17151720
*/
1716-
p_share_storage= new Parts_share_storage(num_subparts);
1717-
if (!p_share_storage)
1721+
p_share_refs= new Parts_share_refs;
1722+
if (!p_share_refs)
17181723
DBUG_RETURN(HA_ERR_OUT_OF_MEM);
1719-
if (m_new_partitions_shares.push_back(p_share_storage))
1724+
if (p_share_refs->init(num_subparts))
1725+
DBUG_RETURN(HA_ERR_OUT_OF_MEM);
1726+
if (m_new_partitions_share_refs.push_back(p_share_refs))
17201727
DBUG_RETURN(HA_ERR_OUT_OF_MEM);
17211728
do
17221729
{
@@ -1729,7 +1736,7 @@ int ha_partition::change_partitions(HA_CREATE_INFO *create_info,
17291736
mem_alloc_error(sizeof(handler));
17301737
DBUG_RETURN(HA_ERR_OUT_OF_MEM);
17311738
}
1732-
if ((*new_file)->set_ha_share_storage(&p_share_storage->ha_shares[j]))
1739+
if ((*new_file)->set_ha_share_ref(&p_share_refs->ha_shares[j]))
17331740
{
17341741
DBUG_RETURN(HA_ERR_OUT_OF_MEM);
17351742
}
@@ -2842,27 +2849,26 @@ bool ha_partition::populate_partition_name_hash()
28422849
@retval false Sucess
28432850
*/
28442851

2845-
bool ha_partition::set_ha_share_storage(Handler_share **ha_share_arg)
2852+
bool ha_partition::set_ha_share_ref(Handler_share **ha_share_arg)
28462853
{
28472854
Handler_share **ha_shares;
28482855
uint i;
2849-
DBUG_ENTER("ha_partition::set_ha_share_storage");
2856+
DBUG_ENTER("ha_partition::set_ha_share_ref");
28502857

2851-
DBUG_ASSERT(!ha_share);
2852-
DBUG_ASSERT(ha_share_arg);
28532858
DBUG_ASSERT(!part_share);
28542859
DBUG_ASSERT(table_share);
28552860
DBUG_ASSERT(!m_is_clone_of);
28562861
DBUG_ASSERT(m_tot_parts);
2857-
ha_share= ha_share_arg;
2862+
if (handler::set_ha_share_ref(ha_share_arg))
2863+
DBUG_RETURN(true);
28582864
if (!(part_share= get_share()))
28592865
DBUG_RETURN(true);
2860-
DBUG_ASSERT(part_share->partitions_shares);
2861-
DBUG_ASSERT(part_share->partitions_shares->num_parts >= m_tot_parts);
2862-
ha_shares= part_share->partitions_shares->ha_shares;
2866+
DBUG_ASSERT(part_share->partitions_share_refs);
2867+
DBUG_ASSERT(part_share->partitions_share_refs->num_parts >= m_tot_parts);
2868+
ha_shares= part_share->partitions_share_refs->ha_shares;
28632869
for (i= 0; i < m_tot_parts; i++)
28642870
{
2865-
if (m_file[i]->set_ha_share_storage(&ha_shares[i]))
2871+
if (m_file[i]->set_ha_share_ref(&ha_shares[i]))
28662872
DBUG_RETURN(true);
28672873
}
28682874
DBUG_RETURN(false);
@@ -2882,34 +2888,27 @@ bool ha_partition::set_ha_share_storage(Handler_share **ha_share_arg)
28822888

28832889
Partition_share *ha_partition::get_share()
28842890
{
2885-
Handler_share *tmp_ha_share;
2886-
Partition_share *tmp_part_share;
2891+
Partition_share *tmp_share;
28872892
DBUG_ENTER("ha_partition::get_share");
28882893
DBUG_ASSERT(table_share);
28892894

2890-
if (!(tmp_ha_share= get_ha_share_ptr()))
2895+
lock_shared_ha_data();
2896+
if (!(tmp_share= static_cast<Partition_share*>(get_ha_share_ptr())))
28912897
{
2892-
tmp_part_share= new Partition_share;
2893-
if (!tmp_part_share)
2898+
tmp_share= new Partition_share;
2899+
if (!tmp_share)
28942900
goto err;
2895-
if (tmp_part_share->init(m_tot_parts))
2901+
if (tmp_share->init(m_tot_parts))
28962902
{
2897-
delete tmp_part_share;
2903+
delete tmp_share;
2904+
tmp_share= NULL;
28982905
goto err;
28992906
}
2900-
part_share= tmp_part_share;
2901-
2902-
set_ha_share_ptr(static_cast<Handler_share*>(tmp_part_share));
2903-
DBUG_RETURN(tmp_part_share);
2907+
set_ha_share_ptr(static_cast<Handler_share*>(tmp_share));
29042908
}
2905-
2906-
DBUG_RETURN(static_cast<Partition_share*>(tmp_ha_share));
29072909
err:
2908-
/*
2909-
LOCK_ha_data is taken in get_ha_share_ptr and not released if not found.
2910-
*/
29112910
unlock_shared_ha_data();
2912-
DBUG_RETURN(NULL);
2911+
DBUG_RETURN(tmp_share);
29132912
}
29142913

29152914

@@ -3184,7 +3183,7 @@ handler *ha_partition::clone(const char *name, MEM_ROOT *mem_root)
31843183

31853184
/*
31863185
We will not clone each partition's handler here, it will be done in
3187-
ha_partition::open() for clones. Also set_ha_share_storage is not needed
3186+
ha_partition::open() for clones. Also set_ha_share_ref is not needed
31883187
here, since 1) ha_share is copied in the constructor used above
31893188
2) each partition's cloned handler will set it from its original.
31903189
*/

‎sql/ha_partition.h‎

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
#define HA_PARTITION_INCLUDED
33

44
/*
5-
Copyright (c) 2005, 2011, Oracle and/or its affiliates. All rights reserved.
5+
Copyright (c) 2005, 2012, Oracle and/or its affiliates. All rights reserved.
66
77
This program is free software; you can redistribute it and/or modify
88
it under the terms of the GNU General Public License as published by
@@ -54,25 +54,38 @@ typedef struct st_part_name_def
5454
} PART_NAME_DEF;
5555

5656
/** class where to save partitions Handler_share's */
57-
class Parts_share_storage
57+
class Parts_share_refs
5858
{
5959
public:
6060
uint num_parts; /**< Size of ha_share array */
6161
Handler_share **ha_shares; /**< Storage for each part */
62-
Parts_share_storage(uint arg_num_parts)
62+
Parts_share_refs()
6363
{
64-
num_parts= arg_num_parts;
65-
/* Allocate an array of Handler_share pointers */
66-
ha_shares= new Handler_share *[num_parts];
67-
memset(ha_shares, 0, sizeof(Handler_share*) * num_parts);
64+
num_parts= 0;
65+
ha_shares= NULL;
6866
}
69-
~Parts_share_storage()
67+
~Parts_share_refs()
7068
{
7169
uint i;
7270
for (i= 0; i < num_parts; i++)
7371
if (ha_shares[i])
7472
delete ha_shares[i];
75-
delete [] ha_shares;
73+
if (ha_shares)
74+
delete [] ha_shares;
75+
}
76+
bool init(uint arg_num_parts)
77+
{
78+
DBUG_ASSERT(!num_parts && !ha_shares);
79+
num_parts= arg_num_parts;
80+
/* Allocate an array of Handler_share pointers */
81+
ha_shares= new Handler_share *[num_parts];
82+
if (!ha_shares)
83+
{
84+
num_parts= 0;
85+
return true;
86+
}
87+
memset(ha_shares, 0, sizeof(Handler_share*) * num_parts);
88+
return false;
7689
}
7790
};
7891

@@ -93,16 +106,16 @@ class Partition_share : public Handler_share
93106
bool partition_name_hash_initialized;
94107
HASH partition_name_hash;
95108
/** Storage for each partitions Handler_share */
96-
Parts_share_storage *partitions_shares;
109+
Parts_share_refs *partitions_share_refs;
97110
Partition_share() {}
98111
~Partition_share()
99112
{
100113
DBUG_ENTER("Partition_share::~Partition_share");
101114
mysql_mutex_destroy(&auto_inc_mutex);
102115
if (partition_name_hash_initialized)
103116
my_hash_free(&partition_name_hash);
104-
if (partitions_shares)
105-
delete partitions_shares;
117+
if (partitions_share_refs)
118+
delete partitions_share_refs;
106119
DBUG_VOID_RETURN;
107120
}
108121
bool init(uint num_parts);
@@ -251,7 +264,7 @@ class ha_partition :public handler
251264
/** Stores shared auto_increment etc. */
252265
Partition_share *part_share;
253266
/** Temporary storage for new partitions Handler_shares during ALTER */
254-
List<Parts_share_storage> m_new_partitions_shares;
267+
List<Parts_share_refs> m_new_partitions_share_refs;
255268
public:
256269
Partition_share *get_part_share() { return part_share; }
257270
handler *clone(const char *name, MEM_ROOT *mem_root);
@@ -361,7 +374,7 @@ class ha_partition :public handler
361374
bool is_subpart);
362375
bool populate_partition_name_hash();
363376
Partition_share *get_share();
364-
bool set_ha_share_storage(Handler_share **ha_share);
377+
bool set_ha_share_ref(Handler_share **ha_share);
365378

366379
public:
367380

‎sql/handler.cc‎

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.
1+
/* Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved.
22
33
This program is free software; you can redistribute it and/or modify
44
it under the terms of the GNU General Public License as published by
@@ -2139,7 +2139,7 @@ handler *handler::clone(const char *name, MEM_ROOT *mem_root)
21392139

21402140
if (!new_handler)
21412141
return NULL;
2142-
if (new_handler->set_ha_share_storage(ha_share))
2142+
if (new_handler->set_ha_share_ref(ha_share))
21432143
goto err;
21442144

21452145
/*
@@ -6166,49 +6166,37 @@ void handler::use_hidden_primary_key()
61666166

61676167

61686168
/**
6169-
Get an initialized ha_share
6169+
Get an initialized ha_share.
61706170
6171-
@return Initialized ha_data_share
6172-
@retval NULL ha_share is not yet initialized,
6173-
table_share->LOCK_ha_data is taken.
6171+
@return Initialized ha_share
6172+
@retval NULL ha_share is not yet initialized.
61746173
@retval != NULL previous initialized ha_share.
61756174
61766175
@note
6177-
Helper function to avoid code duplication. If there is no ha_data_share
6178-
found and it is not a temporary table LOCK_ha_data is not release and
6179-
must be unlocked by the caller.
6176+
If not a temp table, then LOCK_ha_data must be held.
61806177
*/
61816178

61826179
Handler_share *handler::get_ha_share_ptr()
61836180
{
6184-
Handler_share *found_share;
61856181
DBUG_ENTER("handler::get_ha_share_ptr");
61866182
DBUG_ASSERT(ha_share && table_share);
61876183

6188-
lock_shared_ha_data();
6184+
#ifndef DBUG_OFF
6185+
if (table_share->tmp_table == NO_TMP_TABLE)
6186+
mysql_mutex_assert_owner(&table_share->LOCK_ha_data);
6187+
#endif
61896188

6190-
if (!(found_share= *ha_share))
6191-
{
6192-
/*
6193-
No ha_data_share set yet, leave LOCK_ha_data locked
6194-
and let the caller prepare it and set it with set_ha_share_ptr().
6195-
*/
6196-
DBUG_RETURN(NULL);
6197-
}
6198-
unlock_shared_ha_data();
6199-
DBUG_RETURN(found_share);
6189+
DBUG_RETURN(*ha_share);
62006190
}
62016191

62026192

62036193
/**
6204-
Set ha_data_share in the TABLE_SHARE
6194+
Set ha_share to be used by all instances of the same table/partition.
62056195
6206-
@param ha_data Data to be shared
6207-
@param ha_data_destroy Callback to destroy ha_data
6196+
@param ha_share Handler_share to be shared.
62086197
6209-
@note Helper function to avoid code duplication.
6210-
If not a temp table, then LOCK_ha_data must be held,
6211-
and is released in this function.
6198+
@note
6199+
If not a temp table, then LOCK_ha_data must be held.
62126200
*/
62136201

62146202
void handler::set_ha_share_ptr(Handler_share *arg_ha_share)
@@ -6221,7 +6209,6 @@ void handler::set_ha_share_ptr(Handler_share *arg_ha_share)
62216209
#endif
62226210

62236211
*ha_share= arg_ha_share;
6224-
unlock_shared_ha_data();
62256212
DBUG_VOID_RETURN;
62266213
}
62276214

‎sql/handler.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
#define HANDLER_INCLUDED
33

44
/*
5-
Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.
5+
Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved.
66
77
This program is free software; you can redistribute it and/or
88
modify it under the terms of the GNU General Public License
@@ -1458,8 +1458,6 @@ class handler :public Sql_alloc
14581458
object. This cloned handler object needs to know about the lock_type used.
14591459
*/
14601460
int m_lock_type;
1461-
1462-
protected:
14631461
/**
14641462
Pointer where to store/retrieve the Handler_share pointer.
14651463
For non partitioned handlers this is &TABLE_SHARE::ha_share.
@@ -2479,7 +2477,7 @@ class handler :public Sql_alloc
24792477
{ return HA_ERR_WRONG_COMMAND; }
24802478
virtual int rename_partitions(const char *path)
24812479
{ return HA_ERR_WRONG_COMMAND; }
2482-
virtual bool set_ha_share_storage(Handler_share **arg_ha_share)
2480+
virtual bool set_ha_share_ref(Handler_share **arg_ha_share)
24832481
{
24842482
DBUG_ASSERT(!ha_share);
24852483
DBUG_ASSERT(arg_ha_share);

‎sql/partition_info.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2006, 2011, Oracle and/or its affiliates. All rights reserved.
1+
/* Copyright (c) 2006, 2012, Oracle and/or its affiliates. All rights reserved.
22
33
This program is free software; you can redistribute it and/or modify
44
it under the terms of the GNU General Public License as published by

‎sql/sql_base.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.
1+
/* Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved.
22
33
This program is free software; you can redistribute it and/or modify
44
it under the terms of the GNU General Public License as published by

‎sql/sql_partition.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2005, 2011, Oracle and/or its affiliates. All rights reserved.
1+
/* Copyright (c) 2005, 2012, Oracle and/or its affiliates. All rights reserved.
22
33
This program is free software; you can redistribute it and/or modify
44
it under the terms of the GNU General Public License as published by
@@ -6550,7 +6550,7 @@ void handle_alter_part_error(ALTER_PARTITION_PARAM_TYPE *lpt,
65506550
part_info= lpt->part_info->get_clone();
65516551
close_all_tables_for_name(thd, table->s, false);
65526552
}
6553-
else if (table->m_needs_reopen)
6553+
else
65546554
{
65556555
err_exclusive_lock:
65566556
/*

0 commit comments

Comments
 (0)