-
Notifications
You must be signed in to change notification settings - Fork 521
PS-11264: Vector index support in Data Dictionary #6000
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: vector-mvp
Are you sure you want to change the base?
Changes from all commits
b6197ec
b3928a7
fe4fb2d
2c15ffa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| CREATE TABLE t1 ( | ||
| id BIGINT UNSIGNED PRIMARY KEY, | ||
| v1 VECTOR ( 1234 ) NOT NULL, | ||
| KEY( v1 ) TYPE hnsw | ||
| ); | ||
| SHOW CREATE TABLE t1; | ||
| Table Create Table | ||
| t1 CREATE TABLE `t1` ( | ||
| `id` bigint unsigned NOT NULL, | ||
| `v1` vector(1234) NOT NULL, | ||
| PRIMARY KEY (`id`), | ||
| VECTOR KEY `v1` (`v1`) TYPE `hnsw` | ||
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci | ||
| SHOW INDEXES FROM t1; | ||
| Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment Visible Expression | ||
| t1 0 PRIMARY 1 id A 0 NULL NULL BTREE YES NULL | ||
| t1 1 v1 1 v1 A 0 1 NULL VECTOR YES NULL | ||
| SELECT * FROM INFORMATION_SCHEMA.INNODB_INDEXES; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure the full output of this statement is stable enough and won't break if, for example, e.g. new table is added to data-dictionary or some new component installed by default? I think it is better to exclude rows unrelated to other tables/indexes and possibly mask fragile things like index and table ids...
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also how about testing other INFORMATION_SCHEMA tables which contain information about indexes, |
||
| INDEX_ID NAME TABLE_ID TYPE N_FIELDS PAGE_NO SPACE MERGE_THRESHOLD | ||
| 157 PRIMARY 1063 3 6 4 1 50 | ||
| 187 GEN_CLUST_INDEX 1082 1 4 4 18 50 | ||
| 189 PRIMARY 1084 3 4 4 20 50 | ||
| 188 PRIMARY 1083 3 3 4 19 50 | ||
| 161 GEN_CLUST_INDEX 1067 1 4 4 5 50 | ||
| 162 pattern 1067 0 2 5 5 50 | ||
| 192 PRIMARY 1087 3 4 4 21 50 | ||
| 193 v1 1087 1024 1 -1 21 50 | ||
| DROP TABLE t1; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| CREATE TABLE t1 ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be honest I don't understand why this test is necessary... |
||
| a CHAR( 2 ), | ||
| KEY ( a(1) ) | ||
| ) ENGINE = ROCKSDB; | ||
| SHOW KEYS FROM t1; | ||
| Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment Visible Expression | ||
| t1 1 a 1 a A NULL 1 NULL YES SE_SPECIFIC YES NULL | ||
| DROP TABLE t1; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| CREATE TABLE t1 ( | ||
| id BIGINT UNSIGNED PRIMARY KEY, | ||
| v1 VECTOR ( 1234 ) NOT NULL, | ||
| KEY( v1 ) TYPE hnsw | ||
| ); | ||
| SHOW CREATE TABLE t1; | ||
| SHOW INDEXES FROM t1; | ||
|
|
||
| SELECT * FROM INFORMATION_SCHEMA.INNODB_INDEXES; | ||
|
|
||
| DROP TABLE t1; | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| $ROCKSDB_OPT $ROCKSDB_LOAD_ADD |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| --source include/have_rocksdb.inc | ||
|
|
||
| CREATE TABLE t1 ( | ||
| a CHAR( 2 ), | ||
| KEY ( a(1) ) | ||
| ) ENGINE = ROCKSDB; | ||
|
|
||
| SHOW KEYS FROM t1; | ||
|
|
||
| DROP TABLE t1; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11112,6 +11112,18 @@ ER_LOG_NAME_NOT_MATCHING_SEC_LOG_PATH_CLIENT | |
| # Start of Percona Server 8.4/9.7 error messages to be sent to client | ||
| # | ||
|
|
||
| ER_INDEX_MUST_HAVE_COMPATIBLE_COLUMN | ||
| eng "A %.20s index may only contain a %.20s type column." | ||
|
|
||
| ER_VECTOR_INDEX_NEEDS_PK | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be honest I am not sure that it is worth to have ER_VECTOR_INDEX_NEEDS_PK and ER_ONLY_SINGLE_VECTOR_INDEX_ALLOWED... While I agree it is a good practice to The problem is that once we publish code with new error code/error message it is not easy to retract it, So perhaps it is better to use ER_NOT_SUPPORTED_YET or ER_FEATURE_UNSUPPORTED @satya-bodapati @percona-mhansson what do you think?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we can print custom string along with it, using ER_NOT_SUPPORTED_YET is ok.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes we can. |
||
| eng "Vector index can only be created in tables with a BIGINT UNSIGNED primary key." | ||
|
|
||
| ER_ONLY_SINGLE_VECTOR_INDEX_ALLOWED | ||
| eng "A table can have at most one vector index." | ||
|
percona-mhansson marked this conversation as resolved.
|
||
|
|
||
| ER_TABLE_CANT_HANDLE_INDEX | ||
| eng "The used table type doesn't support %.20s indexes" | ||
|
|
||
| start-error-number 7100 | ||
|
|
||
| # | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |
|
|
||
| #include "sql/create_field.h" | ||
|
|
||
| #include "field_types.h" | ||
| #include "m_string.h" | ||
| #include "mysql/strings/dtoa.h" | ||
| #include "sql-common/my_decimal.h" | ||
|
|
@@ -200,9 +201,10 @@ bool Create_field::init( | |
| const LEX_CSTRING *fld_comment, const char *fld_change, | ||
| List<String> *fld_interval_list, const CHARSET_INFO *fld_charset, | ||
| bool has_explicit_collation, uint fld_geom_type, | ||
| const LEX_CSTRING *fld_zip_dict_name, Value_generator *fld_gcol_info, Value_generator *fld_default_val_expr, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand that this change is side-effect of applying clang-format to the whole modified file,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will it not fail the clang-format check in that case? I assume that at some point some incorrectly formatted files managed to sneak in, but the check will look at touched files, right? (whole files)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIU clang-format check does not check the whole file which was touched but only lines which were affected by commit. So you don't have to re-format the whole file to make the check pass. |
||
| LEX_CSTRING fld_masking_policy, std::optional<gis::srid_t> srid, | ||
| dd::Column::enum_hidden_type hidden, bool is_array_arg) { | ||
| const LEX_CSTRING *fld_zip_dict_name, Value_generator *fld_gcol_info, | ||
| Value_generator *fld_default_val_expr, LEX_CSTRING fld_masking_policy, | ||
| std::optional<gis::srid_t> srid, dd::Column::enum_hidden_type hidden, | ||
| bool is_array_arg) { | ||
| uint sign_len, allowed_type_modifier = 0; | ||
| ulong max_field_charlength = MAX_FIELD_CHARLENGTH; | ||
|
|
||
|
|
@@ -780,7 +782,8 @@ size_t Create_field::key_length() const { | |
| case MYSQL_TYPE_JSON: | ||
| case MYSQL_TYPE_VAR_STRING: | ||
| case MYSQL_TYPE_STRING: | ||
| case MYSQL_TYPE_VARCHAR: { | ||
| case MYSQL_TYPE_VARCHAR: | ||
| case MYSQL_TYPE_VECTOR: { | ||
| return std::min(max_display_width_in_bytes(), | ||
| static_cast<size_t>(MAX_FIELD_BLOBLENGTH)); | ||
| } | ||
|
|
@@ -794,10 +797,6 @@ size_t Create_field::key_length() const { | |
| } | ||
| return pack_length() + (max_display_width_in_bytes() & 7 ? 1 : 0); | ||
| } | ||
| /* LCOV_EXCL_START */ | ||
| case MYSQL_TYPE_VECTOR: | ||
| assert(false); // Key on VECTOR type column is not supported. | ||
| /* LCOV_EXCL_STOP */ | ||
| default: { | ||
| return pack_length(is_array); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -735,6 +735,10 @@ bool fill_dd_columns_from_create_fields(THD *thd, dd::Abstract_table *tab_obj, | |
| col_options->set("is_array", true); | ||
| } | ||
|
|
||
| if (field.sql_type == MYSQL_TYPE_VECTOR) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I have said above I believe this vector_index marker belongs to Index::options() instead. IMO even if you prefer to keep Column rows/objects participating in vector indexes marked as such for some reason, it is cleaner to do so only for columns which are part of index only, and not all vector columns. |
||
| col_options->set("vector_index", true); | ||
| } | ||
|
|
||
| // | ||
| // Write intervals | ||
| // | ||
|
|
@@ -825,6 +829,9 @@ static dd::Index::enum_index_algorithm dd_get_new_index_algorithm_type( | |
|
|
||
| case HA_KEY_ALG_FULLTEXT: | ||
| return dd::Index::IA_FULLTEXT; | ||
|
|
||
| case HA_KEY_ALG_VECTOR: | ||
| return dd::Index::IA_SE_SPECIFIC; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it simplify if we introduce dd::Index::IA_VECTOR ? To tell if an index is a vector or not, we are iterating all elements of the vector and returning true if at least one element is a vector type. I see that we already have an in-memory flag HA_VECTOR. Maybe it simplifies if we introduce dd::Index::IT_VECTOR?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would indeed be simpler. The problem is that these correspond to a table definition that we are not allowed to change. We are free to do what we want in-memory, but on-disk metadata cannot be altered.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, MySQL lacks an extensible (DD) framework for adding new types of columns or indexes. Adding a new type in downstream means that we conflict with upstream uses the next bit/flag 😞
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One doubt, if user drops all vecotr indexes/columns etc, would this flag (dd::Index::IT_VECTOR) in DD still cause upstream mysqld to not use our datadir? :thinking:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's not only that. @dlenev can probably fill in the gaps here, but from what I understand, we can't change the DD table definitions (in SQL).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, adding new enum elements in ENUM definitions in DD Indexes table will break compatibility with Upstream, which we should avoid. And yes, this won't be change which can be easily fixed by dropping all vector indexes in the system, as not the contents, but definition of DD table is changed in this case. In theory we can introduce IA_VECTOR and IT_VECTOR on DD API/Index object level and map it to the same IA_SE_SPECIFIC and IT_MULTIPLE in DD tables underneath the API. But IMO this won't simplify things and can be more confusing actually (IMO it is nice when DD API concepts and contents of DD tables match!).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having said above I think the code can be improved by adding comment here explaining why we map VECTOR algorithm to IA_SE_SPECIFIC. Basically, explain that we don't want to break binary compatibility of DD tables definitions with Upstream by introducing new elements in index algorithm and types enum in DD indexes table. |
||
| } | ||
|
|
||
| /* purecov: begin deadcode */ | ||
|
|
@@ -836,6 +843,8 @@ static dd::Index::enum_index_algorithm dd_get_new_index_algorithm_type( | |
| } | ||
|
|
||
| static dd::Index::enum_index_type dd_get_new_index_type(const KEY *key) { | ||
| if (key->flags & HA_VECTOR) return dd::Index::IT_MULTIPLE; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is a good idea to add similar comment here as well (perhaps minimal, referencing the previous comment). |
||
|
|
||
| if (key->flags & HA_FULLTEXT) return dd::Index::IT_FULLTEXT; | ||
|
|
||
| if (key->flags & HA_SPATIAL) return dd::Index::IT_SPATIAL; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,7 +102,7 @@ class Show_statistics : public Statistics { | |
| } | ||
|
|
||
| // This view definition is hidden from user. | ||
| bool hidden() const override { return true; } | ||
| bool hidden() const override { return false; } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? Perhaps some unreverted debug-only change? |
||
| }; | ||
|
|
||
| } // namespace system_views | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,11 +64,17 @@ class Sdi_rcontext; | |
| class Sdi_wcontext; | ||
|
|
||
| static const std::set<String_type> default_valid_option_keys = { | ||
| "column_format", "geom_type", | ||
| "interval_count", "not_secondary", | ||
| "storage", "treat_bit_as_char", "zip_dict_id", | ||
| "is_array", "gipk" /* generated implicit primary key column */, | ||
| "masking_policy"}; | ||
| "column_format", | ||
| "geom_type", | ||
| "interval_count", | ||
| "not_secondary", | ||
| "storage", | ||
| "treat_bit_as_char", | ||
| "zip_dict_id", | ||
| "is_array", | ||
| "gipk" /* generated implicit primary key column */, | ||
| "masking_policy", | ||
| "vector_index"}; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm... The fact that we mark vector index as such by only storing special marker among options of Column object looks conceptually wrong to me... IMO this information should really belong to Index options instead (i.e. we should store marker in Index_impl::options()). IMO this misplacement is the reason why you had to introduce non-trivial dd_index_has_vector_column() and do so much changes in sq/dd/info_schema/show.cc |
||
|
|
||
| /////////////////////////////////////////////////////////////////////////// | ||
| // Column_impl implementation. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ | |
| #include "sql/dd/string_type.h" | ||
| #include "sql/item_cmpfunc.h" // Item_func_case | ||
| #include "sql/parse_location.h" // POS | ||
| #include "sql/parse_tree_nodes.h" // PT_joined_table_on | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is overly complex solution to simple problem: We need to identify index as vector index in output of SHOW KEYS show statement as well This can be easily done by adjusting INFORMATION_SCHEMA.STATISTICS definition in sql/dd/impl/system_views/statistics.cc instead of involving subqueries and so on. Updated I_S.STATISTICS can simply check if Index has vector_index property set in the options field for the index (see my comment above about placement of vector_index marker) and expose this info in the INDEX_TYPE column of I_S table. |
||
| #include "sql/sql_class.h" | ||
| #include "sql/sql_lex.h" | ||
| #include "sql/table.h" | ||
|
|
@@ -820,6 +821,14 @@ Query_block *build_show_keys_query(const POS &pos, THD *thd, | |
| static const LEX_CSTRING field_expression = {STRING_WITH_LEN("EXPRESSION")}; | ||
| static const LEX_CSTRING alias_expression = {STRING_WITH_LEN("Expression")}; | ||
|
|
||
| // Fields for column type lookup via information_schema.COLUMNS | ||
| static const LEX_CSTRING columns_view_name = {STRING_WITH_LEN("COLUMNS")}; | ||
| static const LEX_CSTRING field_data_type = {STRING_WITH_LEN("DATA_TYPE")}; | ||
| static const LEX_CSTRING alias_data_type = {STRING_WITH_LEN("Data_type")}; | ||
| static const LEX_CSTRING alias_col_db = {STRING_WITH_LEN("Col_db")}; | ||
| static const LEX_CSTRING alias_col_tbl = {STRING_WITH_LEN("Col_tbl")}; | ||
| static const LEX_CSTRING alias_col_name = {STRING_WITH_LEN("Col_name")}; | ||
|
|
||
| // Get the current logged in schema name | ||
| LEX_CSTRING cur_db; | ||
| if (table_ident->db.str) { | ||
|
|
@@ -858,16 +867,83 @@ Query_block *build_show_keys_query(const POS &pos, THD *thd, | |
| sub_query.add_select_item(alias_column_pos, alias_column_pos)) | ||
| return nullptr; | ||
|
|
||
| // ... FROM information_schema.columns ... | ||
| // ... FROM information_schema.SHOW_STATISTICS ... | ||
| if (sub_query.add_from_item(INFORMATION_SCHEMA_NAME, system_view_name)) | ||
| return nullptr; | ||
|
|
||
| // Build columns sub query for column type lookup | ||
| Select_lex_builder cols_query(&pos, thd); | ||
| if (cols_query.add_select_item(field_database, alias_col_db) || | ||
| cols_query.add_select_item(field_table, alias_col_tbl) || | ||
| cols_query.add_select_item(field_column_name, alias_col_name) || | ||
| cols_query.add_select_item(field_data_type, alias_data_type) || | ||
| cols_query.add_from_item(INFORMATION_SCHEMA_NAME, columns_view_name)) | ||
| return nullptr; | ||
|
|
||
| /* | ||
| Build the top level query | ||
|
|
||
| SELECT Table, ..., | ||
| IF(Index_type = 'SE_SPECIFIC', | ||
| IF(Data_type = 'vector', 'VECTOR', Index_type), | ||
| Index_type) AS Index_type, ... | ||
| FROM (SELECT ... FROM information_schema.SHOW_STATISTICS) AS SHOW_STATISTICS | ||
| LEFT JOIN | ||
| (SELECT TABLE_SCHEMA AS Col_db, TABLE_NAME AS Col_tbl, | ||
| COLUMN_NAME AS Col_name, DATA_TYPE AS Data_type | ||
| FROM information_schema.COLUMNS) AS COLUMNS | ||
| ON Column_name = Col_name AND Col_db = <db> AND Col_tbl = <tbl> | ||
| WHERE Database = <db> AND Table = <tbl> | ||
| ORDER BY INDEX_ORDINAL_POSITION, COLUMN_ORDINAL_POSITION | ||
|
|
||
| */ | ||
|
|
||
| Select_lex_builder top_query(&pos, thd); | ||
|
|
||
| Item *index_type_item = | ||
| new (thd->mem_root) Item_field(pos, NullS, NullS, alias_type.str); | ||
| if (index_type_item == nullptr) return nullptr; | ||
|
|
||
| Item *se_specific_item = new (thd->mem_root) | ||
| Item_string(STRING_WITH_LEN("SE_SPECIFIC"), system_charset_info); | ||
| if (se_specific_item == nullptr) return nullptr; | ||
|
|
||
| Item *is_se_specific = | ||
| new (thd->mem_root) Item_func_eq(pos, index_type_item, se_specific_item); | ||
| if (is_se_specific == nullptr) return nullptr; | ||
|
|
||
| Item *data_type_item = | ||
| new (thd->mem_root) Item_field(pos, NullS, NullS, alias_data_type.str); | ||
| if (data_type_item == nullptr) return nullptr; | ||
|
|
||
| Item *vector_type_item = new (thd->mem_root) | ||
| Item_string(STRING_WITH_LEN("vector"), system_charset_info); | ||
| if (vector_type_item == nullptr) return nullptr; | ||
|
|
||
| Item *is_vector_column = | ||
| new (thd->mem_root) Item_func_eq(pos, data_type_item, vector_type_item); | ||
| if (is_vector_column == nullptr) return nullptr; | ||
|
|
||
| Item *vector_item = new (thd->mem_root) | ||
| Item_string(STRING_WITH_LEN("VECTOR"), system_charset_info); | ||
| if (vector_item == nullptr) return nullptr; | ||
|
|
||
| Item *index_type_else_item = | ||
| new (thd->mem_root) Item_field(pos, NullS, NullS, alias_type.str); | ||
| if (index_type_else_item == nullptr) return nullptr; | ||
|
|
||
| Item *index_type_if = new (thd->mem_root) | ||
| Item_func_if(pos, is_vector_column, vector_item, index_type_else_item); | ||
| if (index_type_if == nullptr) return nullptr; | ||
|
|
||
| Item *index_type_default_item = | ||
| new (thd->mem_root) Item_field(pos, NullS, NullS, alias_type.str); | ||
| if (index_type_default_item == nullptr) return nullptr; | ||
|
|
||
| Item *index_type_expr = new (thd->mem_root) | ||
| Item_func_if(pos, is_se_specific, index_type_if, index_type_default_item); | ||
| if (index_type_expr == nullptr) return nullptr; | ||
|
|
||
| // SELECT * FROM <sub_query> ... | ||
| if (top_query.add_select_item(alias_table, alias_table) || | ||
| top_query.add_select_item(alias_non_unique, alias_non_unique) || | ||
|
|
@@ -879,13 +955,59 @@ Query_block *build_show_keys_query(const POS &pos, THD *thd, | |
| top_query.add_select_item(alias_sub_part, alias_sub_part) || | ||
| top_query.add_select_item(alias_packed, alias_packed) || | ||
| top_query.add_select_item(alias_null, alias_null) || | ||
| top_query.add_select_item(alias_type, alias_type) || | ||
| top_query.add_select_expr(index_type_expr, alias_type) || | ||
| top_query.add_select_item(alias_comment, alias_comment) || | ||
| top_query.add_select_item(alias_index_comment, alias_index_comment) || | ||
| top_query.add_select_item(alias_visible, alias_visible) || | ||
| top_query.add_select_item(alias_expression, alias_expression) || | ||
| top_query.add_from_item( | ||
| sub_query.prepare_derived_table(system_view_name))) | ||
| top_query.add_select_item(alias_expression, alias_expression)) | ||
| return nullptr; | ||
|
|
||
| // Build LEFT JOIN between stats and columns derived tables | ||
| PT_derived_table *stats_dt = | ||
| sub_query.prepare_derived_table(system_view_name); | ||
| PT_derived_table *cols_dt = | ||
| cols_query.prepare_derived_table(columns_view_name); | ||
| if (stats_dt == nullptr || cols_dt == nullptr) return nullptr; | ||
|
|
||
| // ON Column_name = Col_name AND Col_db = <db> AND Col_tbl = <tbl> | ||
| Item *on_cn_left = | ||
| new (thd->mem_root) Item_field(pos, NullS, NullS, alias_column_name.str); | ||
| Item *on_cn_right = | ||
| new (thd->mem_root) Item_field(pos, NullS, NullS, alias_col_name.str); | ||
| if (on_cn_left == nullptr || on_cn_right == nullptr) return nullptr; | ||
| Item *on_col_name = | ||
| new (thd->mem_root) Item_func_eq(pos, on_cn_left, on_cn_right); | ||
| if (on_col_name == nullptr) return nullptr; | ||
|
|
||
| Item *on_col_db_field = | ||
| new (thd->mem_root) Item_field(pos, NullS, NullS, alias_col_db.str); | ||
| Item *on_col_db_val = new (thd->mem_root) | ||
| Item_string(cur_db.str, cur_db.length, system_charset_info); | ||
| if (on_col_db_field == nullptr || on_col_db_val == nullptr) return nullptr; | ||
| Item *on_col_db = | ||
| new (thd->mem_root) Item_func_eq(pos, on_col_db_field, on_col_db_val); | ||
| if (on_col_db == nullptr) return nullptr; | ||
|
|
||
| Item *on_col_tbl_field = | ||
| new (thd->mem_root) Item_field(pos, NullS, NullS, alias_col_tbl.str); | ||
| Item *on_col_tbl_val = new (thd->mem_root) | ||
| Item_string(table_ident->table.str, table_ident->table.length, | ||
| system_charset_info); | ||
| if (on_col_tbl_field == nullptr || on_col_tbl_val == nullptr) return nullptr; | ||
| Item *on_col_tbl = | ||
| new (thd->mem_root) Item_func_eq(pos, on_col_tbl_field, on_col_tbl_val); | ||
| if (on_col_tbl == nullptr) return nullptr; | ||
|
|
||
| Item *on_col_name_and_db = | ||
| new (thd->mem_root) Item_cond_and(pos, on_col_name, on_col_db); | ||
| if (on_col_name_and_db == nullptr) return nullptr; | ||
| Item *on_condition = | ||
| new (thd->mem_root) Item_cond_and(pos, on_col_name_and_db, on_col_tbl); | ||
| if (on_condition == nullptr) return nullptr; | ||
|
|
||
| auto *left_join = new (thd->mem_root) | ||
| PT_joined_table_on(pos, stats_dt, pos, JTT_LEFT, cols_dt, on_condition); | ||
| if (left_join == nullptr || top_query.add_from_item(left_join)) | ||
| return nullptr; | ||
|
|
||
| // ... WHERE 'Database' = <dbname> ... | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... Why this define has been placed at this position in the list?
Would not it be better/logical/simpler to do merges if it is put it after HA_MULTI_VALUED_KEY instead?
(HA_KEYFLAG_MASK is a macro so AFAIU HA_VECTOR will be resolved at the point where HA_KEYFLAG_MAS is used, and not defined!).
I would also added a comment explaining choice of constant (1 << 30 vs say 1 << 20 which is HA_MULTI_VALUED_KEY << 1).