maria-developers team mailing list archive

[Zardosht Kasheff <zardosht@xxxxxxxxx>]

Hello,

I am wondering if we can resurrect this discussion. In summary, I have
a patch that I think I can apply to 5.1, although I realize it is not
the ideal solution. It is a solution that I could code that minimizes
the risk of introducing a regression for storage engines that do not
use this.

The fix is basically to have a new alter table for storage engines
that want this feature, and the old alter table for others.

The two biggest reasons I did it this way was:
 - I did not want to complicate the fix by handling storage engines
that implement handler::add_index.
 - I did not understand the partitioning fixes required.

That being said, what do people think are the next steps that should
be taken? I would love to see this feature in MariaDB 5.3, but I do
not have the expertise to be able to complete the feature. I am just
moving code from one MySQL branch to another.

Thanks
-Zardosht

On Fri, Oct 1, 2010 at 8:59 AM, Zardosht Kasheff <zardosht@xxxxxxxxx> wrote:
> I did it this way for two reasons:
>  - I did not want to try to port partitioning's alter table changes.
> That was getting into a realm where I could easily mess up
>  - I was concerned with implementing the backwards-compatible
> emulation of the old API. I figured i would do it this way for 5.1,
> and then future versions that do not need to be instantly GA-ready
> could either deprecate some of the old interface or have a backward
> compatible emulation.
>
> So, I agree with what you state below about having one alter table
> path, but I figured the safe way to do that would be for future
> versions that are not currently GA. Any thoughts on this?
>
> The cluster code does not support the following alter table flags
> (they do not even exist):
>
> #define HA_ONLINE_ADD_INDEX_NO_WRITES           (1L << 0) /*add index w/lock*/
> #define HA_ONLINE_DROP_INDEX_NO_WRITES          (1L << 1) /*drop index w/lock*/
> #define HA_ONLINE_ADD_UNIQUE_INDEX_NO_WRITES    (1L << 2) /*add unique w/lock*/
> #define HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES   (1L << 3) /*drop uniq. w/lock*/
> #define HA_ONLINE_ADD_PK_INDEX_NO_WRITES        (1L << 4) /*add prim. w/lock*/
> #define HA_ONLINE_DROP_PK_INDEX_NO_WRITES       (1L << 5) /*drop prim. w/lock*/
> #define HA_ONLINE_ADD_INDEX                     (1L << 6) /*add index online*/
> #define HA_ONLINE_DROP_INDEX                    (1L << 7) /*drop index online*/
> #define HA_ONLINE_ADD_UNIQUE_INDEX              (1L << 8) /*add unique online*/
> #define HA_ONLINE_DROP_UNIQUE_INDEX             (1L << 9) /*drop uniq. online*/
> #define HA_ONLINE_ADD_PK_INDEX                  (1L << 10)/*add prim. online*/
> #define HA_ONLINE_DROP_PK_INDEX                 (1L << 11)/*drop prim. online*/
>
> So, what would need to happen is for the default implementations of
> the new interface to check for and support these flags.
>
> -Zarodsht
>
> On Fri, Oct 1, 2010 at 3:44 AM, Kristian Nielsen
> <knielsen@xxxxxxxxxxxxxxx> wrote:
>> Zardosht Kasheff <zardosht@xxxxxxxxx> writes:
>>
>>> That being said, I would love to hear feedback on this patch, and
>>> would also like to understand what (if anything) can be done to get
>>> this into MariaDB 5.1. I realize that may not be possible, but if it
>>> is, I would like to try.
>>
>> I didn't look at the patch yet. However, I would oppose putting this into
>> MariaDB if it is a hodge-podge mixture of the old and the new alter table
>> code.
>>
>> (Just imagine if the existing code you had to work in had already contained 4
>> versions of alter table code, from MySQL 3.x, MySQL 4.0, MySQL 4.1, and MySQL
>> 5.1, say. I'm sure you found the task sufficiently tricky with just one
>> version to understand).
>>
>> On the other hand, it might make good sense to support the old interface in
>> storage engines, for example by implementing a backward-compatible emulation
>> of the old API on top of the new alter table code.
>>
>> How is it done in the original Cluster tree that you based your work on? Does
>> it convert all storage engines to the new interface? Does it implement
>> backwards compatibility on top of the new interface, leaving (some) storage
>> engines unmodified? Or does it contain both the old and the new code, like
>> your patch?
>>
>>  - Kristian.
>>
>