Hi! > If this is ok, I will add this and close MDEV-7700
It looks connect string is visible, but never used. This is different from what I think. I just added patch for connect string to MDEV-7698. Please review it. This changes do not cause error for federated_partition test. Thanks, Kentoku 2016-11-28 20:31 GMT+09:00 Michael Widenius <[email protected]>: > Hi! > > On Thu, Nov 24, 2016 at 8:21 PM, kentoku <[email protected]> wrote: >> Hi! >> >>> In which case is it lost ? >>> Can you add a test to federated_partition.test that shows in which case the >>> connection information is lost? >> >> when create a table like the following >> >> create table tbl_a( >> col_a int, >> primary key(col_a) >> )engine=myisam connection 'some settings for whole table' >> partition by range(col_a) >> ( >> partition pt1 values less than (100) connection 'some settings for pt1', >> partition pt2 values less than maxvalue connection 'some settings for pt2' >> ); >> >> "connection 'some settings for whole table'" is lost. It is not only >> behavior of federated_x. >> >>> One of the original ideas with the partition engine is that each >>> partition can have options >>> that are different from the other partitions. For example, one >>> partition could be with InnoDB, another with MyISAM. For this to >>> work, there needs to exist a mechanism for specifing options per >>> partition, which the federated engine is indirectly using. >> >> Does it means all options have to write with partition information? > > All options that you want to see in SHOW CREATE. > >> How about "engine"? Don't you think it is useful if default engine can >> write as table option and only partition specific engines are write as >> partition option? I thought just like this about connect string, too. > > Normally you don't need to write special options for each partition. In > this case federatedx is a special case. > >>> Where should the connection strings for each partition be stored? >> >> I think this is already stored in par file. This is not problem. >> >>> this needs to work both with Spider but also with other usage of the >>> partitioning engine. >> >> O.K. I understand. >> >>> Where does spider store the tables connect strings ? >> >> Spider doesn't store it. It's in frm file. >> >>> How do you suggest this should work when spider is not used? >> >> I suggest ... >> These connect strings have priority level. Connect strings of >> sub-partition are the first priority. Connect strings of partition are >> the second priority. Connect string of table is the third priority. >> >>> Can you create a patch that will not cause federated_partiton.test to fail? >> >> Yes, I think I can. I'll create it. > > Does this patch do what you need: > > This preserve the CONNECT string from the top table level as it was > originally entered. > > I tested this by doing the following create: > > create table t1 (s1 int primary key) engine=federated connection="QQQ" > partition by list (s1) > (partition p1 values in (1,3) > connection='mysql://[email protected]:$SLAVE_MYPORT/federated/t1_1', > partition p2 values in (2,4) > connection='mysql://[email protected]:$SLAVE_MYPORT/federated/t1_2'); > > and show create table was able to show the original connection="QQQ" > > --- a/sql/ha_partition.cc > +++ b/sql/ha_partition.cc > @@ -3486,13 +3486,15 @@ int ha_partition::open(const char *name, int > mode, uint test_if_locked) > file= m_file; > do > { > + LEX_STRING save_connect_string= table->s->connect_string; > create_partition_name(name_buff, name, name_buffer_ptr, > NORMAL_PART_NAME, > FALSE); > table->s->connect_string = m_connect_string[(uint)(file-m_file)]; > - if ((error= (*file)->ha_open(table, name_buff, mode, > - test_if_locked | HA_OPEN_NO_PSI_CALL))) > + error= (*file)->ha_open(table, name_buff, mode, > + test_if_locked | HA_OPEN_NO_PSI_CALL); > + table->s->connect_string= save_connect_string; > + if (error) > goto err_handler; > - bzero(&table->s->connect_string, sizeof(LEX_STRING)); > if (m_file == file) > m_num_locks= (*file)->lock_count(); > DBUG_ASSERT(m_num_locks == (*file)->lock_count()); > > If this is ok, I will add this and close MDEV-7700 > > Regards, > Monty _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
