Hi, Jacob! On Apr 03, jacob.mathew@mariadb.com wrote:

revision-id: b7aa15458db95149af4f473a90009844a16e9db8 (mariadb-10.2.4-103-gb7aa15458db) parent(s): 3c422e60bbee79bb636e65910c26ac193de70a84 author: Jacob Mathew committer: Jacob Mathew timestamp: 2017-04-03 17:54:32 -0700 message:

MDEV-11117 CHECK constraint fails on intermediate step of ALTER

Fixed the bug by failing the statement with an error message that explains that an auto-increment column may not be used in an expression for a check constraint, a default value or a generated column. Added a test case.

new file mode 100644 index 00000000000..e192fd16677 --- /dev/null +++ b/mysql-test/t/check_constraint_autoinc.test @@ -0,0 +1,7 @@ +# +# An auto-increment column may not be used in an expression for +# a check constraint: MDEV-11117 +# +--echo # AUTO_INCREMENT IN VCOL +-- error ER_AUTOINC_IN_VIRTUAL_COLUMN_FUNCTION +create or replace table t1( c1 int auto_increment primary key, check( c1 > 0 or c1 is null ) );

you could've added this 2-line test to the existing file check_constraint.test No strong arguments either way, though. Pro: mysqltest is executed for every test file, it's faster to have less test files. Don't know if the difference is big, though. Contra: it's easier to see what has failed with one test per file. It doesn't matter now, but might matter in the future, when we'll have more automatic failure analysys.

diff --git a/sql/field.h b/sql/field.h index 50dcb397616..4bd43551648 100644 --- a/sql/field.h +++ b/sql/field.h @@ -591,7 +591,8 @@ static inline const char *vcol_type_name(enum_vcol_info_type type) #define VCOL_NON_DETERMINISTIC 2 #define VCOL_SESSION_FUNC 4 /* uses session data, e.g. USER or DAYNAME */ #define VCOL_TIME_FUNC 8 -#define VCOL_IMPOSSIBLE 16 +#define VCOL_AUTO_INC 16 +#define VCOL_IMPOSSIBLE 32

#define VCOL_NOT_STRICTLY_DETERMINISTIC \ (VCOL_NON_DETERMINISTIC | VCOL_TIME_FUNC | VCOL_SESSION_FUNC) diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt index 930cd2ceaa2..a78e97c2577 100644 --- a/sql/share/errmsg-utf8.txt +++ b/sql/share/errmsg-utf8.txt @@ -7142,6 +7142,8 @@ ER_NO_EIS_FOR_FIELD ER_WARN_AGGFUNC_DEPENDENCE eng "Aggregate function '%-.192s)' of SELECT #%d belongs to SELECT #%d" ukr "Агрегатна функція '%-.192s)' з SELECTу #%d належить до SELECTу #%d" +ER_AUTOINC_IN_VIRTUAL_COLUMN_FUNCTION + eng "Auto-increment column %`s may not be used in an expression for a check constraint, a default value or a generated column."

First, please add error messages at the very end of the file. Second, there's an existing error ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED eng "Function or expression '%s' cannot be used in the %s clause of %`s" which is generally used like my_error(ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0), "NOW()", "CHECK", field->name); or, for subqueries my_error(ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED, MYF(0), "SELECT ...", "DEFAULT", field->name); It'd be best if you could use it. If not, at least rephrase your error message to use ... in the %s clause of %`s pattern, instead of enumerating all possible places where autoincrement is not allowed.

diff --git a/sql/table.cc b/sql/table.cc index 6b15c06cb91..d1772933190 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -2811,6 +2811,19 @@ static bool fix_and_check_vcol_expr(THD *thd, TABLE *table, "???", "?????"); DBUG_RETURN(1); } + else if (res.errors & VCOL_AUTO_INC) + { + /* + An auto_increment field may not be used in an expression for + a check constraint, a default value or a generated column + + Note that this error condition is not detected during parsing + of the statement because the field item does not have a field + pointer at that time + */ + my_error(ER_AUTOINC_IN_VIRTUAL_COLUMN_FUNCTION, MYF(0), res.name); + DBUG_RETURN(1); + } vcol->flags= res.errors;

There is already a check for auto-increment in generated columns. I've added it in commit a418c9920047d5222a0d065343347312127b780f (see the change in item.cc). But the error message is somewhat confusing. And very confusing when applied to CHECK and DEFAULT. Yours is better, so please remove my check, no need to do it twice. Don't forget to re-run tests after that (at least vcol and gcol suites) as some error messages could change. Regards, Sergei Chief Architect MariaDB and security@mariadb.org