The bug was that innobase_get_computed_value() trashed record[0] and data
in Field_blob::value
Fixed by using a record on the heap for innobase_get_computed_value()
Reviewer: Marko Mäkelä
This is to mark that a field is indirectly part of a key, which simplifes
checking if we need to have this field up to date to evaluate a key.
For example:
CREATE TABLE t1 (a int, b int as (a) virtual,
c int as (b) virtual, index(c))
would mark a and b with PART_INDIRECT_KEY_FLAG.
c is marked with PART_KEY_FLAG as before.
The cause of this was several different bugs:
- When using binary logging with binlog_row_image=FULL
the all bits in read_set was set, which caused a
different (wrong) pattern for marking vcol_set.
- TABLE::mark_virtual_columns_for_write() didn't in all
cases mark vcol_set with the vcol_field.
- TABLE::update_virtual_fields() has to update all
vcol fields on REPLACE if binary logging with FULL
is used.
- VCOL_UPDATE_INDEXED should update all vcol fields part
of an index that was not updated by VCOL_UPDATE_FOR_READ
- max_row_length() calculated length of NULL and not
used fields. This didn't cause any crash, but used
more memory than needed.
table.cc:
virtual columns must be computed for INSERT, if they're part
of the partitioning expression.
this change broke gcol.gcol_partition_innodb.
fix CHECK TABLE for partitioned tables and vcols.
sql_partition.cc:
mark prerequisite base columns in full_part_field_set
ha_partition.cc
initialize vcol_set accordingly
InnoDB was too eager to forget the open table (m_mysql_table=NULL)
and that caused it to try to open a table which was opened by the user
not FK-prelocked. The server didn't expect that.
After fixing this, it crashed in gcol.innodb_virtual_fk test, trying to
compute virtual columns for a table that didn't have them. Because
row_upd_store_row() was deleting a row from node->table, while computing
virtual columns in thr->prebuilt->m_mysql_table. Which wasn't necessarily
the same table, and might've not even had virtual columns, even if
node->table did.
multi-update first runs a select to find affected rows, then performs
a separate update step. On the second step WITH CHECK OPTION rows
are read with rnd_read, but the first step might've been done with
keyread.
keyread over indexed virtual columns only reads the column's value, not
dependent base columns. This is reflected in the read_set too. But on
the rnd_read step base columns must be read - thus we need to update the
read_set before doing updates.
when opening 10.1- table that has virtual columns:
1. don't error out if it has vcols over autoinc columns.
just issue a warning.
2. set vcol type properly
3. in innodb: use table->s->stored_fields instead of table->s->fields,
because that's what was stored in innodb data dictionary
vcols in the table definition are intentionaly bad and produce
warnings when a table is opened. In some cases (depending on the
TDC/TC state as left by earlier tests) a table might be opened in the
middle of the test, resulting in spurious warnings. Suppress them.
Also, don't run main.mdev-504 for ps-protocol at all (instead of
disabling ps-protocol inside the test, but still running the test)
MyISAM in compute_vcols() - which is used only in mi_check code -
was computing indexed vcols into an internally allocated buffer
(not record[0]) and the buffer was calculated to be long enough to fit
every keyseg (a keyseg knows where its value in a record buffer is
and the length of the value).
This logic didn' work for prefix keys, because the keyseg length is the
length of a prefix, but the record buffer needs to fit the complete
value of a vcol. In this bug MyISAM was writing a 2K varchar
into a buffer too short.
Also it didn't work for repair-with-keycache, because that code
recalculats all vcols, not only indexed ones.
So, the buffer size (MYISAM_SHARE::vreclength) should include all
vcols' full lengths. But it was calculated in mi_open and low-level
MyISAM code has no knowledge of vcols.
As a fix we now recalculate MYISAM_SHARE::vreclength in
ha_myisam::setup_vcols_for_repair() which is always called
before compute_vcols().
Also, implement MDEV-11027 a little differently from 5.5 and 10.0:
recv_apply_hashed_log_recs(): Change the return type back to void
(DB_SUCCESS was always returned).
Report progress also via systemd using sd_notifyf().
Workaround for join_cache + index on vcols + keyread bug.
Initialize the record to avoid caching garbage in non-read fields.
A proper fix (do not cache non-read fields at all) is done in 10.2
in commits 5d7607f340f..8d99166c697
JOIN_CACHE's were initialized in check_join_cache_usage()
from make_join_readinfo(). After that make_join_readinfo() was looking
whether it's possible to use keyread. Later, after make_join_readinfo(),
optimizer decided whether to use filesort. And even later, at the
execution time, from join_read_first(), keyread was actually enabled.
The problem is, that if a query uses a vcol, base columns that it
depends on are automatically added to the read_set - because they're
needed to calculate the vcol. But if we're doing keyread, vcol is taken
from the index, not calculated, and base columns do not need to be
in the read set (even should not be - as they aren't getting values).
The bug was that JOIN_CACHE used read_set with base columns,
they were not read because of keyread, so it was caching garbage.
So read_set is only known after the keyread was decided. And after the
filesort was decided, as filesort doesn't use keyread. But
check_join_cache_usage() needs to be done in make_join_readinfo(),
as the code below depends on these checks,
Fix: keep JOIN_CACHE checks where they were, but move initialization
down to the very end of JOIN::optimize_inner. If keyread was enabled,
update the read_set to include only columns that are part of the index.
Copy the keyread logic from join_read_first() to happen at optimize time.
It was used for get_datetime_value() and for thd->is_error().
But in fact, get_datetime_value() never used thd argument, because the
cache ptr argument was NULL. And thd->is_error() check was not needed
at that place at all.
it used current_thd->alloc() and allocated on the thd's execution arena,
not on table->expr_arena.
Remove THD::arena_for_cached_items that is temporarily set in
update_virtual_fields(), and replaces THD arena in get_datetime_value().
Instead set THD arena to table->expr_arena for the whole duration
of update_virtual_fields()
Item_func_le included Arg_comparator. Arg_comparator remembered
the current_thd during fix_fields and used that value during
execution to allocate Item_cache in get_datetime_value().
But for vcols fix_fields and val_int can happen in different threads.
Same bug for Item_func_in using in_datetime or cmp_item_datetime,
both also remembered current_thd at fix_fields() to use it later
for get_datetime_value().
As a fix, these objects no longer remember the current_thd,
and get_datetime_value() uses current_thd at run time. This
should not increase the number of current_thd calls much, as
Item_cache is created only once anyway.
Problem was that one internal record buffer in MYISAM was not big enough to handle virtual fields.
Fixed by extending buffer.
Fixed test case to 79 characters
When updating a table with virtual BLOB columns, the following might
happen:
- an old record is read from the table, it has no virtual blob values
- update_virtual_fields() is run, vcol blob gets its value into the
record. But only a pointer to the value is in the table->record[0],
the value is in Field_blob::value String (but it doesn't have to be!
it can be in the record, if the column is just a copy of another
columns: ... b VARCHAR, c BLOB AS (b) ...)
- store_record(table,record[1]), old record now is in record[1]
- fill_record() prepares new values in record[0], vcol blob is updated,
new value replaces the old one in the Field_blob::value
- now both record[1] and record[0] have a pointer that points to the
*new* vcol blob value. Or record[1] has a pointer to nowhere if
Field_blob::value had to realloc.
To fix this I have introduced a new String object 'read_value' in
Field_blob. When updating virtual columns when a row has been read,
the allocated value is stored in 'read_value' instead of 'value'. The
allocated blobs for the new row is stored in 'value' as before.
I also made, as a safety precaution, the insert delayed handling of
blobs more general by using value to store strings instead of the
record. This ensures that virtual functions on delayed insert should
work in as in the case of normal insert.
Triggers are now properly updating the read, write and vcol maps for used
fields. This means that we don't need VCOL_UPDATE_FOR_READ_WRITE anymore
and there is no need for any other special handling of triggers in
update_virtual_fields().
To be able to test how many times virtual fields are invoked, I also
relaxed rules that one can use local (@) variables in DEFAULT and non
persistent virtual field expressions.
