#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;
Attachment: [text/bzr-bundle] bzr/kevin.lewis@oracle.com-20101118051058-3hi9n0au52jomihl.bundle