Skip to content

Commit

Permalink
Fixing a few data type related problems: MDEV-12875, MDEV-12886, MDEV…
Browse files Browse the repository at this point in the history
…-12916

This is a joint patch fixing the following problems:

MDEV-12875 Wrong VIEW column data type for COALESCE(int_column)
MDEV-12886 Different default for INT and BIGINT column in a VIEW for a SELECT with ROLLUP
MDEV-12916 Wrong column data type for an INT field of a cursor-anchored ROW variable

All above problem happened because the global function ::create_tmp_field()
called the top-level Item::create_tmp_field(), which made some tranformation
for INT-result data types. For example, INT(11) became BIGINT(11), because 11
is a corner case and it's not known if it fits or does not fit into INT range,
so Item::create_tmp_field() converted it to BIGINT(11) for safety.

The main idea of this patch is to avoid such tranformations.

1. Fixing Item::create_tmp_field() not to have a special case for INT_RESULT.

   Item::create_tmp_field() is changed not to have a special case
   for INT_RESULT (which earlier made a decision based on Item's max_length).
   It now calls tmp_table_field_from_field_type() for INT_RESULT,
   therefore preserves the original data type (e.g. INT, YEAR) without
   conversion to BIGINT.

   This change is valid, because a number of recent fixes
   (e.g. in Item_func_int, Item_hybrid_func, Item_int, Item_splocal)
   guarantee that item->type_handler() now properly returns
   type_handler_long vs type_handler_longlong. So no adjustment by length
   is needed any more for Items returning INT_RESULT.

   After this change, Item::create_tmp_field() calls
   tmp_table_field_from_field_type() for all XXX_RESULT, except REAL_RESULT.

2. Fixing Item::create_tmp_field() not to have a special case for REAL_RESULT.

   Note, the reason for a special case for REAL_RESULT is to have a special
   constructor for Field_double(), forcing Field_real::not_fixed to be set
   to true.

   Taking into account that only Item_sum descendants actually need a special
   constructor call Field_double(not_fixed=true), not too loose precision
   when mixing individual rows to the aggregate result:
   - renaming Item::create_tmp_field() to Item_sum::create_tmp_field().
   - changing Item::create_tmp_field() just to call
     tmp_table_field_from_field_type() for all XXX_RESULT types.

   A special case for REAL_RESULT in Item::create_tmp_field() is now gone.
   Item::create_tmp_field() is now symmetric for all XXX_RESULT types,
   and now just calls tmp_table_field_from_field_type().

3. Fixing Item_func::create_field_for_create_select() not to have
   a special case for STRING_RESULT.

   After changes #1 and #2, the code in
   Item_func::create_field_for_create_select(), testing result_type(),
   becomes useless, because: now Item::create_tmp_field() and
   tmp_table_field_from_field_type() do exactly the same thing for all
   XXX_RESULT types for Item_func descendants:
   a. It calls tmp_table_field_from_field_type for STRING_RESULT directly.
   b. For other XXX_RESULT, it goes through Item::create_tmp_field(),
      which calls the global function ::create_tmp_field(),
      which calls item->create_tmp_field() for FUNC_ITEM,
      which calls tmp_table_field_from_field_type() again.

   So removing the virtual implementation of
   Item_func::create_field_for_create_select().
   The inherited Item::create_field_for_create_select() now perfectly
   does the job, as it also calls tmp_table_field_from_field_type()
   for FUNC_ITEM, independently from XXX_RESULT type.

4. Taking into account #1 and #2, as well as some recent changes,
   removing virtual implementations:
   - Item_hybrid_func::create_tmp_field()
   - Item_hybrid_func::create_field_for_create_select()
   - Item_int_func::create_tmp_field()
   - Item_int_func::create_field_for_create_select()
   - Item_temporal_func::create_field_for_create_select()
   The derived versions from Item now perfectly work.

5. Moving a piece of code from create_tmp_field_from_item()
   to a new function create_tmp_field_from_item_finalize(),
   to reuse it in two places (see #6).

6. Changing the code responsible for BIT->INT/BIGIN tranformation
   (which is called for the cases when the created table, e.g. HEAP,
    does not fully support BIT) not to call create_tmp_field_from_item(),
   because the latter now calls tmp_table_field_from_field_type() instead
   of create_tmp_field() and thefore cannot do BIT transformation.
   So rewriting this code using a sequence of these calls:
   - item->type_handler_long_or_longlong()
   - handler->make_and_init_table_field()
   - create_tmp_field_from_item_finalize()

7. Miscelaneous changes:
   - Moving type_handler_long_or_longlong() from "protected" to "public",
     as it's now needed in the global function create_tmp_field().

8. The above changes fixed MDEV-12875, MDEV-12886, MDEV-12916.
   So adding tests for these bugs.
  • Loading branch information
Alexander Barkov committed May 25, 2017
1 parent 54e2971 commit 109bc47
Show file tree
Hide file tree
Showing 14 changed files with 196 additions and 43 deletions.
10 changes: 10 additions & 0 deletions mysql-test/r/func_hybrid_type.result
Original file line number Diff line number Diff line change
Expand Up @@ -3547,5 +3547,15 @@ t2 CREATE TABLE `t2` (
) ENGINE=MyISAM DEFAULT CHARSET=latin1
DROP TABLE t2, t1;
#
# MDEV-12875 Wrong VIEW column data type for COALESCE(int_column)
#
CREATE TABLE t1 (a INT);
CREATE OR REPLACE VIEW v1 AS SELECT COALESCE(a) FROM t1;
DESCRIBE v1;
Field Type Null Key Default Extra
COALESCE(a) int(11) YES NULL
DROP VIEW v1;
DROP TABLE t1;
#
# End of 10.3 tests
#
30 changes: 28 additions & 2 deletions mysql-test/r/olap.result
Original file line number Diff line number Diff line change
Expand Up @@ -696,8 +696,8 @@ CREATE VIEW v1 AS
SELECT a, LENGTH(a), COUNT(*) FROM t1 GROUP BY a WITH ROLLUP;
DESC v1;
Field Type Null Key Default Extra
a bigint(11) YES NULL
LENGTH(a) bigint(10) YES NULL
a int(11) YES 0
LENGTH(a) int(10) YES NULL
COUNT(*) bigint(21) NO 0
SELECT * FROM v1;
a LENGTH(a) COUNT(*)
Expand Down Expand Up @@ -766,3 +766,29 @@ b
NULL
DROP TABLE t1, t2;
End of 5.0 tests
#
# Start of 10.3 tests
#
#
# MDEV-12886 Different default for INT and BIGINT column in a VIEW for a SELECT with ROLLUP
#
CREATE TABLE t1 (a int(11) NOT NULL);
INSERT INTO t1 VALUES (1),(2);
CREATE OR REPLACE VIEW v1 AS SELECT a, LENGTH(a), COUNT(*) FROM t1 GROUP BY a WITH ROLLUP;
DESCRIBE v1;
Field Type Null Key Default Extra
a int(11) YES 0
LENGTH(a) int(10) YES NULL
COUNT(*) bigint(21) NO 0
DROP VIEW v1;
DROP TABLE t1;
CREATE TABLE t1 (a bigint(11) NOT NULL);
INSERT INTO t1 VALUES (1),(2);
CREATE OR REPLACE VIEW v1 AS SELECT a, LENGTH(a), COUNT(*) FROM t1 GROUP BY a WITH ROLLUP;
DESCRIBE v1;
Field Type Null Key Default Extra
a bigint(20) YES 0
LENGTH(a) int(10) YES NULL
COUNT(*) bigint(21) NO 0
DROP VIEW v1;
DROP TABLE t1;
21 changes: 21 additions & 0 deletions mysql-test/r/sp-anchor-row-type-cursor.result
Original file line number Diff line number Diff line change
Expand Up @@ -980,3 +980,24 @@ DROP PROCEDURE p1;
#
# End of MDEV-12461 TYPE OF and ROW TYPE OF anchored data types
#
#
# MDEV-12916 Wrong column data type for an INT field of a cursor-anchored ROW variable
#
CREATE PROCEDURE p1()
BEGIN
DECLARE a INT DEFAULT 10;
DECLARE cur1 CURSOR FOR SELECT a;
BEGIN
DECLARE rec1 ROW TYPE OF cur1;
CREATE TABLE t1 AS SELECT rec1.a;
SHOW CREATE TABLE t1;
DROP TABLE t1;
END;
END;
$$
CALL p1();
Table Create Table
t1 CREATE TABLE `t1` (
`rec1.a` int(11) DEFAULT NULL
) ENGINE=MyISAM DEFAULT CHARSET=latin1
DROP PROCEDURE p1;
2 changes: 1 addition & 1 deletion mysql-test/r/view.result
Original file line number Diff line number Diff line change
Expand Up @@ -2843,7 +2843,7 @@ CREATE TABLE t1 (i int, j int);
CREATE VIEW v1 AS SELECT COALESCE(i,j) FROM t1;
DESCRIBE v1;
Field Type Null Key Default Extra
COALESCE(i,j) bigint(11) YES NULL
COALESCE(i,j) int(11) YES NULL
CREATE TABLE t2 SELECT COALESCE(i,j) FROM t1;
DESCRIBE t2;
Field Type Null Key Default Extra
Expand Down
22 changes: 21 additions & 1 deletion mysql-test/suite/compat/oracle/r/sp-cursor-decl.result
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,28 @@ rec2.a
11
Table Create Table
t2 CREATE TABLE "t2" (
"a" bigint(21) DEFAULT NULL,
"a" bigint(20) DEFAULT NULL,
"b" varchar(11) DEFAULT NULL,
"c" double DEFAULT NULL
)
DROP PROCEDURE p1;
#
# MDEV-12916 Wrong column data type for an INT field of a cursor-anchored ROW variable
#
CREATE PROCEDURE p1
AS
a INT DEFAULT 10;
CURSOR cur1 IS SELECT a;
rec1 cur1%ROWTYPE;
BEGIN
CREATE TABLE t1 AS SELECT rec1.a;
SHOW CREATE TABLE t1;
DROP TABLE t1;
END;
$$
CALL p1();
Table Create Table
t1 CREATE TABLE "t1" (
"rec1.a" int(11) DEFAULT NULL
)
DROP PROCEDURE p1;
21 changes: 21 additions & 0 deletions mysql-test/suite/compat/oracle/t/sp-cursor-decl.test
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,24 @@ $$
DELIMITER ;$$
CALL p1();
DROP PROCEDURE p1;


--echo #
--echo # MDEV-12916 Wrong column data type for an INT field of a cursor-anchored ROW variable
--echo #

DELIMITER $$;
CREATE PROCEDURE p1
AS
a INT DEFAULT 10;
CURSOR cur1 IS SELECT a;
rec1 cur1%ROWTYPE;
BEGIN
CREATE TABLE t1 AS SELECT rec1.a;
SHOW CREATE TABLE t1;
DROP TABLE t1;
END;
$$
DELIMITER ;$$
CALL p1();
DROP PROCEDURE p1;
10 changes: 10 additions & 0 deletions mysql-test/t/func_hybrid_type.test
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,16 @@ CREATE TABLE t2 AS SELECT LEAST(a,a) FROM t1;
SHOW CREATE TABLE t2;
DROP TABLE t2, t1;

--echo #
--echo # MDEV-12875 Wrong VIEW column data type for COALESCE(int_column)
--echo #

CREATE TABLE t1 (a INT);
CREATE OR REPLACE VIEW v1 AS SELECT COALESCE(a) FROM t1;
DESCRIBE v1;
DROP VIEW v1;
DROP TABLE t1;

--echo #
--echo # End of 10.3 tests
--echo #
Expand Down
22 changes: 22 additions & 0 deletions mysql-test/t/olap.test
Original file line number Diff line number Diff line change
Expand Up @@ -404,3 +404,25 @@ SELECT DISTINCT b FROM t1, t2 GROUP BY a, b WITH ROLLUP;
DROP TABLE t1, t2;

--echo End of 5.0 tests

--echo #
--echo # Start of 10.3 tests
--echo #

--echo #
--echo # MDEV-12886 Different default for INT and BIGINT column in a VIEW for a SELECT with ROLLUP
--echo #

CREATE TABLE t1 (a int(11) NOT NULL);
INSERT INTO t1 VALUES (1),(2);
CREATE OR REPLACE VIEW v1 AS SELECT a, LENGTH(a), COUNT(*) FROM t1 GROUP BY a WITH ROLLUP;
DESCRIBE v1;
DROP VIEW v1;
DROP TABLE t1;

CREATE TABLE t1 (a bigint(11) NOT NULL);
INSERT INTO t1 VALUES (1),(2);
CREATE OR REPLACE VIEW v1 AS SELECT a, LENGTH(a), COUNT(*) FROM t1 GROUP BY a WITH ROLLUP;
DESCRIBE v1;
DROP VIEW v1;
DROP TABLE t1;
22 changes: 22 additions & 0 deletions mysql-test/t/sp-anchor-row-type-cursor.test
Original file line number Diff line number Diff line change
Expand Up @@ -1071,3 +1071,25 @@ DROP PROCEDURE p1;
--echo #
--echo # End of MDEV-12461 TYPE OF and ROW TYPE OF anchored data types
--echo #


--echo #
--echo # MDEV-12916 Wrong column data type for an INT field of a cursor-anchored ROW variable
--echo #

DELIMITER $$;
CREATE PROCEDURE p1()
BEGIN
DECLARE a INT DEFAULT 10;
DECLARE cur1 CURSOR FOR SELECT a;
BEGIN
DECLARE rec1 ROW TYPE OF cur1;
CREATE TABLE t1 AS SELECT rec1.a;
SHOW CREATE TABLE t1;
DROP TABLE t1;
END;
END;
$$
DELIMITER ;$$
CALL p1();
DROP PROCEDURE p1;
15 changes: 9 additions & 6 deletions sql/item.h
Original file line number Diff line number Diff line change
Expand Up @@ -636,11 +636,6 @@ class Item: public Value_source,
return (null_value= item->get_date_with_conversion(ltime, fuzzydate));
}

const Type_handler *type_handler_long_or_longlong() const
{
return Type_handler::type_handler_long_or_longlong(max_char_length());
}

public:
/*
Cache val_str() into the own buffer, e.g. to evaluate constant
Expand Down Expand Up @@ -1655,7 +1650,15 @@ class Item: public Value_source,
// used in row subselects to get value of elements
virtual void bring_value() {}

virtual Field *create_tmp_field(bool group, TABLE *table);
const Type_handler *type_handler_long_or_longlong() const
{
return Type_handler::type_handler_long_or_longlong(max_char_length());
}

virtual Field *create_tmp_field(bool group, TABLE *table)
{
return tmp_table_field_from_field_type(table);
}

virtual Item_field *field_for_view_update() { return 0; }

Expand Down
14 changes: 1 addition & 13 deletions sql/item_func.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,7 @@ class Item_func :public Item_func_or_sum
void signal_divide_by_null();
friend class udf_handler;
Field *create_field_for_create_select(TABLE *table)
{
return result_type() != STRING_RESULT ?
Item::create_tmp_field(false, table) :
tmp_table_field_from_field_type(table);
}
{ return tmp_table_field_from_field_type(table); }
Item *get_tmp_table_item(THD *thd);

my_decimal *val_decimal(my_decimal *);
Expand Down Expand Up @@ -396,10 +392,6 @@ class Item_hybrid_func: public Item_func,
:Item_func(thd, item), Type_handler_hybrid_field_type(item) { }
const Type_handler *type_handler() const
{ return Type_handler_hybrid_field_type::type_handler(); }
Field *create_tmp_field(bool group, TABLE *table)
{ return tmp_table_field_from_field_type(table); }
Field *create_field_for_create_select(TABLE *table)
{ return tmp_table_field_from_field_type(table); }
Field::geometry_type get_geometry_type() const
{ return Type_geometry_attributes::get_geometry_type(); };
void set_geometry_type(uint type)
Expand Down Expand Up @@ -740,10 +732,6 @@ class Item_int_func :public Item_func
double val_real();
String *val_str(String*str);
const Type_handler *type_handler() const= 0;
Field *create_tmp_field(bool group, TABLE *table)
{ return tmp_table_field_from_field_type(table); }
Field *create_field_for_create_select(TABLE *table)
{ return tmp_table_field_from_field_type(table); }
void fix_length_and_dec() {}
};

Expand Down
5 changes: 1 addition & 4 deletions sql/item_sum.h
Original file line number Diff line number Diff line change
Expand Up @@ -508,10 +508,7 @@ class Item_sum :public Item_func_or_sum
}
virtual void make_unique() { force_copy_fields= TRUE; }
Item *get_tmp_table_item(THD *thd);
Field *create_tmp_field(bool group, TABLE *table)
{
return Item::create_tmp_field(group, table);
}
Field *create_tmp_field(bool group, TABLE *table);
virtual bool collect_outer_ref_processor(void *param);
bool init_sum_func_check(THD *thd);
bool check_sum_func(THD *thd, Item **ref);
Expand Down
2 changes: 0 additions & 2 deletions sql/item_timefunc.h
Original file line number Diff line number Diff line change
Expand Up @@ -533,8 +533,6 @@ class Item_temporal_func: public Item_func
bool get_date(MYSQL_TIME *res, ulonglong fuzzy_date) { DBUG_ASSERT(0); return 1; }
my_decimal *val_decimal(my_decimal *decimal_value)
{ return val_decimal_from_date(decimal_value); }
Field *create_field_for_create_select(TABLE *table)
{ return tmp_table_field_from_field_type(table); }
int save_in_field(Field *field, bool no_conversions)
{ return save_date_in_field(field, no_conversions); }
};
Expand Down
43 changes: 29 additions & 14 deletions sql/sql_select.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15879,7 +15879,7 @@ Field *Item::create_tmp_field_int(TABLE *table, uint convert_int_length)
}


Field *Item::create_tmp_field(bool group, TABLE *table)
Field *Item_sum::create_tmp_field(bool group, TABLE *table)
{
Field *UNINIT_VAR(new_field);
MEM_ROOT *mem_root= table->in_use->mem_root;
Expand All @@ -15892,7 +15892,6 @@ Field *Item::create_tmp_field(bool group, TABLE *table)
break;
}
case INT_RESULT:
return create_tmp_field_int(table, MY_INT32_NUM_DECIMAL_DIGITS - 2);
case TIME_RESULT:
case DECIMAL_RESULT:
case STRING_RESULT:
Expand All @@ -15910,6 +15909,22 @@ Field *Item::create_tmp_field(bool group, TABLE *table)
}


static void create_tmp_field_from_item_finalize(THD *thd,
Field *new_field,
Item *item,
Item ***copy_func,
bool modify_item)
{
if (copy_func &&
(item->is_result_field() ||
(item->real_item()->is_result_field())))
*((*copy_func)++) = item; // Save for copy_funcs
if (modify_item)
item->set_result_field(new_field);
if (item->type() == Item::NULL_ITEM)
new_field->is_created_from_null_item= TRUE;
}


/**
Create field for temporary table using type of given item.
Expand Down Expand Up @@ -15940,16 +15955,9 @@ static Field *create_tmp_field_from_item(THD *thd, Item *item, TABLE *table,
{
Field *UNINIT_VAR(new_field);
DBUG_ASSERT(thd == table->in_use);
new_field= item->Item::create_tmp_field(false, table);

if (copy_func &&
(item->is_result_field() ||
(item->real_item()->is_result_field())))
*((*copy_func)++) = item; // Save for copy_funcs
if (modify_item)
item->set_result_field(new_field);
if (item->type() == Item::NULL_ITEM)
new_field->is_created_from_null_item= TRUE;
if ((new_field= item->create_tmp_field(false, table)))
create_tmp_field_from_item_finalize(thd, new_field, item,
copy_func, modify_item);
return new_field;
}

Expand Down Expand Up @@ -16024,6 +16032,8 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
Item::Type orig_type= type;
Item *orig_item= 0;

DBUG_ASSERT(thd == table->in_use);

if (type != Item::FIELD_ITEM &&
item->real_item()->type() == Item::FIELD_ITEM)
{
Expand Down Expand Up @@ -16082,9 +16092,14 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
else if (table_cant_handle_bit_fields && field->field->type() ==
MYSQL_TYPE_BIT)
{
const Type_handler *handler= item->type_handler_long_or_longlong();
*from_field= field->field;
result= create_tmp_field_from_item(thd, item, table, copy_func,
modify_item);
if ((result=
handler->make_and_init_table_field(&item->name,
Record_addr(item->maybe_null),
*item, table)))
create_tmp_field_from_item_finalize(thd, result, item,
copy_func, modify_item);
if (result && modify_item)
field->result_field= result;
}
Expand Down

0 comments on commit 109bc47

Please sign in to comment.