Re: bzr commit into mysql-6.0-backup branch (jorgen.loland:2676) Bug#34867

[Øystein Grøvlen]

Thanks for patch.  Will this be the final 6.0 behavior?  If I am not 
wrong, we have decided that the ideal behavior would be for backup to 
succeed?

Patch looks good. If it were me, I would have changed 
bcat_get_item_create_data to return success, instead of commenting the 
call, but I will not insist on such a change.

I have checked that the updated test fails without the fix, and succeeds 
with it.

Patch approved.

--
Øystein

Jorgen Loland wrote:
> #At file:///localhome/jl208045/mysql/mysql-6.0-backup-34867/
> 
>  2676 Jorgen Loland	2008-08-06
>       Bug#34867 - Backup: crash if altered view
>       
>       Old behavior: Server crash if a view was altered before backing 
>       up the database.
>             
>       New behavior: Backup command reports error.
>             
>       Fix: A bug in si_objects.cc#do_serialize incorrectly returned
>       'false' (success) when it failed to open a view. It now
>       cleans up and returns 'true' (error) in this case.
> modified:
>   mysql-test/lib/mtr_report.pl
>   mysql-test/r/backup_views.result
>   mysql-test/t/backup_views.test
>   sql/backup/stream_v1.c
>   sql/si_objects.cc
> 
> per-file comments:
>   mysql-test/lib/mtr_report.pl
>     Added expected error for backup_views
>   mysql-test/r/backup_views.result
>     Result file including new test for backup of altered table
>   mysql-test/t/backup_views.test
>     Added test case for backup of altered table
>   sql/backup/stream_v1.c
>     bstream_wr_item_def now returns immediately with the error code if an error
> occurs.
>   sql/si_objects.cc
>     do_serialize now cleans up and returns an error code when a view cannot be
> opened
> === modified file 'mysql-test/lib/mtr_report.pl'
> --- a/mysql-test/lib/mtr_report.pl	2008-08-05 08:04:30 +0000
> +++ b/mysql-test/lib/mtr_report.pl	2008-08-06 13:04:29 +0000
> @@ -344,6 +344,7 @@ sub mtr_report_stats ($) {
>  		($testname eq 'main.backup_views') and
>  		(
>  		  /Backup: Failed to add view/ or
> +		  /Backup: Failed to obtain meta-data for view/ or
>  		  /Restore: Could not restore view/
>  		) or
>   	 
> 
> === modified file 'mysql-test/r/backup_views.result'
> --- a/mysql-test/r/backup_views.result	2008-08-05 08:04:30 +0000
> +++ b/mysql-test/r/backup_views.result	2008-08-06 13:04:29 +0000
> @@ -528,6 +528,29 @@ ERROR HY000: Failed to add view `bup_db2
>  *** EXIT Backup of database with missing view dependency
>  
>  
> +*** ENTER Backup of database with altered view should report error, not crash
> server
> +
> +initializing test
> +DROP DATABASE bup_db1;
> +DROP DATABASE bup_db2;
> +RESTORE FROM 'bup_objectview.bak';
> +backup_id
> +#
> +USE bup_db1;
> +CREATE VIEW alter1 AS SELECT 5;
> +CREATE VIEW alter2 AS SELECT * FROM alter1;
> +ALTER VIEW alter1 AS SELECT 6;
> +
> +Testing view selecting from altered view
> +
> +SELECT * FROM alter2;
> +ERROR HY000: View 'bup_db1.alter2' references invalid table(s) or column(s) or
> function(s) or definer/invoker of view lack rights to use them
> +BACKUP DATABASE bup_db1 TO 'bup_alterview.bak';
> +ERROR HY000: View 'bup_db1.alter2' references invalid table(s) or column(s) or
> function(s) or definer/invoker of view lack rights to use them
> +
> +*** EXIT Backup of database with altered view
> +
> +
>  ***  DROP bup_db1, bup_db2 DATABASE ****
>  
>  DROP DATABASE bup_db1;
> 
> === modified file 'mysql-test/t/backup_views.test'
> --- a/mysql-test/t/backup_views.test	2008-08-05 08:04:30 +0000
> +++ b/mysql-test/t/backup_views.test	2008-08-06 13:04:29 +0000
> @@ -290,7 +290,7 @@ USE bup_db1;
>  SELECT * FROM t1;
>  
>  
> -###############
> +############### Bug#34902
>  --echo 
>  --echo *** ENTER Backup of database with missing view dependency should fail but not
> crash server
>  --echo 
> @@ -339,6 +339,40 @@ BACKUP DATABASE bup_db2 TO 'bup_shouldfa
>  --echo *** EXIT Backup of database with missing view dependency
>  --echo 
>  
> +############### Bug#34867
> +--echo 
> +--echo *** ENTER Backup of database with altered view should report error, not crash
> server
> +--echo 
> +
> +--echo initializing test
> +
> +# start with the backed up database
> +DROP DATABASE bup_db1;
> +DROP DATABASE bup_db2;
> +
> +replace_column 1 #;
> +RESTORE FROM 'bup_objectview.bak';
> +
> +USE bup_db1;
> +CREATE VIEW alter1 AS SELECT 5;
> +CREATE VIEW alter2 AS SELECT * FROM alter1;
> +ALTER VIEW alter1 AS SELECT 6;
> +
> +--echo
> +--echo Testing view selecting from altered view
> +--echo 
> +
> +--error ER_VIEW_INVALID
> +SELECT * FROM alter2;
> +
> +#fails
> +--error ER_VIEW_INVALID
> +BACKUP DATABASE bup_db1 TO 'bup_alterview.bak';
> +
> +--echo 
> +--echo *** EXIT Backup of database with altered view
> +--echo 
> +
>  ###############
>  
>  # Test cleanup section
> @@ -364,4 +398,7 @@ DROP DATABASE bup_db2;
>  --error 0,1
>  --remove_file $MYSQLTEST_VARDIR/master-data/bup_shouldfail2.bak
>  
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_alterview.bak
> +
>  #BUG#35249 Mysql server crash for delete operation followed by backup for Default
> Drivers.
> 
> === modified file 'sql/backup/stream_v1.c'
> --- a/sql/backup/stream_v1.c	2008-05-14 00:35:24 +0000
> +++ b/sql/backup/stream_v1.c	2008-08-06 13:04:29 +0000
> @@ -1555,14 +1555,25 @@ int bstream_wr_item_def(backup_stream *s
>    blob data;
>    int ret=BSTREAM_OK;
>  
> -  if (bcat_get_item_create_query(cat,item,&query) == BSTREAM_OK)
> +  ret= bcat_get_item_create_query(cat,item,&query);
> +  if (ret == BSTREAM_OK) 
>      flags |= BSTREAM_FLAG_HAS_CREATE_STMT;
> +  else if (ret == BSTREAM_ERROR) 
> +    goto wr_error;
>  
> -  if (bcat_get_item_create_data(cat,item,&data) == BSTREAM_OK)
> +  // bcat_get_item_create_data not in use yet. 
> +  /*
> +  ret= bcat_get_item_create_data(cat,item,&data);
> +  if (ret == BSTREAM_OK)
>      flags |= BSTREAM_FLAG_HAS_EXTRA_DATA;
> -
> +  else if (ret == BSTREAM_ERROR) 
> +    goto wr_error;
> +  */
> +  
>    ret= bstream_wr_meta_item(s,kind,flags,item);
> -
> +  if (ret == BSTREAM_ERROR) 
> +    goto wr_error;
> +  
>    /* save create query and/or create data */
>  
>    if (flags & BSTREAM_FLAG_HAS_EXTRA_DATA)
> 
> === modified file 'sql/si_objects.cc'
> --- a/sql/si_objects.cc	2008-07-09 07:12:43 +0000
> +++ b/sql/si_objects.cc	2008-08-06 13:04:29 +0000
> @@ -1769,8 +1769,11 @@ bool TableObj::do_serialize(THD *thd, St
>    /*
>      Open the view and its base tables or views
>    */
> -  if (open_normal_and_derived_tables(thd, table_list, 0))
> -    DBUG_RETURN(FALSE);
> +  if (open_normal_and_derived_tables(thd, table_list, 0)) {
> +    close_thread_tables(thd);
> +    thd->lex->select_lex.table_list.empty();
> +    DBUG_RETURN(TRUE);
> +  }
>  
>    /*
>      Setup view specific variables and settings
> 
> 


-- 
Øystein Grøvlen, Senior Staff Engineer
Sun Microsystems, Database Group
Trondheim, Norway