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 <igor@askmonty.org> Reply-To: maria-developers@lists.launchpad.net To: commits@mariadb.org

#At lp:maria/5.2 based on revid:knielsen@knielsen-hq.org-20100709120309-xzhk02q8coq7m6tl

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