Hello Sanja, Can you please review a patch for MDEV-11672?
Thanks!
commit c41ccee0fcf5d5354faceadcb77e2e508075be74 Author: Alexander Barkov <[email protected]> Date: Tue Dec 27 19:44:54 2016 +0400 MDEV-11672 mysql_list_field() returns wrong default values for VIEW The problem happened because Item_ident_for_show::field_type() always returned MYSQL_TYPE_DOUBLE and ignored the actual data type of the referenced Field. As a result, the execution always used Item_ident_for_show::val_real() to send the default value of the field, so most default values for non-numeric types were displayed as '0'. This patch: 1. (Cleanup) Removes Send_field::charsetnr, as it's been unused since introduction of Item::charset_for_protocol() in MySQL-5.5. 2. Introduces a new virtual method Type_handler::charset_for_protocol(), returning item->collation.collation for string data types, or &my_charset_bin for non-string data types. 3. Changes Item::charset_for_protocol() from virtual to non-virtual. It now calls type_handler()->charset_for_protocol(). As a good side effect, duplicate code in Item::charset_for_protocol() and Item_temporal_hybrid_func() is now gone. 4. Fixes Item_ident_for_show::field_type() to correctly return its data type according to the data type of the referenced field. This actually fixes the problem reported in MDEV-11672. Now the default value is sent using a correct method, e.g. val_str() for VARCHAR/TEXT. This required additional changes: - in DBUG_ASSERT in Protocol::store(const char *,size_t,CHARSET_INFO), This method is now used by mysql_list_fields(), which (unlike normal SELECT queries) does not set field_types/field_pos/field_count. - Item_ident_for_show::Item_ident_for_show() now sets the collation according to the referenced field, to make charset_for_protocol() return the correct value. diff --git a/sql/field.cc b/sql/field.cc index 480fcfb..592445d 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1944,7 +1944,6 @@ void Field::make_field(Send_field *field) field->org_col_name= ""; } field->col_name= field_name; - field->charsetnr= charset()->number; field->length=field_length; field->type=type(); field->flags=table->maybe_null ? (flags & ~NOT_NULL_FLAG) : flags; diff --git a/sql/field.h b/sql/field.h index 7410288..3d7cd77 100644 --- a/sql/field.h +++ b/sql/field.h @@ -3939,7 +3939,7 @@ class Send_field :public Sql_alloc { const char *table_name,*org_table_name; const char *col_name,*org_col_name; ulong length; - uint charsetnr, flags, decimals; + uint flags, decimals; enum_field_types type; Send_field() {} }; diff --git a/sql/item.cc b/sql/item.cc index 2a10587..3cb0276 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -2397,7 +2397,6 @@ void Item_ident_for_show::make_field(THD *thd, Send_field *tmp_field) tmp_field->table_name= tmp_field->org_table_name= table_name; tmp_field->db_name= db_name; tmp_field->col_name= tmp_field->org_col_name= field->field_name; - tmp_field->charsetnr= field->charset()->number; tmp_field->length=field->field_length; tmp_field->type=field->type(); tmp_field->flags= field->table->maybe_null ? @@ -4259,7 +4258,6 @@ void Item_param::make_field(THD *thd, Send_field *field) field->org_col_name= m_out_param_info->org_col_name; field->length= m_out_param_info->length; - field->charsetnr= m_out_param_info->charsetnr; field->flags= m_out_param_info->flags; field->decimals= m_out_param_info->decimals; field->type= m_out_param_info->type; @@ -5792,7 +5790,6 @@ void Item::init_make_field(Send_field *tmp_field, tmp_field->org_col_name= empty_name; tmp_field->table_name= empty_name; tmp_field->col_name= name; - tmp_field->charsetnr= collation.collation->number; tmp_field->flags= (maybe_null ? 0 : NOT_NULL_FLAG) | (my_binary_compare(charset_for_protocol()) ? BINARY_FLAG : 0); diff --git a/sql/item.h b/sql/item.h index ed7d07e..d62ffbc 100644 --- a/sql/item.h +++ b/sql/item.h @@ -1362,14 +1362,9 @@ class Item: public Value_source, static CHARSET_INFO *default_charset(); - /* - For backward compatibility, to make numeric - data types return "binary" charset in client-side metadata. - */ - virtual CHARSET_INFO *charset_for_protocol(void) const + CHARSET_INFO *charset_for_protocol(void) const { - return cmp_type() == STRING_RESULT ? collation.collation : - &my_charset_bin; + return type_handler()->charset_for_protocol(this); }; virtual bool walk(Item_processor processor, bool walk_subquery, void *arg) @@ -2319,7 +2314,9 @@ class Item_ident_for_show :public Item Item_ident_for_show(THD *thd, Field *par_field, const char *db_arg, const char *table_name_arg): Item(thd), field(par_field), db_name(db_arg), table_name(table_name_arg) - {} + { + collation.set(par_field->charset()); + } enum Type type() const { return FIELD_ITEM; } double val_real() { return field->val_real(); } @@ -2327,9 +2324,7 @@ class Item_ident_for_show :public Item String *val_str(String *str) { return field->val_str(str); } my_decimal *val_decimal(my_decimal *dec) { return field->val_decimal(dec); } void make_field(THD *thd, Send_field *tmp_field); - CHARSET_INFO *charset_for_protocol(void) const - { return field->charset_for_protocol(); } - enum_field_types field_type() const { return MYSQL_TYPE_DOUBLE; } + enum_field_types field_type() const { return field->type(); } Item* get_copy(THD *thd, MEM_ROOT *mem_root) { return get_item_copy<Item_ident_for_show>(thd, mem_root, this); } }; @@ -2507,8 +2502,6 @@ class Item_field :public Item_ident DBUG_ASSERT(field_type() == MYSQL_TYPE_GEOMETRY); return field->get_geometry_type(); } - CHARSET_INFO *charset_for_protocol(void) const - { return field->charset_for_protocol(); } friend class Item_default_value; friend class Item_insert_value; friend class st_select_lex_unit; diff --git a/sql/item_timefunc.h b/sql/item_timefunc.h index ad92f50..1014326 100644 --- a/sql/item_timefunc.h +++ b/sql/item_timefunc.h @@ -573,18 +573,6 @@ class Item_temporal_hybrid_func: public Item_temporal_func, { return Type_handler_hybrid_field_type::result_type(); } enum Item_result cmp_type () const { return Type_handler_hybrid_field_type::cmp_type(); } - CHARSET_INFO *charset_for_protocol() const - { - /* - Can return TIME, DATE, DATETIME or VARCHAR depending on arguments. - Send using "binary" when TIME, DATE or DATETIME, - or using collation.collation when VARCHAR - (which is fixed from @@collation_connection in fix_length_and_dec). - */ - DBUG_ASSERT(fixed == 1); - return Item_temporal_hybrid_func::field_type() == MYSQL_TYPE_STRING ? - collation.collation : &my_charset_bin; - } /** Fix the returned timestamp to match field_type(), which is important for val_str(). diff --git a/sql/protocol.cc b/sql/protocol.cc index f8b68c0..3cc7307 100644 --- a/sql/protocol.cc +++ b/sql/protocol.cc @@ -1116,7 +1116,7 @@ bool Protocol_text::store(const char *from, size_t length, #ifndef DBUG_OFF DBUG_PRINT("info", ("Protocol_text::store field %u (%u): %.*s", field_pos, field_count, (int) length, (length == 0 ? "" : from))); - DBUG_ASSERT(field_pos < field_count); + DBUG_ASSERT(field_types == 0 || field_pos < field_count); DBUG_ASSERT(field_types == 0 || field_types[field_pos] == MYSQL_TYPE_DECIMAL || field_types[field_pos] == MYSQL_TYPE_BIT || diff --git a/sql/sql_type.cc b/sql/sql_type.cc index 0f6424e..6ff72c0 100644 --- a/sql/sql_type.cc +++ b/sql/sql_type.cc @@ -101,6 +101,23 @@ Type_handler_string_result::type_handler_adjusted_to_max_octet_length( } +CHARSET_INFO *Type_handler::charset_for_protocol(const Item *item) const +{ + /* + For backward compatibility, to make numeric + data types return "binary" charset in client-side metadata. + */ + return &my_charset_bin; +} + + +CHARSET_INFO * +Type_handler_string_result::charset_for_protocol(const Item *item) const +{ + return item->collation.collation; +} + + const Type_handler * Type_handler::get_handler_by_cmp_type(Item_result type) { diff --git a/sql/sql_type.h b/sql/sql_type.h index e1a5883..b57c070 100644 --- a/sql/sql_type.h +++ b/sql/sql_type.h @@ -246,6 +246,7 @@ class Type_handler virtual Item_result result_type() const= 0; virtual Item_result cmp_type() const= 0; virtual const Type_handler *type_handler_for_comparison() const= 0; + virtual CHARSET_INFO *charset_for_protocol(const Item *item) const; virtual const Type_handler* type_handler_adjusted_to_max_octet_length(uint max_octet_length, CHARSET_INFO *cs) const @@ -628,6 +629,7 @@ class Type_handler_string_result: public Type_handler public: Item_result result_type() const { return STRING_RESULT; } Item_result cmp_type() const { return STRING_RESULT; } + CHARSET_INFO *charset_for_protocol(const Item *item) const; virtual ~Type_handler_string_result() {} const Type_handler *type_handler_for_comparison() const; const Type_handler * diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index 58e3dda..9e997c2 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -8398,6 +8398,53 @@ static void test_list_fields() } +static void test_list_fields_default() +{ + int rc, i; + myheader("test_list_fields_default"); + + rc= mysql_query(mysql, "DROP TABLE IF EXISTS t1"); + myquery(rc); + + rc= mysql_query(mysql, + "CREATE TABLE t1 (" + " i1 INT NOT NULL DEFAULT 0," + " s1 VARCHAR(10) CHARACTER SET latin1 NOT NULL DEFAULT 's1def')"); + myquery(rc); + + rc= mysql_query(mysql, "DROP VIEW IF EXISTS v1"); + myquery(rc); + + rc= mysql_query(mysql, "CREATE VIEW v1 AS SELECT * FROM t1"); + myquery(rc); + + /* + Checking that mysql_list_fields() returns the same result + for a TABLE and a VIEW on the same table. + */ + for (i= 0; i < 2; i++) + { + const char *table_name= i == 0 ? "t1" : "v1"; + MYSQL_RES *result= mysql_list_fields(mysql, table_name, NULL); + mytest(result); + + rc= my_process_result_set(result); + DIE_UNLESS(rc == 0); + + verify_prepare_field(result, 0, "i1", "i1", MYSQL_TYPE_LONG, + table_name, table_name, current_db, 11, "0"); + + verify_prepare_field(result, 1, "s1", "s1", MYSQL_TYPE_VAR_STRING, + table_name, table_name, current_db, 10, "s1def"); + + mysql_free_result(result); + } + + myquery(mysql_query(mysql, "DROP VIEW v1")); + myquery(mysql_query(mysql, "DROP TABLE t1")); +} + + static void test_bug19671() { MYSQL_RES *result; @@ -19558,6 +19605,7 @@ static struct my_tests_st my_tests[]= { { "test_fetch_column", test_fetch_column }, { "test_mem_overun", test_mem_overun }, { "test_list_fields", test_list_fields }, + { "test_list_fields_default", test_list_fields_default }, { "test_free_result", test_free_result }, { "test_free_store_result", test_free_store_result }, { "test_sqlmode", test_sqlmode },
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
