Re: bzr commit into mysql-5.1 branch (gluh:2673) Bug#38291

[Alexander Nozdrin]

Hi,

thanks for working on this!

Generally, I'm Okay with your patch. I have only one note and few  
minor comments.
I moved the bug into 'Patch Approved', in assumption that you will be  
Ok with them.
If you're not, please get back to me and we'll discuss them.

"major" note: you added an assignment for 'Send_field::org_col_name',  
but there is another
"forgotten" attribute: 'org_table_name' -- I believe, it should also  
be assigned.

> #At file:///home/gluh/MySQL/bazaar/mysql-5.1-bug-38291/
>
>  2673 Sergey Glukhov	2008-07-25
>       Bug#38291 memory corruption and server crash with view/sp/ 
> function
>       The problem:
>       Send_field.org_col_name has broken value on secondary execution.

I would add here, that it happened due to forgotten assignment of some  
Send_field attributes.

> per-file messages:
>   mysql-test/r/metadata.result
>     result fix

Could you explain a little bit more why this result file has changed?
Probably something like this:

The result file was changed, because now forgotten attributes are  
properly set.

> === modified file 'mysql-test/t/sp.test'
> --- a/mysql-test/t/sp.test	2008-05-13 12:06:32 +0000
> +++ b/mysql-test/t/sp.test	2008-07-25 11:20:35 +0000
> @@ -8178,6 +8178,48 @@ select replace(@full_mode, 'ALLOW_INVALI
>  select name from mysql.proc where name = 'p' and sql_mode =  
> @full_mode;
>  drop procedure p;
>
> +#
> +# Bug#38291 memory corruption and server crash with view/sp/function
> +#
> +
> +create table t1(
> +form_control_name varchar(50) not null,
> +form_trkno bigint(19) not null,
> +form_control_trkno bigint(19) not null auto_increment primary key,
> +PARENT_FORM_CONTROL_TRKNO bigint(19) default NULL);
> +
> +delimiter //;
> +drop function if exists f1//
> +create function f1 (formtrkno numeric(15), parentformcontroltrkno  
> numeric(15))
> +                    returns varchar(1000) charset latin1
> +begin
> +return "aaaaa" ;
> +end //
> +delimiter ;//
> +
> +create view v1 as
> +select f1(form_trkno,parent_form_control_trkno) as  
> parent_control_name from t1;

As I understand it, the column names really do not matter. Moreover,  
it seems,
the number of columns of the base table does not matter neither. I  
would propose
to "simplify" the test case like this:

CREATE TABLE t1(c1 INT);

CREATE FUNCTION f1(p1 INT) RETURNS VARCHAR(32)
   RETURN 'aaa';

CREATE VIEW v1 AS SELECT f1(c1) FROM t1;

This way we leave only essential parts of the test and don't pollute  
the test case
with customer's schema specific.

> === modified file 'sql/item.cc'
> --- a/sql/item.cc	2008-07-14 10:50:31 +0000
> +++ b/sql/item.cc	2008-07-25 11:20:35 +0000
> @@ -5927,6 +5927,8 @@ void Item_ref::make_field(Send_field *fi
>      field->table_name= table_name;
>    if (db_name)
>      field->db_name= db_name;
> +  if (orig_field_name)
> +    field->org_col_name= orig_field_name;

Again, I believe, here should be also an assignment of  
Send_field::org_table_name.