Commit 9aaf62d0 authored by Alexander Barkov's avatar Alexander Barkov

MDEV-15926 MEDIUMINT returns wrong I_S attributes

Problem:

The logic in store_column_type() with a switch on field type was
hard to follow. The part for MEDIUMINT (MYSQL_TYPE_INT24) was not correct.
It erroneously calculated the precision of MEDIUMINT UNSIGNED
as 7 instead of 8.

A similar hard-to-follow switch doing some type specific calculations
resided in adjust_max_effective_column_length(). It was also wrong for
MEDIUMINT (reported as a separate issue in MDEV-15946).

Solution:

1. Introducing a new class Information_schema_numeric_attributes
2. Adding a new virtual method Field::information_schema_numeric_attributes()
3. Splitting the logic in store_column_type() into virtual
   implementations of information_schema_numeric_attributes().
4. In order to avoid adding duplicate code for the integer data types,
   adding a new virtual method Field_int::numeric_precision(),
   which returns the number of digits.

Additional changes:

1. Adding the "const" qualifier to Field::max_display_length()

2. Moving the code from adjust_max_effective_column_length()
  directly to Field::max_display_length().
  There was no any sense to have two implementations:
  - a set of wrong virtual implementations for Field_xxx::max_display_length()
  - additional code in adjust_max_effective_column_length() fixing
    bad results of Field_xxx::max_display_length()
  This change is safe:
  - The code using Field::max_display_length()
    in field.cc, sql_show.cc, sql_type.cc is not affected.
  - The code in rpl_utility.cc is also not affected.
    See a new DBUG_ASSSERT and new comments explaining why.

  In the new reduction, Field_xxx::max_display_length() returns
  correct results for all integer types (except MEDIUMINT, see below).

  Putting implementations of numeric_precision() and max_display_length()
  near each other in field.h made the logic much clearer and thus
  helped to reveal bad results for Field_medium::max_display_length(),
  which returns 9 instead of 8 for signed MEDIUMINT fields.
  This problem will be addressed separately (MDEV-15946).

Note, this change is also useful for pluggable data types (see MDEV-4912),
as now a user defined Field_xxx has a way to control what's returned
in INFORMATION_SCHEMA.COLUMNS.NUMERIC_PRECISION and
INFORMATION_SCHEMA.COLUMNS.NUMERIC_SCALE by implementing
a desired behavior in Field_xxx::information_schema_numeric_attributes().
parent 38c799c9
......@@ -93,3 +93,18 @@ DROP TABLE t1;
#
# End of 10.2 tests
#
#
# Start of 10.3 tests
#
#
# MDEV-15926 MEDIUMINT returns wrong I_S attributes
#
CREATE TABLE t1 (a MEDIUMINT, b MEDIUMINT UNSIGNED);
SELECT COLUMN_NAME, NUMERIC_PRECISION FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA='test' AND TABLE_NAME='t1' ORDER BY COLUMN_NAME;
COLUMN_NAME NUMERIC_PRECISION
a 7
b 8
DROP TABLE t1;
#
# End of 10.3 tests
#
......@@ -76,3 +76,20 @@ DROP TABLE t1;
--echo #
--echo # End of 10.2 tests
--echo #
--echo #
--echo # Start of 10.3 tests
--echo #
--echo #
--echo # MDEV-15926 MEDIUMINT returns wrong I_S attributes
--echo #
CREATE TABLE t1 (a MEDIUMINT, b MEDIUMINT UNSIGNED);
SELECT COLUMN_NAME, NUMERIC_PRECISION FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA='test' AND TABLE_NAME='t1' ORDER BY COLUMN_NAME;
DROP TABLE t1;
--echo #
--echo # End of 10.3 tests
--echo #
......@@ -427,9 +427,9 @@ def test tb1 f19 19 NULL YES smallint NULL NULL 5 0 NULL NULL NULL smallint(5) u
def test tb1 f2 2 NULL YES char 0 0 NULL NULL NULL latin1 latin1_bin char(0) select,insert,update,references NEVER NULL
def test tb1 f20 20 NULL YES smallint NULL NULL 5 0 NULL NULL NULL smallint(5) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f21 21 NULL YES mediumint NULL NULL 7 0 NULL NULL NULL mediumint(9) select,insert,update,references NEVER NULL
def test tb1 f22 22 NULL YES mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned select,insert,update,references NEVER NULL
def test tb1 f23 23 NULL YES mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f24 24 NULL YES mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f22 22 NULL YES mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned select,insert,update,references NEVER NULL
def test tb1 f23 23 NULL YES mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f24 24 NULL YES mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f25 25 NULL YES int NULL NULL 10 0 NULL NULL NULL int(11) select,insert,update,references NEVER NULL
def test tb1 f26 26 NULL YES int NULL NULL 10 0 NULL NULL NULL int(10) unsigned select,insert,update,references NEVER NULL
def test tb1 f27 27 NULL YES int NULL NULL 10 0 NULL NULL NULL int(10) unsigned zerofill select,insert,update,references NEVER NULL
......@@ -543,9 +543,9 @@ def test tb3 f135 18 999 NO smallint NULL NULL 5 0 NULL NULL NULL smallint(5) un
def test tb3 f136 19 00999 NO smallint NULL NULL 5 0 NULL NULL NULL smallint(5) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f137 20 00999 NO smallint NULL NULL 5 0 NULL NULL NULL smallint(5) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f138 21 9999 NO mediumint NULL NULL 7 0 NULL NULL NULL mediumint(9) select,insert,update,references NEVER NULL
def test tb3 f139 22 9999 NO mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned select,insert,update,references NEVER NULL
def test tb3 f140 23 00009999 NO mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f141 24 00009999 NO mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f139 22 9999 NO mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned select,insert,update,references NEVER NULL
def test tb3 f140 23 00009999 NO mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f141 24 00009999 NO mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f142 25 99999 NO int NULL NULL 10 0 NULL NULL NULL int(11) select,insert,update,references NEVER NULL
def test tb3 f143 26 99999 NO int NULL NULL 10 0 NULL NULL NULL int(10) unsigned select,insert,update,references NEVER NULL
def test tb3 f144 27 0000099999 NO int NULL NULL 10 0 NULL NULL NULL int(10) unsigned zerofill select,insert,update,references NEVER NULL
......
......@@ -434,9 +434,9 @@ def test tb1 f19 11 NULL YES smallint NULL NULL 5 0 NULL NULL NULL smallint(5) u
def test tb1 f2 2 NULL YES char 1 1 NULL NULL NULL latin1 latin1_bin char(1) select,insert,update,references NEVER NULL
def test tb1 f20 12 NULL YES smallint NULL NULL 5 0 NULL NULL NULL smallint(5) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f21 13 NULL YES mediumint NULL NULL 7 0 NULL NULL NULL mediumint(9) select,insert,update,references NEVER NULL
def test tb1 f22 14 NULL YES mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned select,insert,update,references NEVER NULL
def test tb1 f23 15 NULL YES mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f24 16 NULL YES mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f22 14 NULL YES mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned select,insert,update,references NEVER NULL
def test tb1 f23 15 NULL YES mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f24 16 NULL YES mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f25 17 NULL YES int NULL NULL 10 0 NULL NULL NULL int(11) select,insert,update,references NEVER NULL
def test tb1 f26 18 NULL YES int NULL NULL 10 0 NULL NULL NULL int(10) unsigned select,insert,update,references NEVER NULL
def test tb1 f27 19 NULL YES int NULL NULL 10 0 NULL NULL NULL int(10) unsigned zerofill select,insert,update,references NEVER NULL
......@@ -538,9 +538,9 @@ def test tb3 f135 12 999 NO smallint NULL NULL 5 0 NULL NULL NULL smallint(5) un
def test tb3 f136 13 00999 NO smallint NULL NULL 5 0 NULL NULL NULL smallint(5) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f137 14 00999 NO smallint NULL NULL 5 0 NULL NULL NULL smallint(5) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f138 15 9999 NO mediumint NULL NULL 7 0 NULL NULL NULL mediumint(9) select,insert,update,references NEVER NULL
def test tb3 f139 16 9999 NO mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned select,insert,update,references NEVER NULL
def test tb3 f140 17 00009999 NO mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f141 18 00009999 NO mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f139 16 9999 NO mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned select,insert,update,references NEVER NULL
def test tb3 f140 17 00009999 NO mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f141 18 00009999 NO mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f142 19 99999 NO int NULL NULL 10 0 NULL NULL NULL int(11) select,insert,update,references NEVER NULL
def test tb3 f143 20 99999 NO int NULL NULL 10 0 NULL NULL NULL int(10) unsigned select,insert,update,references NEVER NULL
def test tb3 f144 21 0000099999 NO int NULL NULL 10 0 NULL NULL NULL int(10) unsigned zerofill select,insert,update,references NEVER NULL
......
......@@ -476,9 +476,9 @@ def test tb1 f19 19 NULL YES smallint NULL NULL 5 0 NULL NULL NULL smallint(5) u
def test tb1 f2 2 NULL YES char 1 1 NULL NULL NULL latin1 latin1_bin char(1) select,insert,update,references NEVER NULL
def test tb1 f20 20 NULL YES smallint NULL NULL 5 0 NULL NULL NULL smallint(5) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f21 21 NULL YES mediumint NULL NULL 7 0 NULL NULL NULL mediumint(9) select,insert,update,references NEVER NULL
def test tb1 f22 22 NULL YES mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned select,insert,update,references NEVER NULL
def test tb1 f23 23 NULL YES mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f24 24 NULL YES mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f22 22 NULL YES mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned select,insert,update,references NEVER NULL
def test tb1 f23 23 NULL YES mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f24 24 NULL YES mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb1 f25 25 NULL YES int NULL NULL 10 0 NULL NULL NULL int(11) select,insert,update,references NEVER NULL
def test tb1 f26 26 NULL YES int NULL NULL 10 0 NULL NULL NULL int(10) unsigned select,insert,update,references NEVER NULL
def test tb1 f27 27 NULL YES int NULL NULL 10 0 NULL NULL NULL int(10) unsigned zerofill select,insert,update,references NEVER NULL
......@@ -600,9 +600,9 @@ def test tb3 f135 18 999 NO smallint NULL NULL 5 0 NULL NULL NULL smallint(5) un
def test tb3 f136 19 00999 NO smallint NULL NULL 5 0 NULL NULL NULL smallint(5) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f137 20 00999 NO smallint NULL NULL 5 0 NULL NULL NULL smallint(5) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f138 21 9999 NO mediumint NULL NULL 7 0 NULL NULL NULL mediumint(9) select,insert,update,references NEVER NULL
def test tb3 f139 22 9999 NO mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned select,insert,update,references NEVER NULL
def test tb3 f140 23 00009999 NO mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f141 24 00009999 NO mediumint NULL NULL 7 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f139 22 9999 NO mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned select,insert,update,references NEVER NULL
def test tb3 f140 23 00009999 NO mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f141 24 00009999 NO mediumint NULL NULL 8 0 NULL NULL NULL mediumint(8) unsigned zerofill select,insert,update,references NEVER NULL
def test tb3 f142 25 99999 NO int NULL NULL 10 0 NULL NULL NULL int(11) select,insert,update,references NEVER NULL
def test tb3 f143 26 99999 NO int NULL NULL 10 0 NULL NULL NULL int(10) unsigned select,insert,update,references NEVER NULL
def test tb3 f144 27 0000099999 NO int NULL NULL 10 0 NULL NULL NULL int(10) unsigned zerofill select,insert,update,references NEVER NULL
......
......@@ -11072,7 +11072,7 @@ bool Column_definition::set_compressed(const char *method)
length
*/
uint32 Field_blob::max_display_length()
uint32 Field_blob::max_display_length() const
{
switch (packlength)
{
......
This diff is collapsed.
......@@ -3187,33 +3187,6 @@ inline static uint32
adjust_max_effective_column_length(Field *field_par, uint32 max_length)
{
uint32 new_max_length= field_par->max_display_length();
uint32 sign_length= (field_par->flags & UNSIGNED_FLAG) ? 0 : 1;
switch (field_par->type())
{
case MYSQL_TYPE_INT24:
/*
Compensate for MAX_MEDIUMINT_WIDTH being 1 too long (8)
compared to the actual number of digits that can fit into
the column.
*/
new_max_length+= 1;
/* fall through */
case MYSQL_TYPE_LONG:
case MYSQL_TYPE_TINY:
case MYSQL_TYPE_SHORT:
/* Take out the sign and add a conditional sign */
new_max_length= new_max_length - 1 + sign_length;
break;
/* BINGINT is always 20 no matter the sign */
case MYSQL_TYPE_LONGLONG:
/* make gcc happy */
default:
break;
}
/* Adjust only if the actual precision based one is bigger than specified */
return new_max_length > max_length ? new_max_length : max_length;
}
......
......@@ -42,6 +42,12 @@ max_display_length_for_temporal2_field(uint32 int_display_length,
@param sql_type Type of the field
@param metadata The metadata from the master for the field.
@return Maximum length of the field in bytes.
The precise values calculated by field->max_display_length() and
calculated by max_display_length_for_field() can differ (by +1 or -1)
for integer data types (TINYINT, SMALLINT, MEDIUMINT, INT, BIGINT).
This slight difference is not important here, because we call
this function only for two *different* integer data types.
*/
static uint32
max_display_length_for_field(enum_field_types sql_type, unsigned int metadata)
......@@ -737,6 +743,16 @@ can_convert_field_to(Field *field,
case MYSQL_TYPE_INT24:
case MYSQL_TYPE_LONG:
case MYSQL_TYPE_LONGLONG:
/*
max_display_length_for_field() is not fully precise for the integer
data types. So its result cannot be compared to the result of
field->max_dispay_length() when the table field and the binlog field
are of the same type.
This code should eventually be rewritten not to use
compare_lengths(), to detect subtype/supetype relations
just using the type codes.
*/
DBUG_ASSERT(source_type != field->real_type());
*order_var= compare_lengths(field, source_type, metadata);
DBUG_ASSERT(*order_var != 0);
DBUG_RETURN(is_conversion_ok(*order_var, rli));
......
......@@ -5636,7 +5636,6 @@ static void store_column_type(TABLE *table, Field *field, CHARSET_INFO *cs,
uint offset)
{
bool is_blob;
int decimals, field_length;
const char *tmp_buff;
char column_type_buff[MAX_FIELD_WIDTH];
String column_type(column_type_buff, sizeof(column_type_buff), cs);
......@@ -5686,35 +5685,10 @@ static void store_column_type(TABLE *table, Field *field, CHARSET_INFO *cs,
They are set to -1 if they should not be set (we should return NULL)
*/
field_length= -1;
decimals= field->decimals();
Information_schema_numeric_attributes num=
field->information_schema_numeric_attributes();
switch (field->type()) {
case MYSQL_TYPE_NEWDECIMAL:
field_length= ((Field_new_decimal*) field)->precision;
break;
case MYSQL_TYPE_DECIMAL:
field_length= field->field_length - (decimals ? 2 : 1);
break;
case MYSQL_TYPE_TINY:
case MYSQL_TYPE_SHORT:
case MYSQL_TYPE_LONG:
case MYSQL_TYPE_INT24:
field_length= field->max_display_length() - 1;
break;
case MYSQL_TYPE_LONGLONG:
field_length= field->max_display_length() -
((field->flags & UNSIGNED_FLAG) ? 0 : 1);
break;
case MYSQL_TYPE_BIT:
field_length= field->max_display_length();
decimals= -1; // return NULL
break;
case MYSQL_TYPE_FLOAT:
case MYSQL_TYPE_DOUBLE:
field_length= field->field_length;
if (decimals == NOT_FIXED_DEC)
decimals= -1; // return NULL
break;
case MYSQL_TYPE_TIME:
case MYSQL_TYPE_TIMESTAMP:
case MYSQL_TYPE_DATETIME:
......@@ -5727,15 +5701,15 @@ static void store_column_type(TABLE *table, Field *field, CHARSET_INFO *cs,
}
/* NUMERIC_PRECISION column */
if (field_length >= 0)
if (num.has_precision())
{
table->field[offset + 3]->store((longlong) field_length, TRUE);
table->field[offset + 3]->store((longlong) num.precision(), true);
table->field[offset + 3]->set_notnull();
/* NUMERIC_SCALE column */
if (decimals >= 0)
if (num.has_scale())
{
table->field[offset + 4]->store((longlong) decimals, TRUE);
table->field[offset + 4]->store((longlong) num.scale(), true);
table->field[offset + 4]->set_notnull();
}
}
......
......@@ -911,6 +911,46 @@ class Record_addr
};
class Information_schema_numeric_attributes
{
enum enum_attr
{
ATTR_NONE= 0,
ATTR_PRECISION= 1,
ATTR_SCALE= 2,
ATTR_PRECISION_AND_SCALE= (ATTR_PRECISION|ATTR_SCALE)
};
uint m_precision;
uint m_scale;
enum_attr m_available_attributes;
public:
Information_schema_numeric_attributes()
:m_precision(0), m_scale(0),
m_available_attributes(ATTR_NONE)
{ }
Information_schema_numeric_attributes(uint precision)
:m_precision(precision), m_scale(0),
m_available_attributes(ATTR_PRECISION)
{ }
Information_schema_numeric_attributes(uint precision, uint scale)
:m_precision(precision), m_scale(scale),
m_available_attributes(ATTR_PRECISION_AND_SCALE)
{ }
bool has_precision() const { return m_available_attributes & ATTR_PRECISION; }
bool has_scale() const { return m_available_attributes & ATTR_SCALE; }
uint precision() const
{
DBUG_ASSERT(has_precision());
return (uint) m_precision;
}
uint scale() const
{
DBUG_ASSERT(has_scale());
return (uint) m_scale;
}
};
class Type_handler
{
protected:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment