Skip to content

Commit

Permalink
MDEV-11672 mysql_list_field() returns wrong default values for VIEW
Browse files Browse the repository at this point in the history
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:
   a. Removes Send_field::charsetnr, as it's been unused since
      introduction of Item::charset_for_protocol() in MySQL-5.5.
   b. Adds the "const" qualifier to Field::char_length().
      This is needed for (5.a), see below.

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::charset_for_protocol() 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, or val_int() for INT/BIGINT.
   This required additional changes:
   a. in DBUG_ASSERT in Protocol::store(const char *,size_t,CHARSET_INFO),
      This method is now used by mysqld_list_fields(), which
      (unlike normal SELECT queries) does not set
      field_types/field_pos/field_count.
   b. Item_ident_for_show::Item_ident_for_show() now set standard attributes
      (collation, decimals, max_length, unsigned_flag) according to the
      referenced field, to make charset_for_protocol() return the correct
      value and to make mysqld_list_fields() correctly send default
      values.

5. In order to share the code between Item_field::set_field() and
   Item_ident_for_show::Item_ident_for_show():
   a. Introduces a new method Type_std_attributes::set(const Field*)
   b. To make (a) possible, moves Item::fix_char_length() from Item
      to Type_std_attributes, also moves char_to_byte_length_safe()
      from item.h to sql_type.h
   c. Additionally, moves Item::fix_length_and_charset() and
      Item::max_char_length() from Item to Type_std_attributes.
      This is not directly needed for the fix and is done just for symmetry
      with fix_char_length(), as these three methods are directly related
      to each other.
  • Loading branch information
Alexander Barkov committed Dec 29, 2016
1 parent d8c695e commit f613888
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 58 deletions.
3 changes: 1 addition & 2 deletions sql/field.cc
Expand Up @@ -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;
Expand Down Expand Up @@ -10747,7 +10746,7 @@ Column_definition::Column_definition(THD *thd, Field *old_field,
length
*/

uint32 Field_blob::char_length()
uint32 Field_blob::char_length() const
{
switch (packlength)
{
Expand Down
6 changes: 3 additions & 3 deletions sql/field.h
Expand Up @@ -1353,7 +1353,7 @@ class Field: public Value_source
longlong convert_decimal2longlong(const my_decimal *val, bool unsigned_flag,
int *err);
/* The max. number of characters */
virtual uint32 char_length()
virtual uint32 char_length() const
{
return field_length / charset()->mbmaxlen;
}
Expand Down Expand Up @@ -3344,7 +3344,7 @@ class Field_blob :public Field_longstr {
bool has_charset(void) const
{ return charset() == &my_charset_bin ? FALSE : TRUE; }
uint32 max_display_length();
uint32 char_length();
uint32 char_length() const;
uint is_equal(Create_field *new_field);
private:
int do_save_field_metadata(uchar *first_byte);
Expand Down Expand Up @@ -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() {}
};
Expand Down
9 changes: 1 addition & 8 deletions sql/item.cc
Expand Up @@ -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 ?
Expand Down Expand Up @@ -2559,15 +2558,11 @@ void Item_field::set_field(Field *field_par)
{
field=result_field=field_par; // for easy coding with fields
maybe_null=field->maybe_null();
decimals= field->decimals();
Type_std_attributes::set(field_par);
table_name= *field_par->table_name;
field_name= field_par->field_name;
db_name= field_par->table->s->db.str;
alias_name_used= field_par->table->alias_name_used;
unsigned_flag= MY_TEST(field_par->flags & UNSIGNED_FLAG);
collation.set(field_par->charset(), field_par->derivation(),
field_par->repertoire());
fix_char_length(field_par->char_length());

max_length= adjust_max_effective_column_length(field_par, max_length);

Expand Down Expand Up @@ -4259,7 +4254,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;
Expand Down Expand Up @@ -5792,7 +5786,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);
Expand Down
38 changes: 6 additions & 32 deletions sql/item.h
Expand Up @@ -96,13 +96,6 @@ enum precedence {

typedef Bounds_checked_array<Item*> Ref_ptr_array;

static inline uint32
char_to_byte_length_safe(uint32 char_length_arg, uint32 mbmaxlen_arg)
{
ulonglong tmp= ((ulonglong) char_length_arg) * mbmaxlen_arg;
return (tmp > UINT_MAX32) ? (uint32) UINT_MAX32 : (uint32) tmp;
}

bool mark_unsupported_function(const char *where, void *store, uint result);

/* convenience helper for mark_unsupported_function() above */
Expand Down Expand Up @@ -1362,14 +1355,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)
Expand Down Expand Up @@ -1687,20 +1675,8 @@ class Item: public Value_source,
{ return Field::GEOM_GEOMETRY; };
String *check_well_formed_result(String *str, bool send_error= 0);
bool eq_by_collation(Item *item, bool binary_cmp, CHARSET_INFO *cs);
uint32 max_char_length() const
{ return max_length / collation.collation->mbmaxlen; }
bool too_big_for_varchar() const
{ return max_char_length() > CONVERT_IF_BIGGER_TO_BLOB; }
void fix_length_and_charset(uint32 max_char_length_arg, CHARSET_INFO *cs)
{
max_length= char_to_byte_length_safe(max_char_length_arg, cs->mbmaxlen);
collation.collation= cs;
}
void fix_char_length(uint32 max_char_length_arg)
{
max_length= char_to_byte_length_safe(max_char_length_arg,
collation.collation->mbmaxlen);
}
/*
Return TRUE if the item points to a column of an outer-joined table.
*/
Expand Down Expand Up @@ -2319,17 +2295,17 @@ 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)
{}
{
Type_std_attributes::set(par_field);
}

enum Type type() const { return FIELD_ITEM; }
double val_real() { return field->val_real(); }
longlong val_int() { return field->val_int(); }
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); }
};
Expand Down Expand Up @@ -2507,8 +2483,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;
Expand Down
12 changes: 0 additions & 12 deletions sql/item_timefunc.h
Expand Up @@ -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().
Expand Down
2 changes: 1 addition & 1 deletion sql/protocol.cc
Expand Up @@ -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 ||
Expand Down
26 changes: 26 additions & 0 deletions sql/sql_type.cc
Expand Up @@ -56,6 +56,15 @@ Type_handler_newdecimal type_handler_newdecimal;
Type_handler_datetime type_handler_datetime;


void Type_std_attributes::set(const Field *field)
{
decimals= field->decimals();
unsigned_flag= MY_TEST(field->flags & UNSIGNED_FLAG);
collation.set(field->charset(), field->derivation(), field->repertoire());
fix_char_length(field->char_length());
}


/**
This method is used by:
- Item_user_var_as_out_param::field_type()
Expand Down Expand Up @@ -101,6 +110,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)
{
Expand Down
22 changes: 22 additions & 0 deletions sql/sql_type.h
Expand Up @@ -176,6 +176,13 @@ class DTCollation {
};


static inline uint32
char_to_byte_length_safe(uint32 char_length_arg, uint32 mbmaxlen_arg)
{
ulonglong tmp= ((ulonglong) char_length_arg) * mbmaxlen_arg;
return (tmp > UINT_MAX32) ? (uint32) UINT_MAX32 : (uint32) tmp;
}

/**
A class to store type attributes for the standard data types.
Does not include attributes for the extended data types
Expand Down Expand Up @@ -217,6 +224,19 @@ class Type_std_attributes
{
*this= other;
}
void set(const Field *field);
uint32 max_char_length() const
{ return max_length / collation.collation->mbmaxlen; }
void fix_length_and_charset(uint32 max_char_length_arg, CHARSET_INFO *cs)
{
max_length= char_to_byte_length_safe(max_char_length_arg, cs->mbmaxlen);
collation.collation= cs;
}
void fix_char_length(uint32 max_char_length_arg)
{
max_length= char_to_byte_length_safe(max_char_length_arg,
collation.collation->mbmaxlen);
}
};


Expand Down Expand Up @@ -246,6 +266,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
Expand Down Expand Up @@ -628,6 +649,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 *
Expand Down
71 changes: 71 additions & 0 deletions tests/mysql_client_test.c
Expand Up @@ -8398,6 +8398,76 @@ 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,"
" i3 BIGINT UNSIGNED NOT NULL DEFAULT 0xFFFFFFFFFFFFFFFF,"
" s1 VARCHAR(10) CHARACTER SET latin1 NOT NULL DEFAULT 's1def',"
" d1 DECIMAL(31,1) NOT NULL DEFAULT 111111111122222222223333333333.9,"
" t1 DATETIME(6) NOT NULL DEFAULT '2001-01-01 10:20:30.123456',"
" e1 ENUM('a','b') NOT NULL DEFAULT 'a'"
")");
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, "i3", "i3", MYSQL_TYPE_LONGLONG,
table_name, table_name, current_db,
20, "18446744073709551615");

verify_prepare_field(result, 2, "s1", "s1", MYSQL_TYPE_VAR_STRING,
table_name, table_name, current_db,
10, "s1def");

verify_prepare_field(result, 3, "d1", "d1", MYSQL_TYPE_NEWDECIMAL,
table_name, table_name, current_db,
33, "111111111122222222223333333333.9");

verify_prepare_field(result, 4, "t1", "t1", MYSQL_TYPE_DATETIME,
table_name, table_name, current_db,
26, "2001-01-01 10:20:30.123456");

verify_prepare_field(result, 5, "e1", "e1", MYSQL_TYPE_STRING,
table_name, table_name, current_db,
1, "a");

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;
Expand Down Expand Up @@ -19558,6 +19628,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 },
Expand Down

0 comments on commit f613888

Please sign in to comment.