[Maria-developers] Need help merging MDEV-7387 from 10.0 to 10.1
Hi Bar, Can you please merge the following patch 10.0 -> 10.1? commit bc902a2bfc46add0708896c07621e3707f66d95f Author: Alexander Barkov <bar@mariadb.org> Date: Fri Mar 13 16:12:54 2015 +0400 MDEV-7387 [PATCH] Alter table xxx CHARACTER SET utf8, CONVERT TO CHARACTER SET latin1 should fail Eg. run `git merge bc902a2bfc46add0708896c07621e3707f66d95f` in a 10.1 tree. Or alternatively, tell me how to resolve the below conflict in the merge, which is seen in sql/handler.h, so I can do the merge? (This conflict is blocking me from pushing MDEV-7249, MDEV-5289, MDEV-7668, and MDEV-7802 into 10.1...) - Kristian. ----------------------------------------------------------------------- sql/handler.h: <<<<<<< HEAD void init() { bzero(this, sizeof(*this)); } bool tmp_table() const { return options & HA_LEX_CREATE_TMP_TABLE; } void use_default_db_type(THD *thd) { db_type= tmp_table() ? ha_default_tmp_handlerton(thd) : ha_default_handlerton(thd); } }; /** This struct is passed to handler table routines, e.g. ha_create(). It does not include the "OR REPLACE" and "IF NOT EXISTS" parts, as these parts are handled on the SQL level and are not needed on the handler level. */ struct HA_CREATE_INFO: public Table_scope_and_contents_source_st, public Schema_specification_st { void init() { Table_scope_and_contents_source_st::init(); Schema_specification_st::init(); } }; /** This struct is passed to mysql_create_table() and similar creation functions, as well as to show_create_table(). */ struct Table_specification_st: public HA_CREATE_INFO, public DDL_options_st { // Deep initialization void init() { HA_CREATE_INFO::init(); DDL_options_st::init(); } void init(DDL_options_st::Options options) { HA_CREATE_INFO::init(); DDL_options_st::init(options); } /* Quick initialization, for parser. Most of the HA_CREATE_INFO is left uninitialized. It gets fully initialized in sql_yacc.yy, only when the parser scans a related keyword (e.g. CREATE, ALTER). */ void lex_start() { HA_CREATE_INFO::options= 0; DDL_options_st::init(); } ||||||| merged common ancestors bool tmp_table() { return options & HA_LEX_CREATE_TMP_TABLE; } ======= bool tmp_table() { return options & HA_LEX_CREATE_TMP_TABLE; } bool check_conflicting_charset_declarations(CHARSET_INFO *cs); bool add_table_option_default_charset(CHARSET_INFO *cs) { // cs can be NULL, e.g.: CREATE TABLE t1 (..) CHARACTER SET DEFAULT; if (check_conflicting_charset_declarations(cs)) return true; default_table_charset= cs; used_fields|= HA_CREATE_USED_DEFAULT_CHARSET; return false; } bool add_alter_list_item_convert_to_charset(CHARSET_INFO *cs) { /* cs cannot be NULL, as sql_yacc.yy translates CONVERT TO CHARACTER SET DEFAULT to CONVERT TO CHARACTER SET <character-set-of-the-current-database> TODO: Should't we postpone resolution of DEFAULT until the character set of the table owner database is loaded from its db.opt? */ DBUG_ASSERT(cs); if (check_conflicting_charset_declarations(cs)) return true; table_charset= default_table_charset= cs; used_fields|= (HA_CREATE_USED_CHARSET | HA_CREATE_USED_DEFAULT_CHARSET); return false; }
> bc902a2bfc46add0708896c07621e3707f66d95f
https://github.com/MariaDB/server/pull/42 All conflicts resolved and selective tests run. ----- Original Message -----
Hi Bar,
Can you please merge the following patch 10.0 -> 10.1?
commit bc902a2bfc46add0708896c07621e3707f66d95f Author: Alexander Barkov <bar@mariadb.org> Date: Fri Mar 13 16:12:54 2015 +0400
MDEV-7387 [PATCH] Alter table xxx CHARACTER SET utf8, CONVERT TO CHARACTER SET latin1 should fail
Eg. run `git merge bc902a2bfc46add0708896c07621e3707f66d95f` in a 10.1 tree.
Or alternatively, tell me how to resolve the below conflict in the merge, which is seen in sql/handler.h, so I can do the merge?
(This conflict is blocking me from pushing MDEV-7249, MDEV-5289, MDEV-7668, and MDEV-7802 into 10.1...)
- Kristian.
----------------------------------------------------------------------- sql/handler.h:
<<<<<<< HEAD void init() { bzero(this, sizeof(*this)); } bool tmp_table() const { return options & HA_LEX_CREATE_TMP_TABLE; } void use_default_db_type(THD *thd) { db_type= tmp_table() ? ha_default_tmp_handlerton(thd) : ha_default_handlerton(thd); } };
/** This struct is passed to handler table routines, e.g. ha_create(). It does not include the "OR REPLACE" and "IF NOT EXISTS" parts, as these parts are handled on the SQL level and are not needed on the handler level. */ struct HA_CREATE_INFO: public Table_scope_and_contents_source_st, public Schema_specification_st { void init() { Table_scope_and_contents_source_st::init(); Schema_specification_st::init(); } };
/** This struct is passed to mysql_create_table() and similar creation functions, as well as to show_create_table(). */ struct Table_specification_st: public HA_CREATE_INFO, public DDL_options_st { // Deep initialization void init() { HA_CREATE_INFO::init(); DDL_options_st::init(); } void init(DDL_options_st::Options options) { HA_CREATE_INFO::init(); DDL_options_st::init(options); } /* Quick initialization, for parser. Most of the HA_CREATE_INFO is left uninitialized. It gets fully initialized in sql_yacc.yy, only when the parser scans a related keyword (e.g. CREATE, ALTER). */ void lex_start() { HA_CREATE_INFO::options= 0; DDL_options_st::init(); } ||||||| merged common ancestors bool tmp_table() { return options & HA_LEX_CREATE_TMP_TABLE; } ======= bool tmp_table() { return options & HA_LEX_CREATE_TMP_TABLE; } bool check_conflicting_charset_declarations(CHARSET_INFO *cs); bool add_table_option_default_charset(CHARSET_INFO *cs) { // cs can be NULL, e.g.: CREATE TABLE t1 (..) CHARACTER SET DEFAULT; if (check_conflicting_charset_declarations(cs)) return true; default_table_charset= cs; used_fields|= HA_CREATE_USED_DEFAULT_CHARSET; return false; } bool add_alter_list_item_convert_to_charset(CHARSET_INFO *cs) { /* cs cannot be NULL, as sql_yacc.yy translates CONVERT TO CHARACTER SET DEFAULT to CONVERT TO CHARACTER SET <character-set-of-the-current-database> TODO: Should't we postpone resolution of DEFAULT until the character set of the table owner database is loaded from its db.opt? */ DBUG_ASSERT(cs); if (check_conflicting_charset_declarations(cs)) return true; table_charset= default_table_charset= cs; used_fields|= (HA_CREATE_USED_CHARSET | HA_CREATE_USED_DEFAULT_CHARSET); return false; }
>> bc902a2bfc46add0708896c07621e3707f66d95f
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
-- -- Daniel Black, Engineer @ Open Query (http://openquery.com.au) Remote expertise & maintenance for MySQL/MariaDB server environments.
----- Original Message -----
Hi Bar,
Can you please merge the following patch 10.0 -> 10.1?
commit bc902a2bfc46add0708896c07621e3707f66d95f Author: Alexander Barkov <bar@mariadb.org> Date: Fri Mar 13 16:12:54 2015 +0400
MDEV-7387 [PATCH] Alter table xxx CHARACTER SET utf8, CONVERT TO CHARACTER SET latin1 should fail
Eg. run `git merge bc902a2bfc46add0708896c07621e3707f66d95f` in a 10.1 tree.
Single merge: https://github.com/MariaDB/server/pull/43 -- -- Daniel Black, Engineer @ Open Query (http://openquery.com.au) Remote expertise & maintenance for MySQL/MariaDB server environments.
Hi Daniel, On 04/18/2015 08:30 AM, Daniel Black wrote:
----- Original Message -----
Hi Bar,
Can you please merge the following patch 10.0 -> 10.1?
commit bc902a2bfc46add0708896c07621e3707f66d95f Author: Alexander Barkov <bar@mariadb.org> Date: Fri Mar 13 16:12:54 2015 +0400
MDEV-7387 [PATCH] Alter table xxx CHARACTER SET utf8, CONVERT TO CHARACTER SET latin1 should fail
Eg. run `git merge bc902a2bfc46add0708896c07621e3707f66d95f` in a 10.1 tree.
Single merge:
Thanks for your help! We appreciate your contributions very much. The idea of your merge patch is absolutely correct: to have "default_table_charset" and "used_fields" in the same structure. However, I think I'll resolve this merge conflict in a slightly different way. Instead of moving "used_fields" into Schema_specification_st, I'll just put move the new methods into HA_CREATE_INFO. Let me explain why: In 10.1 we split HA_CREATE_INFO into two parts: - Schema_specification_st - Table_scope_and_contents_source_st The reasons to do this were: - To avoid passing unnecessary information into functions and methods - To save on the involved structures initialization - To remove some duplicate code mysql_create_db() and mysql_alter_db() need only the Schema_specification_st part, while the entire HA_CREATE_INFO was passed in 10.0 and earlier into these functions. Before being passed to the above functions, HA_CREATE_INFO was initialized with bzero(). sizeof(HA_CREATE_INFO) is 304 bytes, while sizeof(Schema_specification_st) is only 8 bytes at the moment. So most of initialization was not really necessary. I don't want to move "used_fields" from Table_scope_and_contents_source_st to Schema_specification_st for exactly the same reason: because mysql_create_db() and mysql_alter_db() don't need it. So let's keep "used_fields" in Table_scope_and_contents_source_st, and just move the new methods to the HA_CREATE_INFO level, which has access to both "used_fields" and "default_table_charset". I'll push a patch in a few minutes hopefully. Running tests at the moment.
Hi Kristian, Daniel, I just pushed a merge patch into 10.1. On 04/20/2015 11:28 AM, Alexander Barkov wrote:
Hi Daniel,
On 04/18/2015 08:30 AM, Daniel Black wrote:
----- Original Message -----
Hi Bar,
Can you please merge the following patch 10.0 -> 10.1?
commit bc902a2bfc46add0708896c07621e3707f66d95f Author: Alexander Barkov <bar@mariadb.org> Date: Fri Mar 13 16:12:54 2015 +0400
MDEV-7387 [PATCH] Alter table xxx CHARACTER SET utf8, CONVERT TO CHARACTER SET latin1 should fail
Eg. run `git merge bc902a2bfc46add0708896c07621e3707f66d95f` in a 10.1 tree.
Single merge:
Thanks for your help! We appreciate your contributions very much.
The idea of your merge patch is absolutely correct: to have "default_table_charset" and "used_fields" in the same structure.
However, I think I'll resolve this merge conflict in a slightly different way.
Instead of moving "used_fields" into Schema_specification_st, I'll just put move the new methods into HA_CREATE_INFO.
Let me explain why:
In 10.1 we split HA_CREATE_INFO into two parts:
- Schema_specification_st - Table_scope_and_contents_source_st
The reasons to do this were:
- To avoid passing unnecessary information into functions and methods - To save on the involved structures initialization - To remove some duplicate code
mysql_create_db() and mysql_alter_db() need only the Schema_specification_st part, while the entire HA_CREATE_INFO was passed in 10.0 and earlier into these functions.
Before being passed to the above functions, HA_CREATE_INFO was initialized with bzero(). sizeof(HA_CREATE_INFO) is 304 bytes, while sizeof(Schema_specification_st) is only 8 bytes at the moment. So most of initialization was not really necessary.
I don't want to move "used_fields" from Table_scope_and_contents_source_st to Schema_specification_st for exactly the same reason: because mysql_create_db() and mysql_alter_db() don't need it.
So let's keep "used_fields" in Table_scope_and_contents_source_st, and just move the new methods to the HA_CREATE_INFO level, which has access to both "used_fields" and "default_table_charset".
I'll push a patch in a few minutes hopefully. Running tests at the moment.
Hi Kristian, On 04/17/2015 05:28 PM, Kristian Nielsen wrote:
Hi Bar,
Can you please merge the following patch 10.0 -> 10.1?
commit bc902a2bfc46add0708896c07621e3707f66d95f Author: Alexander Barkov <bar@mariadb.org> Date: Fri Mar 13 16:12:54 2015 +0400
MDEV-7387 [PATCH] Alter table xxx CHARACTER SET utf8, CONVERT TO CHARACTER SET latin1 should fail
Eg. run `git merge bc902a2bfc46add0708896c07621e3707f66d95f` in a 10.1 tree.
The merge conflict happened because HA_CREATE_INFO was split into parts in 10.1: - Schema_specification_st - Table_scope_and_contents_source_st (to share duplicate code between CREATE DATABASE and CREATE TABLE) Automatic merge is putting the new methods (introduced in MDEV-7387) into Table_scope_and_contentes_source_st, while they should actually go into HA_CREATE_INFO. I'll let you know when I'm done. Running tests at the moment.
Or alternatively, tell me how to resolve the below conflict in the merge, which is seen in sql/handler.h, so I can do the merge?
(This conflict is blocking me from pushing MDEV-7249, MDEV-5289, MDEV-7668, and MDEV-7802 into 10.1...)
- Kristian.
----------------------------------------------------------------------- sql/handler.h:
<<<<<<< HEAD void init() { bzero(this, sizeof(*this)); } bool tmp_table() const { return options & HA_LEX_CREATE_TMP_TABLE; } void use_default_db_type(THD *thd) { db_type= tmp_table() ? ha_default_tmp_handlerton(thd) : ha_default_handlerton(thd); } };
/** This struct is passed to handler table routines, e.g. ha_create(). It does not include the "OR REPLACE" and "IF NOT EXISTS" parts, as these parts are handled on the SQL level and are not needed on the handler level. */ struct HA_CREATE_INFO: public Table_scope_and_contents_source_st, public Schema_specification_st { void init() { Table_scope_and_contents_source_st::init(); Schema_specification_st::init(); } };
/** This struct is passed to mysql_create_table() and similar creation functions, as well as to show_create_table(). */ struct Table_specification_st: public HA_CREATE_INFO, public DDL_options_st { // Deep initialization void init() { HA_CREATE_INFO::init(); DDL_options_st::init(); } void init(DDL_options_st::Options options) { HA_CREATE_INFO::init(); DDL_options_st::init(options); } /* Quick initialization, for parser. Most of the HA_CREATE_INFO is left uninitialized. It gets fully initialized in sql_yacc.yy, only when the parser scans a related keyword (e.g. CREATE, ALTER). */ void lex_start() { HA_CREATE_INFO::options= 0; DDL_options_st::init(); } ||||||| merged common ancestors bool tmp_table() { return options & HA_LEX_CREATE_TMP_TABLE; } ======= bool tmp_table() { return options & HA_LEX_CREATE_TMP_TABLE; } bool check_conflicting_charset_declarations(CHARSET_INFO *cs); bool add_table_option_default_charset(CHARSET_INFO *cs) { // cs can be NULL, e.g.: CREATE TABLE t1 (..) CHARACTER SET DEFAULT; if (check_conflicting_charset_declarations(cs)) return true; default_table_charset= cs; used_fields|= HA_CREATE_USED_DEFAULT_CHARSET; return false; } bool add_alter_list_item_convert_to_charset(CHARSET_INFO *cs) { /* cs cannot be NULL, as sql_yacc.yy translates CONVERT TO CHARACTER SET DEFAULT to CONVERT TO CHARACTER SET <character-set-of-the-current-database> TODO: Should't we postpone resolution of DEFAULT until the character set of the table owner database is loaded from its db.opt? */ DBUG_ASSERT(cs); if (check_conflicting_charset_declarations(cs)) return true; table_charset= default_table_charset= cs; used_fields|= (HA_CREATE_USED_CHARSET | HA_CREATE_USED_DEFAULT_CHARSET); return false; }
>> bc902a2bfc46add0708896c07621e3707f66d95f
Alexander Barkov <bar@mariadb.org> writes:
The merge conflict happened because HA_CREATE_INFO was split into parts in 10.1:
- Schema_specification_st - Table_scope_and_contents_source_st
(to share duplicate code between CREATE DATABASE and CREATE TABLE)
Automatic merge is putting the new methods (introduced in MDEV-7387) into Table_scope_and_contentes_source_st, while they should actually go into HA_CREATE_INFO.
Thanks for the help with the merge. That was exactly my thinking, seeing that the code had been split in 10.1, I wanted to be sure that the merge was done correctly. I've merged the remainder of 10.0 into 10.1 now, most of which was my replication stuff. - Kristian.
participants (3)
-
Alexander Barkov -
Daniel Black -
Kristian Nielsen