maria-developers team mailing list archive

[kentoku <kentokushiba@xxxxxxxxx>]

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 <michael.widenius@xxxxxxxxx>:
> Hi!
>
> On Thu, Nov 24, 2016 at 8:21 PM, kentoku <kentokushiba@xxxxxxxxx> 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://root@127.0.0.1:$SLAVE_MYPORT/federated/t1_1',
>    partition p2 values in (2,4)
>      connection='mysql://root@127.0.0.1:$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