diff options
author | Vladislav Vaintroub <wlad@mariadb.com> | 2020-09-22 18:12:58 +0200 |
---|---|---|
committer | Vladislav Vaintroub <wlad@mariadb.com> | 2020-10-13 01:03:33 +0200 |
commit | bfd0a07c6f1e0381a3c6863aaacf1be5e64f4393 (patch) | |
tree | 1e81322bbbe443fd5e457b82c5b6957ef0f5bf77 | |
parent | b4fb15ccd4f2864483f8644c0236e63c814c8beb (diff) | |
download | mariadb-git-bb-10.6-cache-metadata.tar.gz |
MDEV-19237 Do not resend unchanged result set metadata for prepared statementsbb-10.6-cache-metadata
-rw-r--r-- | include/mysql_com.h | 8 | ||||
-rw-r--r-- | include/mysql_version.h.in | 4 | ||||
m--------- | libmariadb | 0 | ||||
-rw-r--r-- | sql/mysqld.cc | 1 | ||||
-rw-r--r-- | sql/protocol.cc | 239 | ||||
-rw-r--r-- | sql/protocol.h | 2 | ||||
-rw-r--r-- | sql/sql_class.h | 43 | ||||
-rw-r--r-- | sql/sql_prepare.cc | 19 | ||||
-rw-r--r-- | tests/mysql_client_test.c | 116 |
9 files changed, 413 insertions, 19 deletions
diff --git a/include/mysql_com.h b/include/mysql_com.h index b4f1a3c8f94..023e3f59754 100644 --- a/include/mysql_com.h +++ b/include/mysql_com.h @@ -304,7 +304,8 @@ enum enum_indicator_type #define MARIADB_CLIENT_STMT_BULK_OPERATIONS (1ULL << 34) /* support of extended metadata (e.g. type/format information) */ #define MARIADB_CLIENT_EXTENDED_METADATA (1ULL << 35) - +/* Do not resend metadata for prepared statements, since 10.6*/ +#define MARIADB_CLIENT_CACHE_METADATA (1ULL << 36) #ifdef HAVE_COMPRESS #define CAN_CLIENT_COMPRESS CLIENT_COMPRESS #else @@ -337,15 +338,16 @@ enum enum_indicator_type CLIENT_PS_MULTI_RESULTS | \ CLIENT_SSL_VERIFY_SERVER_CERT | \ CLIENT_REMEMBER_OPTIONS | \ - MARIADB_CLIENT_PROGRESS | \ CLIENT_PLUGIN_AUTH | \ CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA | \ CLIENT_SESSION_TRACK |\ CLIENT_DEPRECATE_EOF |\ CLIENT_CONNECT_ATTRS |\ + CLIENT_CAN_HANDLE_EXPIRED_PASSWORDS |\ + MARIADB_CLIENT_PROGRESS | \ MARIADB_CLIENT_STMT_BULK_OPERATIONS |\ MARIADB_CLIENT_EXTENDED_METADATA|\ - CLIENT_CAN_HANDLE_EXPIRED_PASSWORDS) + MARIADB_CLIENT_CACHE_METADATA) /* Switch off the flags that are optional and depending on build flags diff --git a/include/mysql_version.h.in b/include/mysql_version.h.in index ad719634335..ed8dd06b9b1 100644 --- a/include/mysql_version.h.in +++ b/include/mysql_version.h.in @@ -18,9 +18,13 @@ #define MYSQL_SERVER_SUFFIX_DEF "@MYSQL_SERVER_SUFFIX@" #define FRM_VER @DOT_FRM_VERSION@ #define MYSQL_VERSION_ID @MYSQL_VERSION_ID@ +#ifndef MYSQL_PORT #define MYSQL_PORT @MYSQL_TCP_PORT@ +#endif #define MYSQL_PORT_DEFAULT @MYSQL_TCP_PORT_DEFAULT@ +#ifndef MYSQL_UNIX_ADDR #define MYSQL_UNIX_ADDR "@MYSQL_UNIX_ADDR@" +#endif #define MYSQL_CONFIG_NAME "my" #define MYSQL_COMPILATION_COMMENT "@COMPILATION_COMMENT@" #define SERVER_MATURITY_LEVEL @SERVER_MATURITY_LEVEL@ diff --git a/libmariadb b/libmariadb -Subproject a746c3af449a8754e78ad7971e59e79af7957cd +Subproject e18eeaee90683e228d3223a0fb4637403eeb9df diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 99ec7c1eb56..530b8dddb5d 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -7172,6 +7172,7 @@ SHOW_VAR status_vars[]= { {"Max_used_connections", (char*) &max_used_connections, SHOW_LONG}, {"Memory_used", (char*) &show_memory_used, SHOW_SIMPLE_FUNC}, {"Memory_used_initial", (char*) &start_memory_used, SHOW_LONGLONG}, + {"Resultset_metadata_skipped", (char *) offsetof(STATUS_VAR, skip_metadata_count),SHOW_LONG_STATUS}, {"Not_flushed_delayed_rows", (char*) &delayed_rows_in_use, SHOW_LONG_NOFLUSH}, {"Open_files", (char*) &my_file_opened, SHOW_SINT}, {"Open_streams", (char*) &my_stream_opened, SHOW_LONG_NOFLUSH}, diff --git a/sql/protocol.cc b/sql/protocol.cc index d5e6a7b92bf..756bc47c7fa 100644 --- a/sql/protocol.cc +++ b/sql/protocol.cc @@ -910,6 +910,205 @@ bool Protocol_text::store_field_metadata(const THD * thd, return false; } +/* + MARIADB_CLIENT_CACHE_METADATA support. + + Bulk of the code below is dedicated to detecting whether column metadata has + changed after prepare, or between executions of a prepared statement. + + For some prepared statements, metadata can't change without going through + Prepared_Statement::reprepare(), which makes detecting changes easy. + + Others, "SELECT ?" & Co, are more fragile, and sensitive to input parameters, + or user variables. Detecting metadata change for this class of PS is harder, + we calculate signature (hash value), and check whether this changes between + executions. This is a more expensive method. +*/ + +/* + Detect whether Item is a PS parameter, or user variable, + or SP output parameter, or a function that depends on one + of those. + + It is not easy to detect Item changes, that affect + columninfo sent to the client. These changes do not go through + reprepare. + + TODO : does not work due to MDEV-23913. One can trust nothing, + everything about prepared statements is fragile. +*/ +static bool is_fragile_columnifo(Item *it) +{ +#define MDEV_23913_FIXED 0 +#if MDEV_23913_FIXED + if (dynamic_cast<Item_param *>(it)) + return true; + + if (dynamic_cast<Item_func_user_var *>(it)) + return true; + + if (dynamic_cast <Item_sp_variable*>(it)) + return true; + + /* Check arguments of functions.*/ + auto item_args= dynamic_cast<Item_args *>(it); + if (!item_args) + return false; + auto args= item_args->arguments(); + auto arg_count= item_args->argument_count(); + for (uint i= 0; i < arg_count; i++) + { + if (is_fragile_columnifo(args[i])) + return true; + } + return false; +#else /* MDEV-23913 fixed*/ + return true; +#endif +} + +#define INVALID_METADATA_CHECKSUM 0 +/* + Calculate signature for column info sent to the client as CRC32 over data, + that goes into the column info packet. + We assume that if checksum does not change, then column info was not modified. +*/ +static uint32 calc_metadata_hash(THD *thd, List<Item> *list) +{ + List_iterator_fast<Item> it(*list); + Item *item; + uint32 crc32_c= 0; + while ((item= it++)) + { + Send_field field(thd, item); + auto field_type= item->type_handler()->field_type(); + auto charset= item->charset_for_protocol(); + /* + The data below should contain everything that influences + content of the column info packet. + */ + LEX_CSTRING data[]= + { + field.table_name, + field.org_table_name, + field.col_name, + field.org_col_name, + field.db_name, + field.attr(MARIADB_FIELD_ATTR_DATA_TYPE_NAME), + field.attr(MARIADB_FIELD_ATTR_FORMAT_NAME), + {(const char *) &field.length, sizeof(field.length)}, + {(const char *) &field.flags, sizeof(field.flags)}, + {(const char *) &field.decimals, sizeof(field.decimals)}, + {(const char *) &charset, sizeof(charset)}, + {(const char *) &field_type, sizeof(field_type)}, + }; + for (const auto &chunk : data) + crc32_c= my_crc32c(crc32_c, chunk.str, chunk.length); + } + + if (crc32_c == INVALID_METADATA_CHECKSUM) + return 1; + return crc32_c; +} + +/* + Return if metadata columns have changed since last call to this function. +*/ +static bool metadata_columns_changed(send_column_info_state &state, THD *thd, + List<Item> &list) +{ + if (!state.initialized) + { + state.initialized= true; + state.immutable= true; + Item *item; + List_iterator_fast<Item> it(list); + while ((item= it++)) + { + if (is_fragile_columnifo(item)) + { + state.immutable= false; + state.checksum= calc_metadata_hash(thd, &list); + break; + } + } + state.last_charset= thd->variables.character_set_client; + return true; + } + + /* + Since column info can change under our feet, we use more expensive + checksumming to check if column metadata has not changed since last time. + */ + if (!state.immutable) + { + uint32 checksum= calc_metadata_hash(thd, &list); + if (checksum != state.checksum) + { + state.checksum= checksum; + state.last_charset= thd->variables.character_set_client; + return true; + } + } + + /* + Character_set_client influences result set metadata, thus resend metadata + whenever it changes. + */ + if (state.last_charset != thd->variables.character_set_client) + { + state.last_charset= thd->variables.character_set_client; + return true; + } + + return false; +} + +/* + Determine whether column info must be sent to the client. + Skip column info, if client supports caching, and (prepared) statement + output fields have not changed. +*/ +static bool should_send_column_info(THD* thd, List<Item>* list, uint flags) +{ + if (!(thd->client_capabilities & MARIADB_CLIENT_CACHE_METADATA)) + { + /* Client does not support abbreviated metadata.*/ + return true; + } + + if (!thd->cur_stmt) + { + /* Neither COM_PREPARE nor COM_EXECUTE run.*/ + return true; + } + + if (thd->spcont) + { + /* Always sent full metadata from inside the stored procedure.*/ + return true; + } + + if (flags & Protocol::SEND_FORCE_COLUMN_INFO) + return true; + + auto &column_info_state= thd->cur_stmt->column_info_state; +#ifndef DBUG_OFF + auto cmd= thd->get_command(); +#endif + + DBUG_ASSERT(cmd == COM_STMT_EXECUTE || cmd == COM_STMT_PREPARE); + DBUG_ASSERT(cmd != COM_STMT_PREPARE || !column_info_state.initialized); + + bool ret= metadata_columns_changed(column_info_state, thd, *list); + + DBUG_ASSERT(cmd != COM_STMT_PREPARE || ret); + if (!ret) + thd->status_var.skip_metadata_count++; + + return ret; +} + /** Send name and type of result to client. @@ -936,30 +1135,44 @@ bool Protocol::send_result_set_metadata(List<Item> *list, uint flags) Protocol_text prot(thd, thd->variables.net_buffer_length); DBUG_ENTER("Protocol::send_result_set_metadata"); + bool send_column_info= should_send_column_info(thd, list, flags); + if (flags & SEND_NUM_ROWS) - { // Packet with number of elements - uchar buff[MAX_INT_WIDTH]; + { + /* + Packet with number of columns. + + Will also have a 1 byte column info indicator, in case + MARIADB_CLIENT_CACHE_METADATA client capability is set. + */ + uchar buff[MAX_INT_WIDTH+1]; uchar *pos= net_store_length(buff, list->elements); + if (thd->client_capabilities & MARIADB_CLIENT_CACHE_METADATA) + *pos++= (uchar)send_column_info; + DBUG_ASSERT(pos <= buff + sizeof(buff)); if (my_net_write(&thd->net, buff, (size_t) (pos-buff))) DBUG_RETURN(1); } + if (send_column_info) + { #ifndef DBUG_OFF - field_handlers= (const Type_handler**) thd->alloc(sizeof(field_handlers[0]) * - list->elements); + field_handlers= (const Type_handler **) thd->alloc( + sizeof(field_handlers[0]) * list->elements); #endif - for (uint pos= 0; (item=it++); pos++) - { - prot.prepare_for_resend(); - if (prot.store_item_metadata(thd, item, pos)) - goto err; - if (prot.write()) - DBUG_RETURN(1); + for (uint pos= 0; (item= it++); pos++) + { + prot.prepare_for_resend(); + if (prot.store_item_metadata(thd, item, pos)) + goto err; + if (prot.write()) + DBUG_RETURN(1); #ifndef DBUG_OFF - field_handlers[pos]= item->type_handler(); + field_handlers[pos]= item->type_handler(); #endif + } } if (flags & SEND_EOF) @@ -1683,7 +1896,7 @@ bool Protocol_binary::send_out_parameters(List<Item_param> *sp_params) thd->server_status|= SERVER_PS_OUT_PARAMS | SERVER_MORE_RESULTS_EXISTS; /* Send meta-data. */ - if (send_result_set_metadata(&out_param_lst, SEND_NUM_ROWS | SEND_EOF)) + if (send_result_set_metadata(&out_param_lst, SEND_NUM_ROWS | SEND_EOF|SEND_FORCE_COLUMN_INFO)) return TRUE; /* Send data. */ diff --git a/sql/protocol.h b/sql/protocol.h index 19899885599..a1868342ab4 100644 --- a/sql/protocol.h +++ b/sql/protocol.h @@ -93,7 +93,7 @@ public: virtual ~Protocol() {} void init(THD* thd_arg); - enum { SEND_NUM_ROWS= 1, SEND_EOF= 2 }; + enum { SEND_NUM_ROWS= 1, SEND_EOF= 2, SEND_FORCE_COLUMN_INFO= 4 }; virtual bool send_result_set_metadata(List<Item> *list, uint flags); bool send_list_fields(List<Field> *list, const TABLE_LIST *table_list); bool send_result_set_row(List<Item> *row_items); diff --git a/sql/sql_class.h b/sql/sql_class.h index b0f68aa8fab..8ad3ecb3153 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -929,6 +929,12 @@ typedef struct system_status_var ulong lost_connections; ulong max_statement_time_exceeded; /* + Number of times where column info was not + sent with prepared statement metadata. + */ + ulong skip_metadata_count; + + /* Number of statements sent from the client */ ulong questions; @@ -947,6 +953,7 @@ typedef struct system_status_var ulonglong table_open_cache_hits; ulonglong table_open_cache_misses; ulonglong table_open_cache_overflows; + ulonglong send_metadata_skips; double last_query_cost; double cpu_time, busy_time; uint32 threads_running; @@ -1192,6 +1199,38 @@ public: class Server_side_cursor; +/* + Struct to catch changes in column metadata that is sent to client. + in the "result set metadata". Used to support + MARIADB_CLIENT_CACHE_METADATA. +*/ +struct send_column_info_state +{ + /* Last client charset (affects metadata) */ + CHARSET_INFO *last_charset= nullptr; + + /* Checksum, only used to check changes if 'immutable' is false*/ + uint32 checksum= 0; + + /* + Column info can only be changed by PreparedStatement::reprepare() + + There is a class of "weird" prepared statements like SELECT ? or SELECT @a + that are not immutable, and depend on input parameters or user variables + */ + bool immutable= false; + + bool initialized= false; + + /* Used by PreparedStatement::reprepare()*/ + void reset() + { + initialized= false; + checksum= 0; + } +}; + + /** @class Statement @brief State of a single command executed against this connection. @@ -1281,6 +1320,8 @@ public: LEX_CSTRING db; + send_column_info_state column_info_state; + /* This is set to 1 of last call to send_result_to_client() was ok */ my_bool query_cache_is_applicable; @@ -2381,6 +2422,8 @@ public: /* Last created prepared statement */ Statement *last_stmt; + Statement *cur_stmt= 0; + inline void set_last_stmt(Statement *stmt) { last_stmt= (is_error() ? NULL : stmt); } inline void clear_last_stmt() { last_stmt= NULL; } diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index 6753b857b54..261f745cb24 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -328,9 +328,13 @@ static bool send_prep_stmt(Prepared_statement *stmt, uint columns) error= my_net_write(net, buff, sizeof(buff)); if (stmt->param_count && likely(!error)) { + /* + Force the column info to be written + (in this case PS parameter type info). + */ error= thd->protocol_text.send_result_set_metadata((List<Item> *) &stmt->lex->param_list, - Protocol::SEND_EOF); + Protocol::SEND_EOF|Protocol::SEND_FORCE_COLUMN_INFO); } if (likely(!error)) @@ -3222,10 +3226,15 @@ static void mysql_stmt_execute_common(THD *thd, thd->protocol= &thd->protocol_binary; MYSQL_EXECUTE_PS(thd->m_statement_psi, stmt->m_prepared_stmt); + auto save_cur_stmt= thd->cur_stmt; + thd->cur_stmt= stmt; + if (!bulk_op) stmt->execute_loop(&expanded_query, open_cursor, packet, packet_end); else stmt->execute_bulk_loop(&expanded_query, open_cursor, packet, packet_end); + + thd->cur_stmt= save_cur_stmt; thd->protocol= save_protocol; sp_cache_enforce_limit(thd->sp_proc_cache, stored_program_cache_size); @@ -3235,7 +3244,6 @@ static void mysql_stmt_execute_common(THD *thd, /* Close connection socket; for use with client testing (Bug#43560). */ DBUG_EXECUTE_IF("close_conn_after_stmt_execute", vio_shutdown(thd->net.vio,SHUT_RD);); - DBUG_VOID_RETURN; } @@ -3971,6 +3979,8 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) old_stmt_arena= thd->stmt_arena; thd->stmt_arena= this; + auto save_cur_stmt= thd->cur_stmt; + thd->cur_stmt= this; Parser_state parser_state; if (parser_state.init(thd, thd->query(), thd->query_length())) @@ -3978,6 +3988,7 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) thd->restore_backup_statement(this, &stmt_backup); thd->restore_active_arena(this, &stmt_backup); thd->stmt_arena= old_stmt_arena; + thd->cur_stmt = save_cur_stmt; DBUG_RETURN(TRUE); } @@ -3987,6 +3998,7 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) lex_start(thd); lex->context_analysis_only|= CONTEXT_ANALYSIS_ONLY_PREPARE; + error= (parse_sql(thd, & parser_state, NULL) || thd->is_error() || init_param_array(this)); @@ -4062,6 +4074,7 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) cleanup_stmt(); thd->restore_backup_statement(this, &stmt_backup); thd->stmt_arena= old_stmt_arena; + thd->cur_stmt= save_cur_stmt; if (likely(error == 0)) { @@ -4090,6 +4103,7 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) if (thd->spcont == NULL) general_log_write(thd, COM_STMT_PREPARE, query(), query_length()); } + DBUG_RETURN(error); } @@ -4503,6 +4517,7 @@ Prepared_statement::reprepare() it's failed, we need to return all the warnings to the user. */ thd->get_stmt_da()->clear_warning_info(thd->query_id); + column_info_state.reset(); } return error; } diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index e656b85ab65..b6a08e59ded 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -19200,6 +19200,8 @@ static void test_bug57058() static void test_bug11766854() { struct st_mysql_client_plugin *plugin; + if (getenv("QA_AUTH_CLIENT_SO") == NULL) + return; /*plugin not built.*/ DBUG_ENTER("test_bug11766854"); myheader("test_bug11766854"); @@ -20996,6 +20998,119 @@ static void test_execute_direct() #endif } +static void assert_metadata_skipped_count_equals(MYSQL *mysql, int val) +{ + MYSQL_ROW row; + MYSQL_RES *result; + int rc= mysql_query(mysql, "SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE " + "VARIABLE_NAME='Resultset_metadata_skipped'"); + myquery(rc); + result= mysql_use_result(mysql); + mytest(result); + row= mysql_fetch_row(result); + DIE_UNLESS(atoi(row[0]) == val); + mysql_free_result(result); +} + +static void flush_session_status(MYSQL* mysql) +{ + int rc= mysql_query(mysql, "flush status"); + myquery(rc); +} + +static void exec_stmt(MYSQL_STMT* stmt) +{ + int rc= mysql_stmt_execute(stmt); + check_execute(stmt, rc); + rc= mysql_stmt_store_result(stmt); + check_execute(stmt, rc); +} + +static void test_cache_metadata() +{ + char char_val[]= "blah"; + int int_val = 1; + MYSQL_BIND param= {0}; + my_bool is_null= FALSE; + + + MYSQL_STMT* stmt= mysql_stmt_init(mysql); + check_stmt(stmt); + int rc= mysql_stmt_prepare(stmt, "SELECT ?", -1); + myquery(rc); + + param.buffer= char_val; + param.buffer_type= MYSQL_TYPE_STRING; + param.is_null= &is_null; + param.buffer_length = 4; + + rc= mysql_stmt_bind_param(stmt,¶m); + exec_stmt(stmt); + + flush_session_status(mysql); + /* Execute the statement again, check that metadata is skipped*/ + exec_stmt(stmt); + assert_metadata_skipped_count_equals(mysql, 1); + + flush_session_status(mysql); + /* + Execute the statement again, such that metadata changes, + (using LONG parameter in bind for "SELECT ?", instead of string. + Check that metadata is NOT skipped. + */ + param.buffer= &int_val; + param.buffer_type= MYSQL_TYPE_LONG; + param.is_null= &is_null; + rc= mysql_stmt_bind_param(stmt, ¶m); + exec_stmt(stmt); + assert_metadata_skipped_count_equals(mysql, 0); + mysql_stmt_close(stmt); + + + /* + Test with real table, and DDL which causes column info to be + changed. + */ + stmt= mysql_stmt_init(mysql); + + rc= mysql_query( + mysql, "CREATE OR REPLACE TABLE t1 (a int, b bigint) engine=memory"); + myquery(rc); + + flush_session_status(mysql); + check_stmt(stmt); + rc= mysql_stmt_prepare(stmt, "SELECT * from t1", -1); + myquery(rc); + + exec_stmt(stmt); + /* Metadata skipped, since already sent with COM_STMT_PREPARE result.*/ + assert_metadata_skipped_count_equals(mysql, 1); + + flush_session_status(mysql); + exec_stmt(stmt); + /* Metadata skipped again*/ + assert_metadata_skipped_count_equals(mysql, 1); + + rc= mysql_query(mysql, "ALTER TABLE t1 MODIFY b CHAR(10)"); + myquery(rc); + + /* Column metadata WILL change for the next execution due to DDL*/ + flush_session_status(mysql); + exec_stmt(stmt); + assert_metadata_skipped_count_equals(mysql, 0); + + /* On reexecution, PS column metadata will NOT change. */ + flush_session_status(mysql); + exec_stmt(stmt); + assert_metadata_skipped_count_equals(mysql, 1); + + rc= mysql_query(mysql, "DROP TABLE t1"); + myquery(rc); + + mysql_stmt_close(stmt); +} + + static struct my_tests_st my_tests[]= { { "disable_query_logs", disable_query_logs }, { "test_view_sp_list_fields", test_view_sp_list_fields }, @@ -21292,6 +21407,7 @@ static struct my_tests_st my_tests[]= { { "test_mdev18408", test_mdev18408 }, { "test_mdev20261", test_mdev20261 }, { "test_execute_direct", test_execute_direct }, + { "test_cache_metadata", test_cache_metadata}, { 0, 0 } }; |