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@gmail.com> 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@knielsen-hq.org> wrote:
Zardosht Kasheff <zardosht@gmail.com> 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.