Hi, Oleksandr! On Aug 09, Oleksandr Byelkin wrote:
diff --git a/mysql-test/r/func_default.result b/mysql-test/r/func_default.result index 535be10da86..1f331af287c 100644 --- a/mysql-test/r/func_default.result +++ b/mysql-test/r/func_default.result @@ -8,7 +8,7 @@ explain extended select default(str), default(strnull), default(intg), default(r id select_type table type possible_keys key key_len ref rows filtered Extra 1 SIMPLE t1 system NULL NULL NULL NULL 1 100.00 Warnings: -Note 1003 select default('') AS `default(str)`,default('') AS `default(strnull)`,default(0) AS `default(intg)`,default(0) AS `default(rel)` from dual +Note 1003 select default(`str`) AS `default(str)`,default(`strnull`) AS `default(strnull)`,default(`intg`) AS `default(intg)`,default(`rel`) AS `default(rel)` from dual This looks rather fishy. There can be no columns str and strnull in dual. it is how table elimination work, table is eliminated (table is read) but default values is taking from it.
Sure, but the result is not a valid statement. What if you do create view v1 as select default(str), default(strnull) from t1; will this work?
Also previous output was also fishy (if not more) what is default('') ?
May be default(constant) simply returns this constant? Anyway, just try the view as above and make sure that it works...
diff --git a/sql/field.cc b/sql/field.cc index 9ca9663f066..e12adbf47b3 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -70,8 +70,25 @@ const char field_separator=','; #define BLOB_PACK_LENGTH_TO_MAX_LENGH(arg) \ ((ulong) ((1LL << MY_MIN(arg, 4) * 8) - 1))
-#define ASSERT_COLUMN_MARKED_FOR_READ DBUG_ASSERT(!table || (!table->read_set || bitmap_is_set(table->read_set, field_index))) -#define ASSERT_COLUMN_MARKED_FOR_WRITE_OR_COMPUTED DBUG_ASSERT(is_stat_field || !table || (!table->write_set || bitmap_is_set(table->write_set, field_index) || (table->vcol_set && bitmap_is_set(table->vcol_set, field_index)))) +// Column marked for read or the field set to read out or record[0] or [1] +#define ASSERT_COLUMN_MARKED_FOR_READ \ + DBUG_ASSERT(!table || \ + (!table->read_set || \ + bitmap_is_set(table->read_set, field_index) || \ + (!(ptr >= table->record[0] && \ + ptr < table->record[0] + table->s->reclength) && \ + !(ptr >= table->record[1] && \ + ptr < table->record[1] + table->s->reclength)))) why did you add record[1] to the assert? I think, having just record[0] would've been enough just for safety, I think record 1 still can be used for reading/writing values during update (old/new values)
Yes, but first the row is read into record[0] and then copied into record[1]. So, I think, it's always enough to check only record[0] access.
+ // If non-constant default value expression if (def_field->default_value && def_field->default_value->flags) { uchar *newptr= (uchar*) thd->alloc(1+def_field->pack_length()); if (!newptr) goto error; + /* + Even if DEFAULT() do not read tables fields, the default value + expression can do it. + */ fix_session_vcol_expr_for_read(thd, def_field, def_field->default_value); if (thd->mark_used_columns != MARK_COLUMNS_NONE) def_field->default_value->expr->walk(&Item::register_field_in_read_map, 1, 0); better call update_used_tables() from here. OK, but actually it should be more or less the same
I hope it'll be exactly the same :) The point is to have this default_value->expr->walk(&Item::register_field_in_read_map) logic only in one place (in update_used_tables), without changing the behavior. Regards, Sergei Chief Architect MariaDB and security@mariadb.org