From: kevin.lewis Date: November 22 2010 9:57pm Subject: bzr commit into mysql-5.5-innodb branch (kevin.lewis:3222) List-Archive: http://lists.mysql.com/commits/124690 Message-Id: <20101122215707.24BBC77F1B0@kevin-lewis-macbook.local> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0505901215==" --===============0505901215== 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-22 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. This also includes code style comments by Marko. Macros that do 'if' start with 'CHECK'. All macros now start with a command, not '{'. Macros that were only used once are expanded and are no longer macros. 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,10 +6496,11 @@ create_clustered_index_when_no_primary( /*****************************************************************//** Return a display name for the row format @return row format name */ - -const char *get_row_format_name( -/*============================*/ -enum row_type row_format) /*!< in: Row Format */ +UNIV_INTERN +const char* +get_row_type_name( +/*==============*/ + enum row_type row_format) /*!< in: Row Format */ { switch (row_format) { case ROW_TYPE_COMPACT: @@ -6514,12 +6515,76 @@ 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"); } +/** Issue a custom warning message an for illegal create option. +@param text warning text +@param parm parameters for the warning */ +#define CREATE_OPTION_MESSAGE(text, parm) \ + push_warning_printf( \ + thd, MYSQL_ERROR::WARN_LEVEL_WARN, \ + ER_ILLEGAL_HA_CREATE_OPTION, \ + text, parm) + +/** Warning message when a row type is missing innnodb-file-per-table */ +#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)) + +/** If file-per-table is missing, issue warning and set ret false */ +#define CHECK_ERROR_ROW_TYPE_NEEDS_FILE_PER_TABLE \ + if (!srv_file_per_table) { \ + MESSAGE_ROW_TYPE_NEEDS_FILE_PER_TABLE; \ + ret = FALSE; \ + } + +/** Warning message when a row type is missing a file_format > Antelope */ +#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)) + +/** If file-format is Antelope, issue warning and set ret false */ +#define CHECK_ERROR_ROW_TYPE_NEEDS_GT_ANTELOPE \ + if (srv_file_format < DICT_TF_FORMAT_ZIP) { \ + MESSAGE_ROW_TYPE_NEEDS_GT_ANTELOPE; \ + ret = FALSE; \ + } + +/** Warning message when a key_block_size is missing innnodb-file-per-table */ +#define MESSAGE_KEY_BLOCK_SIZE_NEEDS_FILE_PER_TABLE \ + CREATE_OPTION_MESSAGE( \ + "InnoDB: KEY_BLOCK_SIZE requires" \ + " innodb_file_per_table.", NULL) + +/** If file-per-table is missing, issue warning and set ret false */ +#define CHECK_ERROR_KEY_BLOCK_SIZE_NEEDS_FILE_PER_TABLE \ + if (!srv_file_per_table) { \ + MESSAGE_KEY_BLOCK_SIZE_NEEDS_FILE_PER_TABLE; \ + ret = FALSE; \ + } + +/** Warning message when key_block_size is missing a file_format > Antelope */ +#define MESSAGE_KEY_BLOCK_SIZE_NEEDS_GT_ANTELOPE \ + CREATE_OPTION_MESSAGE( \ + "InnoDB: KEY_BLOCK_SIZE requires" \ + " innodb_file_format > Antelope.", NULL) + +/** If file-format is Antelope, issue warning and set ret false */ +#define CHECK_ERROR_KEY_BLOCK_SIZE_NEEDS_GT_ANTELOPE \ + if (srv_file_format < DICT_TF_FORMAT_ZIP) { \ + MESSAGE_KEY_BLOCK_SIZE_NEEDS_GT_ANTELOPE; \ + ret = FALSE; \ + } + /*****************************************************************//** Validates the create options. We may build on this function in future. For now, it checks two specifiers: @@ -6537,7 +6602,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 +6611,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 +6624,48 @@ create_options_are_valid( case 8: case 16: /* Valid value. */ + CHECK_ERROR_KEY_BLOCK_SIZE_NEEDS_FILE_PER_TABLE; + CHECK_ERROR_KEY_BLOCK_SIZE_NEEDS_GT_ANTELOPE; break; default: - push_warning_printf( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, + CREATE_OPTION_MESSAGE( "InnoDB: invalid KEY_BLOCK_SIZE = %lu." " Valid values are [1, 2, 4, 8, 16]", create_info->key_block_size); ret = FALSE; + 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: + CHECK_ERROR_ROW_TYPE_NEEDS_FILE_PER_TABLE; + CHECK_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 */ + CHECK_ERROR_ROW_TYPE_NEEDS_FILE_PER_TABLE; + CHECK_ERROR_ROW_TYPE_NEEDS_GT_ANTELOPE; + /* fall through since dynamic also shuns KBS */ + case ROW_TYPE_COMPACT: + case ROW_TYPE_REDUNDANT: if (kbs_specified) { - push_warning_printf( - thd, MYSQL_ERROR::WARN_LEVEL_WARN, - ER_ILLEGAL_HA_CREATE_OPTION, + CREATE_OPTION_MESSAGE( "InnoDB: cannot specify ROW_FORMAT = %s" " with KEY_BLOCK_SIZE.", - get_row_format_name(row_type)); - ret = FALSE; + get_row_type_name(row_format)); + ret = FALSE; } - default: + break; + case ROW_TYPE_DEFAULT: + break; + case ROW_TYPE_FIXED: + case ROW_TYPE_PAGE: + case ROW_TYPE_NOT_USED: + CREATE_OPTION_MESSAGE( + "InnoDB: invalid ROW_FORMAT specifier.", NULL); + ret = FALSE; break; } @@ -6701,7 +6716,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 +6813,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 +6851,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 +6866,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; --===============0505901215== 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: 372b161b09c378c883141ed8fc4a4c8e3dc502b4 # timestamp: 2010-11-22 15:57:06 -0600 # base_revision_id: vasil.dimov@stripped\ # runnmftkp6kmyrm9 # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWT1UAMsABOzfgGJwevf//3/v //S/7//+YAucfN62l450m0Pdi756tuSrnbkenR5saUaDovBJJMhRtJjJkygwpp6Mmpp6nlGTRoAZ pAABoJRGpgJo0miaUb1QB6R6jQGnqZABoAAABqeiaaST1DQAGgGjRoAMgAAAAACTUpoTVMnqPU0y MjQANAAANBoaAADQEUlMj0mVP1PVPAp4p6j1AAAaNGjIAAHpA0AkRE0EaAk9GIAmRRp6QB6QANB5 Q9ID1C0DAYiEDDTm88kuqiiS5lhAR4FFAqoiqrp2rQdImGBZAVYhFPukoGSZGakxozVWQOSCzBmI RERBJ62qGQMH+rkfii6b3zAjKoEeVYIJR5RBd2wFWOIDsnvdxV32udg/e4lwH/ugGN9/A8/bflvz Nr2ukTkYGfbYcEv0/uxqiZB3rYVMFu+cFDqc0dMHFjCqIg5gAG3kAxbFlJJZoVNMUoDNxVjIhJrT QlMIvNnpH42Y+H7d/5Q/LShSMH3rzDwHSY3fM7b7lAY6pI7gfJh7vXzbaEt32JLR1dLfLK+vPrfZ gK1JwouwtR0vqVxMssQxkYBDC4eorFyxmWLMq1CJIilzExM3atZvYY4WmuN2QB1ehzGdu5Hf715i +cheFfGcPccah7rfbgP74WLWyBgTeYhPLDvm8pDNGqI66U0WcBR8M1M0ieNb83lecSI2E+ZpRFnd k2kFW7SRvXA0ATGgfzaaz5DaRIZ3fbG1ReuLtzsDSMiYHF3UEncr8Lab0k1xNxGNI2erPFMMkoet EnGFX592QugUoRKAtQpSO1NPyOjWkq4tmTTbXDhYys++W2sZjnXJuKKGCiA8TW6IhRzzjzUg3sOs kDPpDITcd3RypcKIpEdDRLWglMqiWHn7Ly7qeyVcSBkw6qz31szt+Q/KedvrGFr6PX9XWgaRp3NI j2fUM8mHp4a2/SuonwapUiNWlZ9SkIFigSlrb153hlDu1hmFo2vJJQ51q5LTciIH8Yhh5Ld6dRpF IbUtxLChZliEaqcWOSK1zbl2Nc2g4tIJZ+N0ID8l1hEpH0p+/rLXN8I44NaNOkKyYtrEvrP7lmkp aWTuzytWvrkNF4jc+o1+OywZCDZOR4kHW1nWbdzy+2QeFHZHdnISK+55E20tw1U5sTq1fLzKMsIT 4NsVTJntHW157nWCKK4PE9zkpZhNIpSjr83LOOods6sG/RfIh3jeImqWk5eF8aZxeK0LQZscbDGy cpxaME/4S6QVVoXfmqDxRJVXgV6QGm9psTWZHqFD0uaSiUC/N9NbLF2JzjFBebqrLDvKXdnFnnPo C4gEDgJ4HAYRUbrONZKbptxWTV1G6Ygl9qmTC1lKEVFaI0zWQuYSzKZQa1MIkYgb161iGTOkXaF0 21ZTOeWzMVX6qpZJSZFgtEvdONa5VlSmblgMj26ks4BlQze4oE3chYCiQg7Ol6yNNxNHTnKg93BH a5KUpghpIyTe0dDPIxqUxXwJhfs8Pu9rhw7UOxQzvc/Rei736ua2L+mVaE7OpULrPYqBqQfhaev0 OVIa+lksqfn3zr6+U6Xie5zaXbdGMmN11veNqUbwRaM8A4gsX+DWlOTU8h6lIYB+Q/Pg/in6OJMY MBfNty7DlFsEEl621StH11z4vTLZERBEFCn3R1zSgSmomFaeYm5AxLe5LEhdxN1UTeOKSa4Wsook iUoijEDkyqX3jgwimkUzmHUpxxapltU1SQzzHYhLAaCOgREURFVZkEZfIPFOLcLSEVSBiRCFYhRg udIouThTwJEZaBuctE0bENPQWRZAVCSVMQbykXsSnGEUQtqRtFQ/5E9KCyRj1DY2Nle5Embul/Ow kYsskaLBdZI0oWaKgoSLAq7EDEbRitLnKWakKWIN5vA4436wdTeQxbhWpuI1gwlANIkKSyYVS4YO SRgFtwCOgUZFtucOVZI2goBQGoE0C5JE1DUQtW9xMLxLAWZJY5iswot0IOWaNCM3MP72sogRtBED iyRvQrBm1u7BHqSJ80cAlIrx7ncKKusEZAlKIqOllCXSFBlAiYs+JtG4InzOgIIMnbrteqUE3ex9 MIeMfgGRC2NCYPR0L09TjynHekXZMKDHWXvH2XUjZyDLqI52dTGyGDlVn1dR/T4owJKnJDeqktK/ TtSnJnKaKT4STDdJ1DsCEsNKl8JlguEgiAJlhmsqKaS7ev1sJnlVqKA94chllMaHcLB19pwFe6qz knAae/I+C8Fso8kGRuXw+K2rxO4P7HFZjCduIa1WbEykUqzBIZvk3y2qiu3k1nAcchwhgznE+Zh5 Yl/CyqnBK2FY1JH1r1FJWd7+aCKb7lyN4ZQCVaDoJ2aBh6rFDZQZA3z9WT+9nKnfygw9nE6t7Ybq UwwuOI6TfTHG1DQEg7/IMTIypKWboNC8b+6GtoqmvHVz7R4pYhYIDfp1tSzr77ivRddF+2li1bnr v56WwVzW2QNuSck2VuGtxJcFQYHDgivufFG3GoIXrVAvkOy6zv1TqDpC6KaOdQ2W9j6+Ja7lhkZo 7c7qGO4C4RiFwTtzrwSYX7Eq5pVLVS94dzgrqXh1GCOHNdBF4fGp6xnp7SBjYXCN0oJGkLxkra0A 1eSGu4ucWA2ay5B4sb9/x9RHnUGwhxod89LM9gxgdlmjbvOZC0Lxcq65K4QSX3mRip0+QssTnbxC RZd0usDbMZSRXQ3ttIMucqRdoMAg3t6UoxHL9YYoOdN+zovJBjQMd50Jhvu/hJ4c8dZi2rbwseiY DZuS2hqFIpS6jfTBTwNqfimO/+R3j2EnQHYGzaF+TYv++QdwagtQZ3VL9k6tUCRl8hQR2cbtUOfJ pnTsUm2yOcaMjfmBcDZTL5Z0OrfFwNrTIxDFgjjCBVRTGjqjpem54BGFBWvUFwVksPwCIoYL6Jut ZLFpUipsBm1rWcMaiAg6SFEML7y4BUUz4ZpKypmEBYyESQBgQCPGMXdYxXQK+ExgroB9PTyyZxiF C3Sg1aTgSkezgVPkK6/CqA00Yn7sYoXyJyaUvsSL0cTTQPoYke2qlFDQ1UU2a5aDpkjQK4WVx1BP 20GnI9kyIzAJjGto0wYpO4O8KUrwz7LpxscMdYSm274V7JGEMJaPETCgA6KZiW4dNNV0l0qsIgK8 AwFKMfKC2l5acUQagaBQkp4l0aRccDR9xjG0OA/2WDRiPLJbw3BK27TlaCzhnJiumk+ZNGyRwjQN RKVcEqcw0IHEGS9WpEQZ6qFQUkJQGSiejUga8hHC8D9TREoJ31DMk3XMXDsA9YoceorHAWHYPoYY iYGzCJBrT1SET1DFeaiB1CTgHAMEXqwxqrsJnC/M6NkGdsCVRgahowhlDPacZfeFhahYouTLNW6K 7AsKIufWKyCnw36doOL65okqXRObNzOTpxThlRgmFM4cYScwtIRCpQOjZEskKUrYkiQHjMVVeSgl EUkrssI84N8YrHWObSFVqG7/taf8XckU4UJA9VADLA== --===============0505901215==--