From: kevin.lewis Date: November 18 2010 5:11am Subject: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222) List-Archive: http://lists.mysql.com/commits/124219 Message-Id: <20101118051110.1B26674F0E3@kevin-lewis-macbook.local> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0307741334==" --===============0307741334== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline #At file:///Users/kevinlewis/Work/Mysql/Bug56628/mysql-5.5-innodb/ based on revid:vasil.dimov@stripped 3222 kevin.lewis@stripped 2010-11-17 RB://518 under review. Code cleanup related to patch for Bug56628. The general approach for InnoDB is to make a reference to each enum value whenever it is used in a switch statement. In addition, no default case should be used for switch statements on enum types. This assures that if there is ever any change in the enum values, the switch will need to change to reflect it since a compiler warning will occur. In this case, the enum row_type is declared in handler.h and could be changed for another storage engine. If so, a warning will occur in the InnoDB build. This patch introduces a series of macros to help consolidate the various warning messages. In the future, there will be more create option incompatibilities so these macros will help keep the code more readable. modified: storage/innobase/handler/ha_innodb.cc === modified file 'storage/innobase/handler/ha_innodb.cc' --- a/storage/innobase/handler/ha_innodb.cc revid:vasil.dimov@stripped +++ b/storage/innobase/handler/ha_innodb.cc revid:kevin.lewis@stripped @@ -6496,8 +6496,7 @@ create_clustered_index_when_no_primary( /*****************************************************************//** Return a display name for the row format @return row format name */ - -const char *get_row_format_name( +const char *get_row_type_name( /*============================*/ enum row_type row_format) /*!< in: Row Format */ { @@ -6514,12 +6513,99 @@ enum row_type row_format) /*!< in: Row return("DEFAULT"); case ROW_TYPE_FIXED: return("FIXED"); - default: + case ROW_TYPE_PAGE: + case ROW_TYPE_NOT_USED: break; } return("NOT USED"); } + +#define CREATE_OPTION_MESSAGE(_text_, _parm_) { \ + push_warning_printf( \ + thd, MYSQL_ERROR::WARN_LEVEL_WARN, \ + ER_ILLEGAL_HA_CREATE_OPTION, \ + _text_, _parm_); \ +} + +#define MESSAGE_ROW_TYPE_NEEDS_FILE_PER_TABLE { \ + CREATE_OPTION_MESSAGE( \ + "InnoDB: ROW_FORMAT=%s requires" \ + " innodb_file_per_table.", \ + get_row_type_name(row_format)); \ +} + +#define ERROR_ROW_TYPE_NEEDS_FILE_PER_TABLE { \ + if (!srv_file_per_table) { \ + MESSAGE_ROW_TYPE_NEEDS_FILE_PER_TABLE; \ + ret = FALSE; \ + } \ +} + +#define MESSAGE_ROW_TYPE_NEEDS_GT_ANTELOPE { \ + CREATE_OPTION_MESSAGE( \ + "InnoDB: ROW_FORMAT=%s requires" \ + " innodb_file_format > Antelope.", \ + get_row_type_name(row_format)); \ +} + +#define ERROR_ROW_TYPE_NEEDS_GT_ANTELOPE { \ + if (srv_file_format < DICT_TF_FORMAT_ZIP) { \ + MESSAGE_ROW_TYPE_NEEDS_GT_ANTELOPE; \ + ret = FALSE; \ + } \ +} + +#define MESSAGE_KEY_BLOCK_SIZE_NEEDS_FILE_PER_TABLE { \ + CREATE_OPTION_MESSAGE( \ + "InnoDB: KEY_BLOCK_SIZE requires" \ + " innodb_file_per_table.", NULL); \ +} + +#define ERROR_KEY_BLOCK_SIZE_NEEDS_FILE_PER_TABLE { \ + if (!srv_file_per_table) { \ + MESSAGE_KEY_BLOCK_SIZE_NEEDS_FILE_PER_TABLE; \ + ret = FALSE; \ + } \ +} + +#define MESSAGE_KEY_BLOCK_SIZE_NEEDS_GT_ANTELOPE { \ + CREATE_OPTION_MESSAGE( \ + "InnoDB: KEY_BLOCK_SIZE requires" \ + " innodb_file_format > Antelope.", NULL); \ +} + +#define ERROR_KEY_BLOCK_SIZE_NEEDS_GT_ANTELOPE { \ + if (srv_file_format < DICT_TF_FORMAT_ZIP) { \ + MESSAGE_KEY_BLOCK_SIZE_NEEDS_GT_ANTELOPE; \ + ret = FALSE; \ + } \ +} + +#define ERROR_ROW_TYPE_SHUNS_KEY_BLOCK_SIZE { \ + if (kbs_specified) { \ + CREATE_OPTION_MESSAGE( \ + "InnoDB: cannot specify ROW_FORMAT = %s" \ + " with KEY_BLOCK_SIZE.", \ + get_row_type_name(row_format)); \ + ret = FALSE; \ + } \ +} + +#define ERROR_INVALID_KEY_BLOCK_SIZE { \ + CREATE_OPTION_MESSAGE( \ + "InnoDB: invalid KEY_BLOCK_SIZE = %lu." \ + " Valid values are [1, 2, 4, 8, 16]", \ + create_info->key_block_size); \ + ret = FALSE; \ +} + +#define ERROR_INVALID_ROW_TYPE { \ + CREATE_OPTION_MESSAGE( \ + "InnoDB: invalid ROW_FORMAT specifier.", NULL); \ + ret = FALSE; \ +} + /*****************************************************************//** Validates the create options. We may build on this function in future. For now, it checks two specifiers: @@ -6537,7 +6623,7 @@ create_options_are_valid( { ibool kbs_specified = FALSE; ibool ret = TRUE; - enum row_type row_type = form->s->row_type; + enum row_type row_format = form->s->row_type; ut_ad(thd != NULL); @@ -6546,23 +6632,6 @@ create_options_are_valid( return(TRUE); } - /* Check for a valid Innodb ROW_FORMAT specifier. For example, - ROW_TYPE_FIXED can be sent to Innodb */ - switch (row_type) { - case ROW_TYPE_COMPACT: - case ROW_TYPE_COMPRESSED: - case ROW_TYPE_DYNAMIC: - case ROW_TYPE_REDUNDANT: - case ROW_TYPE_DEFAULT: - break; - default: - push_warning( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: invalid ROW_FORMAT specifier."); - ret = FALSE; - } - ut_ad(form != NULL); ut_ad(create_info != NULL); @@ -6576,81 +6645,36 @@ create_options_are_valid( case 8: case 16: /* Valid value. */ + ERROR_KEY_BLOCK_SIZE_NEEDS_FILE_PER_TABLE; + ERROR_KEY_BLOCK_SIZE_NEEDS_GT_ANTELOPE; break; default: - push_warning_printf( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: invalid KEY_BLOCK_SIZE = %lu." - " Valid values are [1, 2, 4, 8, 16]", - create_info->key_block_size); - ret = FALSE; + ERROR_INVALID_KEY_BLOCK_SIZE; + break; } } - /* If KEY_BLOCK_SIZE was specified, check for its - dependencies. */ - if (kbs_specified && !srv_file_per_table) { - push_warning( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: KEY_BLOCK_SIZE requires" - " innodb_file_per_table."); - ret = FALSE; - } - - if (kbs_specified && srv_file_format < DICT_TF_FORMAT_ZIP) { - push_warning( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: KEY_BLOCK_SIZE requires" - " innodb_file_format > Antelope."); - ret = FALSE; - } - - switch (row_type) { + /* Check for a valid Innodb ROW_FORMAT specifier and + other incompatibilities. */ + switch (row_format) { case ROW_TYPE_COMPRESSED: - case ROW_TYPE_DYNAMIC: - /* These two ROW_FORMATs require srv_file_per_table - and srv_file_format > Antelope */ - if (!srv_file_per_table) { - push_warning_printf( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: ROW_FORMAT=%s requires" - " innodb_file_per_table.", - get_row_format_name(row_type)); - ret = FALSE; - } - - if (srv_file_format < DICT_TF_FORMAT_ZIP) { - push_warning_printf( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: ROW_FORMAT=%s requires" - " innodb_file_format > Antelope.", - get_row_format_name(row_type)); - ret = FALSE; - } - default: + ERROR_ROW_TYPE_NEEDS_FILE_PER_TABLE; + ERROR_ROW_TYPE_NEEDS_GT_ANTELOPE; break; - } - - switch (row_type) { - case ROW_TYPE_REDUNDANT: - case ROW_TYPE_COMPACT: case ROW_TYPE_DYNAMIC: - /* KEY_BLOCK_SIZE is only allowed with Compressed or Default */ - if (kbs_specified) { - push_warning_printf( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: cannot specify ROW_FORMAT = %s" - " with KEY_BLOCK_SIZE.", - get_row_format_name(row_type)); - ret = FALSE; - } - default: + ERROR_ROW_TYPE_NEEDS_FILE_PER_TABLE; + ERROR_ROW_TYPE_NEEDS_GT_ANTELOPE; + /* fall through since dynamic also shuns KBS */ + case ROW_TYPE_COMPACT: + case ROW_TYPE_REDUNDANT: + ERROR_ROW_TYPE_SHUNS_KEY_BLOCK_SIZE; + break; + case ROW_TYPE_DEFAULT: + break; + case ROW_TYPE_FIXED: + case ROW_TYPE_PAGE: + case ROW_TYPE_NOT_USED: + ERROR_INVALID_ROW_TYPE; break; } @@ -6701,7 +6725,7 @@ ha_innobase::create( const ulint file_format = srv_file_format; const char* stmt; size_t stmt_len; - enum row_type row_type; + enum row_type row_format; DBUG_ENTER("ha_innobase::create"); @@ -6798,50 +6822,37 @@ ha_innobase::create( } if (!srv_file_per_table) { - push_warning( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: KEY_BLOCK_SIZE requires" - " innodb_file_per_table."); + MESSAGE_KEY_BLOCK_SIZE_NEEDS_FILE_PER_TABLE; flags = 0; } if (file_format < DICT_TF_FORMAT_ZIP) { - push_warning( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: KEY_BLOCK_SIZE requires" - " innodb_file_format > Antelope."); + MESSAGE_KEY_BLOCK_SIZE_NEEDS_GT_ANTELOPE; flags = 0; } if (!flags) { - push_warning_printf( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: ignoring" - " KEY_BLOCK_SIZE=%lu.", + CREATE_OPTION_MESSAGE( \ + "InnoDB: ignoring KEY_BLOCK_SIZE=%lu.", create_info->key_block_size); } } - row_type = form->s->row_type; + row_format = form->s->row_type; if (flags) { /* if ROW_FORMAT is set to default, automatically change it to COMPRESSED.*/ - if (row_type == ROW_TYPE_DEFAULT) { - row_type = ROW_TYPE_COMPRESSED; - } else if (row_type != ROW_TYPE_COMPRESSED) { + if (row_format == ROW_TYPE_DEFAULT) { + row_format = ROW_TYPE_COMPRESSED; + } else if (row_format != ROW_TYPE_COMPRESSED) { /* ROW_FORMAT other than COMPRESSED ignores KEY_BLOCK_SIZE. It does not make sense to reject conflicting KEY_BLOCK_SIZE and ROW_FORMAT, because such combinations can be obtained with ALTER TABLE anyway. */ - push_warning_printf( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, + CREATE_OPTION_MESSAGE( \ "InnoDB: ignoring KEY_BLOCK_SIZE=%lu" " unless ROW_FORMAT=COMPRESSED.", create_info->key_block_size); @@ -6849,7 +6860,7 @@ ha_innobase::create( } } else { /* flags == 0 means no KEY_BLOCK_SIZE.*/ - if (row_type == ROW_TYPE_COMPRESSED) { + if (row_format == ROW_TYPE_COMPRESSED) { /* ROW_FORMAT=COMPRESSED without KEY_BLOCK_SIZE implies half the maximum KEY_BLOCK_SIZE. */ @@ -6864,40 +6875,28 @@ ha_innobase::create( } } - switch (row_type) { + switch (row_format) { case ROW_TYPE_REDUNDANT: break; case ROW_TYPE_COMPRESSED: case ROW_TYPE_DYNAMIC: if (!srv_file_per_table) { - push_warning_printf( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: ROW_FORMAT=%s requires" - " innodb_file_per_table.", - get_row_format_name(row_type)); + MESSAGE_ROW_TYPE_NEEDS_FILE_PER_TABLE; } else if (file_format < DICT_TF_FORMAT_ZIP) { - push_warning_printf( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: ROW_FORMAT=%s requires" - " innodb_file_format > Antelope.", - get_row_format_name(row_type)); + MESSAGE_ROW_TYPE_NEEDS_GT_ANTELOPE; } else { flags |= DICT_TF_COMPACT - | (DICT_TF_FORMAT_ZIP - << DICT_TF_FORMAT_SHIFT); + | (DICT_TF_FORMAT_ZIP + << DICT_TF_FORMAT_SHIFT); break; } /* fall through */ case ROW_TYPE_NOT_USED: case ROW_TYPE_FIXED: - default: - push_warning( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, - "InnoDB: assuming ROW_FORMAT=COMPACT."); + case ROW_TYPE_PAGE: + CREATE_OPTION_MESSAGE( \ + "InnoDB: assuming ROW_FORMAT=COMPACT.", NULL); case ROW_TYPE_DEFAULT: case ROW_TYPE_COMPACT: flags = DICT_TF_COMPACT; --===============0307741334== MIME-Version: 1.0 Content-Type: text/bzr-bundle; charset="us-ascii"; name="bzr/kevin.lewis@stripped" Content-Transfer-Encoding: 7bit Content-Disposition: inline # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: kevin.lewis@stripped # target_branch: file:///Users/kevinlewis/Work/Mysql/Bug56628/mysql-\ # 5.5-innodb/ # testament_sha1: 9bae9a5d7b7f2bdf7d6d00db5999151ab97bb3f9 # timestamp: 2010-11-17 23:11:09 -0600 # base_revision_id: vasil.dimov@stripped\ # runnmftkp6kmyrm9 # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWSkNFGsABHFfgEJyenf//3/v //6////+YAp8fDcC6d55qUd7JUDWbSelGgAAOEkkZCJ6nqeptCTwk/U1PT1RganoR5RpoaM1NAAA aDQmjJNNFPIU0bKe1Q0A9QAANAAAND1ABpGjRGqb1TTyn6p6nqAGmgAA0AAAAAACTUlU81Mp6nk1 APFNDQADQADRo0GmQB6g0DjJgmhkMjIyaGgDQZGEA0GjTIYhoAJEhNAjRoCZMiaaGpPUeo9Rpk0A aHog9Jp6hkZLXxl4gIwUEYoijIqqsUWAogC/dlEZAxYUyVCib9kEOYFrFQp/zBEsQSi/C4BRjEQY ie5qkpof7+X54Uc2WwDlc0iPPD69ZnhQUUcKtIW9k5SCR1mWFHfUqj3O0la4UjV4NBHeaPVtx2ZL W5MNzliyAr9f1H0+W3LEBtZrz+mWaf+mLsYIsYW7zd5U+IvmOtts3zutnKUi6KV65/2s6cRGY0Lt LYPAXeVLJwnvO3ZcHxGOmTB39vCzMgX2+aFirx7dMtEHw3ZJUZKuSgSPVKyxnnyAcQEMFYZOaqjR lYtDJzQ4Oimskc4nE2MI8oUpRHtHe9KEC5gzb/Y7RIRaiLWEjv7T2DLmyAwIf3hZhKVADIqZJMGK a6qmULj2StOsD8C544bKqY4ds4NJD6SC+5I7qMo/CaAlYh7NyF18ywVkzgWyERi8jw6EQ9NBjEK8 blXPCa6jNmG0md7b5144VRY5vYHwregONEtW9pHil+tFqsTVVrAQu0i/rEMxEDFi5STgYODjRWT2 XNh8v3ZKsHxi0MRByApjIqI05YHKePaUyVEzZreJdOvr+sQz7lyUX79vKcGa26IT58/i2HhbLLKM jXVYIzzizLu8ytTEhBWr/Nv0hm/tyG5b2Wtt/K4xz3GPek16k9IqBpKUuQsOSFdcKptvW14LiwxJ oOLEoZ3udjJB8krREhHsTofLumxXUFCY586AKJCiCkw+YrABGoiRi/m2YDQQlAFIYlqLgHAhcWCa jCbeTHrHVGSjcA0oCi5B1UdM+glUFrtBYyhaZaQnR4mMyGHGQHovsrCK7xEFSFdCsjzthqgEjAoB pqBvZH6iGFoKoEjmIB1o8coBykmeOJDTiCtkYoyL9k9lIDTzhVZsiVJysxutIKUcsBhgtO+ReaZz upRr7JtVyJBmdF5YJCuGvSBuBiEe7YhggrQGAz1kQoM6Rps2RWeytJrYEQd2RFWrSxhAwfcb5BOc IHSDSpEALXarznDIV5b3BWZrc7jB5AtQBVPByrUsXkMEFjDGjV0G5ncGuICBsoLdJRIMGqXYyV0Q SCesvpQ0UgzGE2ZaNUGss1W64d9+SeWOqCu2SNsjUyuR2esOovV7fKCCLxGF/E+uNPGe3PU7C3OS shOroUCyr2qYflyTGhoYjL0HZE4bhYiYec+0dWgcg69LmzOtbhbqbjinwkg2/vRFiNqb4d2S6Q8h tSDEJ+A3pPj5pDSfYmqEQvH0yHcbjmxkIPL6kjBIP84W/BIOtEdRE6nwCMLRfOkVJ/QyYkWB9ocD 3NqIN6RDUSfFEMc5UVUpWsoTiXp+kwEh7yQ4xg0SGjVycVVVRRTHIZ5IY5qcmQOBJlJlpVUVVWIr AE1RImKC5LSYCRnUOqJ8BCoCzWsMAz6JC9DAExy10gYsyprwGFwopDTIGEPjSHLUkaQ+wIokGpIm lM+yJbxFUi/iNjY2d6InEwFaVSKBeFliRgkGKJgoSLQVNQhgHMYgKLkUX23IGyEiBCkpAtj+XAFy UhGkvQGkV40mCaGaxBQAaRsvSRqSZrHYJGmwQuwSNZirc8oclakUSIBQGQJglzSJUDSQsuBaEjkW kg0WS2WiAu4ASvSJBEkEFJfAA0eQ4iZIvJBMFDG3gkR6RcAkkT5o7JN2SbY23tBMQKY5NEVFMNHN kC0hYMASJqM3OqbQibp0xBEmWth2YgcgQ2+1vXQ8bbcDY1sUC7CTbKD2dOlUxrth6TXDyw2dGqlI 3SOixzah91gaMIRYFp67tqNdotVZm1LH8mMyfSPdrRgzkFwiQUNSRa0rC7c5px7zULY0S6yZYrTF 47hGEVKxHUqU/FtEEZuWsMktGBIUrkS1mqgopTVPLrLnonoAYguxXF3jcXYYylthLZcHJFJVlJIm XJGNSRuZosK7iyL11JSFJ907vGnMqu2ijiUWD1IPnUoMof/QVZXrzGwN9ATzCZBFoAsqAWALdOLg wc1uaYkoPOdHvrNHHM9zMCviGB5Xz0z0NtC8OTNx/D0Zf0LTHRn18w9CJWhUJBw1bGEmc6BPhS63 bwd2HLx7ePjxfPtyL2CdmWXe674EiKBJYOljDRWYS1laDAHmBSjlRXYAYo4V1AWXCvgT492BoRE8 5luasSmGvCW5JhXUlkidswO/5opmu/M6HmOvHU/V0VSVvfij6fT8QXsKoW8klIbEn5bQcW0Rzetj Q5x7/MqUs6m/0QHRo70fMetrRMJEs7FHoeZo0XxQc04rcTXkCvRGUWVxfdcyhx4E6OipVeXJZsH5 zeEYi1BzvSPV8RwRiXhcjFK4g8guBdN+zbnqhSVobKe2S8Ol2o0hXaHEtGzYlmEEC4m3XcS9guJs T84POL/oPAew+HAPYG4NhdpFB93mCPMGYUgNT4Fh3BNIcCI62y4B6UsQ9kyGh9L78ETlj7ySkK0R 1A43EJBRE7TfTA5BZYK7ti8sCki0fgETPxzhsgghJ+0TOQW/JQ03wMYYwEIVK0JATUeOoKrIGGIK oFbqAj0OVl5ev3FsJjEUhD7Ohaw7VKSRoxOEkHr6UKVkkfcJhLAXxyLLh0vR/hIxRyNE0I9c1Un+ F7JYeGm8JoRoN22fQNXRtdpBMO1aBi3hQJSnnfxsldXO7EJVagtZJkmEmggAffOUm540DMskqQiA prC5KV/dBXQ8NF8GQQQkDCW4sjQbcfJjSeC9JUMcL13hsCS40MwdkeBIij2xOaLxqiVm64mZgcWX m/KNMjKZQGYBIwXjdQ1Uhb9h7zSYFYsWIZN7SSsRRJiwpGne90kXjPvItCDILVUsGmTZYM12GBhS ssWlgNFsMmduk7JdArzYZIwRVO1qvDSgrpMRbyF+nLs8gg1VxFlfpS6TnwN0XVNt8guCdow2BC4J KiETWbFJK5EQi2y1SzwEdUZblS4SPv4o4hH/i7kinChIFIaKNYA= --===============0307741334==--