Hello Igor, Please find the feedback below.
On Mon, Jul 12, 2010 at 07:08:34PM -0700, Igor Babaev wrote: > Please review this patch for the 5.2 tree. > > Regards, > Igor. > > -------- Original Message -------- > Subject: [Commits] bzr commit into Mariadb 5.2, with Maria 2.0:maria/5.2 > branch (igor:2821) Bug#604549 > Date: Mon, 12 Jul 2010 18:23:26 -0700 (PDT) > From: Igor Babaev <[email protected]> > Reply-To: [email protected] > To: [email protected] > > #At lp:maria/5.2 based on > revid:[email protected] > > 2821 Igor Babaev 2010-07-12 > Fixed bug #604549. > There was no error thrown when creating a table with a virtual table > computed by an expression returning a row. > This caused a crash when inserting into the table. > > Removed periods at the end of the error messages for virtual columns. > Adjusted output in test result files accordingly. Periods at the end of error messages were apparent for the whole time. Why do we suddenly decide to remove them now? > === modified file 'mysql-test/r/plugin.result' > --- a/mysql-test/r/plugin.result 2010-04-30 20:04:35 +0000 > +++ b/mysql-test/r/plugin.result 2010-07-13 01:23:07 +0000 > @@ -75,9 +75,9 @@ SET SQL_MODE='IGNORE_BAD_TABLE_OPTIONS'; > #illegal value fixed > CREATE TABLE t1 (a int) ENGINE=example ULL=10000000000000000000 > one_or_two='ttt' YESNO=SSS; > Warnings: > -Warning 1651 Incorrect value '10000000000000000000' for option 'ULL' > -Warning 1651 Incorrect value 'ttt' for option 'one_or_two' > -Warning 1651 Incorrect value 'SSS' for option 'YESNO' > +Warning 1652 Incorrect value '10000000000000000000' for option 'ULL' > +Warning 1652 Incorrect value 'ttt' for option 'one_or_two' > +Warning 1652 Incorrect value 'SSS' for option 'YESNO' Why did the warning code change? Is this intentional? > === modified file 'sql/share/errmsg.txt' > --- a/sql/share/errmsg.txt 2010-06-01 19:52:20 +0000 > +++ b/sql/share/errmsg.txt 2010-07-13 01:23:07 +0000 > @@ -6211,28 +6211,31 @@ ER_VCOL_BASED_ON_VCOL > eng "A computed column cannot be based on a computed column" > > ER_VIRTUAL_COLUMN_FUNCTION_IS_NOT_ALLOWED > - eng "Function or expression is not allowed for column '%s'." > + eng "Function or expression is not allowed for column '%s'" > > ER_DATA_CONVERSION_ERROR_FOR_VIRTUAL_COLUMN > - eng "Generated value for computed column '%s' cannot be > converted to type '%s'." > + eng "Generated value for computed column '%s' cannot be > converted to type '%s'" > > ER_PRIMARY_KEY_BASED_ON_VIRTUAL_COLUMN > - eng "Primary key cannot be defined upon a computed column." > + eng "Primary key cannot be defined upon a computed column" > > ER_KEY_BASED_ON_GENERATED_VIRTUAL_COLUMN > - eng "Key/Index cannot be defined on a non-stored computed column." > + eng "Key/Index cannot be defined on a non-stored computed column" > > ER_WRONG_FK_OPTION_FOR_VIRTUAL_COLUMN > - eng "Cannot define foreign key with %s clause on a computed > column." > + eng "Cannot define foreign key with %s clause on a computed column" > > ER_WARNING_NON_DEFAULT_VALUE_FOR_VIRTUAL_COLUMN > - eng "The value specified for computed column '%s' in table '%s' > ignored." > + eng "The value specified for computed column '%s' in table '%s' > ignored" > > ER_UNSUPPORTED_ACTION_ON_VIRTUAL_COLUMN > - eng "'%s' is not yet supported for computed columns." > + eng "'%s' is not yet supported for computed columns" > > ER_CONST_EXPR_IN_VCOL > - eng "Constant expression in computed column function is not > allowed." > + eng "Constant expression in computed column function is not > allowed" > + > +ER_ROW_EXPR_FOR_VCOL > + eng "Expression for computed column cannot return a row" > When one sees this pair of codes ER_CONST_EXPR_IN_VCOL and ER_ROW_EXPR_FOR_VCOL, one can't help asking himself whether that's the only disallowed expressions, and if not, do we have error codes for vcol expressions with - user variables - subqueries - SP calls - etc, etc. Do we handle such cases at all? > ER_DEBUG_SYNC_TIMEOUT > eng "debug sync point wait timed out" > > === modified file 'sql/table.cc' > --- a/sql/table.cc 2010-06-05 14:53:36 +0000 > +++ b/sql/table.cc 2010-07-13 01:23:07 +0000 > @@ -1859,6 +1859,14 @@ bool fix_vcol_expr(THD *thd, > goto end; > } > thd->where= save_where; > +#if 0 > +#else > + if (unlikely(func_expr->result_type() == ROW_RESULT)) > + { > + my_error(ER_ROW_EXPR_FOR_VCOL, MYF(0)); > + goto end; > + } > +#endif Please remove #if/#else. > #ifdef PARANOID > /* > Walk through the Item tree checking if all items are valid > BR Sergey -- Sergey Petrunia, Software Developer Monty Program AB, http://askmonty.org Blog: http://s.petrunia.net/blog _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
