Below is the list of changes that have just been committed into a local
6.0-falcon repository of istruewing. When istruewing does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
ChangeSet@stripped, 2007-06-16 21:55:04+02:00, istruewing@stripped +9 -0
Bug#26827 - table->read_set is set incorrectly,
causing update of a different column
Bug#28158 - table->read_set is set incorrectly,
causing wrong error message in Falcon
Bug#28165 - Falcon: hang after select for update
Bug#28971 - ALTER TABLE followed by UPDATE for a CSV table
make server crash
Some engines require a complete record for update_row().
Engines with this requirement detected so far are Falcon
and CSV.
The server did not provide complete records in all cases.
Most engines don't need them, and constructing a complete
record would be unnecessary effort.
I fixed the problem by adding a new table flag
HA_REQUIRES_ALL_COLUMNS_FOR_UPDATE. This is set by the affected
handlers. mysql_update() calls mark_columns_needed_for_update(),
where the needed columns are marked. Here we notice the flag and
mark all columns for these engines. Consequently complete records
are read for them.
After fixing this, the test for Bug#22852 (Falcon: Packets out
of order after handled error) started to fail. Since we have all
bits set in read_set, write_set is now a subset of read_set. In
this case mysql_update() compares old and new records and skips
handler::update_row() when they are the same. This must not be
done for transactional engines. Another transaction could have
done the same update right before. The skipped update_row() hide
the changes from the transaction. It continues with the unchanged
rows. I added a respective condition.
mysql-test/r/csv.result@stripped, 2007-06-16 21:54:59+02:00, istruewing@stripped +8 -0
Bug#28971 - ALTER TABLE followed by UPDATE for a CSV table
make server crash
Added test result
mysql-test/t/csv.test@stripped, 2007-06-16 21:54:59+02:00, istruewing@stripped +14 -0
Bug#28971 - ALTER TABLE followed by UPDATE for a CSV table
make server crash
Added test
mysql-test/t/disabled.def@stripped, 2007-06-16 21:54:59+02:00, istruewing@stripped +0 -3
Enabled tests fixed by this patch.
Affected are:
Bug#26827 - table->read_set is set incorrectly,
causing update of a different column
Bug#28158 - table->read_set is set incorrectly,
causing wrong error message in Falcon
Bug#28165 - Falcon: hang after select for update
sql/handler.cc@stripped, 2007-06-16 21:54:59+02:00, istruewing@stripped +3 -0
Added DBUG_PRINT for read_set and write_set contents in
column_bitmaps_signal().
sql/handler.h@stripped, 2007-06-16 21:54:59+02:00, istruewing@stripped +2 -0
Bug#26827 - table->read_set is set incorrectly,
causing update of a different column
Bug#28158 - table->read_set is set incorrectly,
causing wrong error message in Falcon
Bug#28165 - Falcon: hang after select for update
Bug#28971 - ALTER TABLE followed by UPDATE for a CSV table
make server crash
Added new table flag HA_REQUIRES_ALL_COLUMNS_FOR_UPDATE.
Some engines require a complete record for update_row().
sql/sql_update.cc@stripped, 2007-06-16 21:54:59+02:00, istruewing@stripped +11 -6
Added comments.
We MUST NOT skip update_row for a transactional engine.
Detected while analyzing problems with the test case for
Bug#22852 - Falcon: Packets out of order after handled error
sql/table.cc@stripped, 2007-06-16 21:54:59+02:00, istruewing@stripped +20 -3
Bug#26827 - table->read_set is set incorrectly,
causing update of a different column
Bug#28158 - table->read_set is set incorrectly,
causing wrong error message in Falcon
Bug#28165 - Falcon: hang after select for update
Bug#28971 - ALTER TABLE followed by UPDATE for a CSV table
make server crash
Set all bits in read_set if new table flag
HA_REQUIRES_ALL_COLUMNS_FOR_UPDATE is set.
Avoided multiple calls to column_bitmaps_signal().
storage/csv/ha_tina.h@stripped, 2007-06-16 21:54:59+02:00, istruewing@stripped +2 -1
Bug#28971 - ALTER TABLE followed by UPDATE for a CSV table
make server crash
Added new table flag HA_REQUIRES_ALL_COLUMNS_FOR_UPDATE.
This engine requires a complete record for update_row().
storage/falcon/ha_falcon.cpp@stripped, 2007-06-16 21:55:00+02:00, istruewing@stripped +5 -5
Bug#26827 - table->read_set is set incorrectly,
causing update of a different column
Bug#28158 - table->read_set is set incorrectly,
causing wrong error message in Falcon
Bug#28165 - Falcon: hang after select for update
Added new table flag HA_REQUIRES_ALL_COLUMNS_FOR_UPDATE.
This engine requires a complete record for update_row().
Added DBUG_PRINT for read_set and write_set contents in
StorageInterface::decodeRecord().
Removed obsolete comments.
# This is a BitKeeper patch. What follows are the unified diffs for the
# set of deltas contained in the patch. The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User: istruewing
# Host: chilla.local
# Root: /home/mydev/mysql-5.1-falcon
--- 1.257/sql/handler.cc 2007-06-16 21:55:15 +02:00
+++ 1.258/sql/handler.cc 2007-06-16 21:55:15 +02:00
@@ -1953,6 +1953,9 @@ void handler::column_bitmaps_signal()
DBUG_ENTER("column_bitmaps_signal");
DBUG_PRINT("info", ("read_set: 0x%lx write_set: 0x%lx", (long) table->read_set,
(long) table->write_set));
+ DBUG_PRINT("info", ("read_set: x%x write_set: x%x bits: %u",
+ *table->read_set->bitmap, *table->write_set->bitmap,
+ table->read_set->n_bits));
DBUG_VOID_RETURN;
}
--- 1.262/sql/handler.h 2007-06-16 21:55:15 +02:00
+++ 1.263/sql/handler.h 2007-06-16 21:55:15 +02:00
@@ -117,6 +117,8 @@
#define HA_HAS_RECORDS (LL(1) << 32) /* records() gives exact count*/
/* Has it's own method of binlog logging */
#define HA_HAS_OWN_BINLOGGING (LL(1) << 33)
+/* Reads full record only if all columns are set in read_set. */
+#define HA_REQUIRES_ALL_COLUMNS_FOR_UPDATE (LL(1) << 34)
/* bits in index_flags(index_number) for what you can do with index */
#define HA_READ_NEXT 1 /* TODO really use this flag */
--- 1.245/sql/sql_update.cc 2007-06-16 21:55:15 +02:00
+++ 1.246/sql/sql_update.cc 2007-06-16 21:55:15 +02:00
@@ -32,8 +32,8 @@ bool compare_record(TABLE *table)
return cmp_record(table,record[1]);
/* Compare null bits */
if (memcmp(table->null_flags,
- table->null_flags+table->s->rec_buff_length,
- table->s->null_bytes))
+ table->null_flags+table->s->rec_buff_length, /*null_flags 2. rec */
+ table->s->null_bytes))
return TRUE; // Diff in NULL value
/* Compare updated fields */
for (Field **ptr= table->field ; *ptr ; ptr++)
@@ -473,11 +473,16 @@ int mysql_update(THD *thd,
/*
We can use compare_record() to optimize away updates if
the table handler is returning all columns OR if
- if all updated columns are read
+ if all updated columns are read.
+ We MUST NOT skip update_row for a transactional engine! If another
+ thread has done the same update (and hence we would skip update_row),
+ the engine would not notice the changed rows and continue to work
+ with the original ones.
*/
- can_compare_record= (!(table->file->ha_table_flags() &
- HA_PARTIAL_COLUMN_READ) ||
- bitmap_is_subset(table->write_set, table->read_set));
+ can_compare_record= (!table->file->has_transactions() &&
+ (!(table->file->ha_table_flags() &
+ HA_PARTIAL_COLUMN_READ) ||
+ bitmap_is_subset(table->write_set, table->read_set)));
while (!(error=info.read_record(&info)) && !thd->killed)
{
--- 1.291/sql/table.cc 2007-06-16 21:55:15 +02:00
+++ 1.292/sql/table.cc 2007-06-16 21:55:15 +02:00
@@ -4187,10 +4187,23 @@ void st_table::mark_columns_needed_for_d
void st_table::mark_columns_needed_for_update()
{
+ bool bitmaps_changed= FALSE;
DBUG_ENTER("mark_columns_needed_for_update");
+
if (triggers)
triggers->mark_fields_used(TRG_EVENT_UPDATE);
- if (file->ha_table_flags() & HA_REQUIRES_KEY_COLUMNS_FOR_DELETE)
+
+ if (file->ha_table_flags() & HA_REQUIRES_ALL_COLUMNS_FOR_UPDATE)
+ {
+ /*
+ This engine requires a complete record for update_row().
+ Reading all columns makes a complete record.
+ */
+ DBUG_PRINT("table", ("marking all columns to read for update"));
+ bitmap_set_all(read_set);
+ bitmaps_changed= TRUE;
+ }
+ else if (file->ha_table_flags() & HA_REQUIRES_KEY_COLUMNS_FOR_DELETE)
{
/* Mark all used key columns for read */
Field **reg_field;
@@ -4198,9 +4211,11 @@ void st_table::mark_columns_needed_for_u
{
/* Merge keys is all keys that had a column refered to in the query */
if (merge_keys.is_overlapping((*reg_field)->part_of_key))
+ {
bitmap_set_bit(read_set, (*reg_field)->field_index);
+ bitmaps_changed= TRUE;
+ }
}
- file->column_bitmaps_signal();
}
if (file->ha_table_flags() & HA_PRIMARY_KEY_REQUIRED_FOR_DELETE)
{
@@ -4214,9 +4229,11 @@ void st_table::mark_columns_needed_for_u
else
{
mark_columns_used_by_index_no_reset(s->primary_key, read_set);
- file->column_bitmaps_signal();
+ bitmaps_changed= TRUE;
}
}
+ if (bitmaps_changed)
+ file->column_bitmaps_signal();
DBUG_VOID_RETURN;
}
--- 1.18/mysql-test/r/csv.result 2007-06-16 21:55:15 +02:00
+++ 1.19/mysql-test/r/csv.result 2007-06-16 21:55:15 +02:00
@@ -5240,3 +5240,11 @@ CREATE TABLE `bug21328` (
insert into bug21328 values (1,NULL,NULL);
alter table bug21328 engine=myisam;
drop table bug21328;
+create table t1(a blob, b int) engine=csv;
+insert into t1 values('a', 1);
+flush tables;
+update t1 set b=2;
+select * from t1;
+a b
+a 2
+drop table t1;
--- 1.23/mysql-test/t/csv.test 2007-06-16 21:55:15 +02:00
+++ 1.24/mysql-test/t/csv.test 2007-06-16 21:55:15 +02:00
@@ -1653,3 +1653,17 @@ CREATE TABLE `bug21328` (
insert into bug21328 values (1,NULL,NULL);
alter table bug21328 engine=myisam;
drop table bug21328;
+
+#
+# BUG#28971 - ALTER TABLE followed by UPDATE for a CSV table make server
+# crash
+#
+create table t1(a blob, b int) engine=csv;
+insert into t1 values('a', 1);
+# Flush tables is here just to make crash more probable. If we remove it,
+# fields will have old values from previous query and server won't crash.
+flush tables;
+update t1 set b=2;
+select * from t1;
+drop table t1;
+
--- 1.29/storage/csv/ha_tina.h 2007-06-16 21:55:15 +02:00
+++ 1.30/storage/csv/ha_tina.h 2007-06-16 21:55:15 +02:00
@@ -99,7 +99,8 @@ public:
const char **bas_ext() const;
ulonglong table_flags() const
{
- return (HA_NO_TRANSACTIONS | HA_REC_NOT_IN_SEQ | HA_NO_AUTO_INCREMENT);
+ return (HA_NO_TRANSACTIONS | HA_REC_NOT_IN_SEQ | HA_NO_AUTO_INCREMENT |
+ HA_REQUIRES_ALL_COLUMNS_FOR_UPDATE);
}
ulong index_flags(uint idx, uint part, bool all_parts) const
{
--- 1.184/storage/falcon/ha_falcon.cpp 2007-06-16 21:55:15 +02:00
+++ 1.185/storage/falcon/ha_falcon.cpp 2007-06-16 21:55:15 +02:00
@@ -613,7 +613,8 @@ ulonglong StorageInterface::table_flags(
{
DBUG_ENTER("StorageInterface::table_flags");
DBUG_RETURN(HA_REC_NOT_IN_SEQ | HA_NULL_IN_KEY | HA_AUTO_PART_KEY |
- HA_PARTIAL_COLUMN_READ | HA_CAN_GEOMETRY);
+ HA_PARTIAL_COLUMN_READ | HA_CAN_GEOMETRY |
+ HA_REQUIRES_ALL_COLUMNS_FOR_UPDATE);
}
@@ -2093,6 +2094,9 @@ void StorageInterface::decodeRecord(ucha
my_ptrdiff_t ptrDiff = buf - table->record[0];
my_bitmap_map *old_map = dbug_tmp_use_all_columns(table, table->write_set);
DBUG_ENTER("StorageInterface::decodeRecord");
+ DBUG_PRINT("ingo", ("read_set: x%x write_set: x%x bits: %u",
+ *table->read_set->bitmap, *old_map,
+ table->read_set->n_bits));
for (uint n = 0; n < table->s->fields; ++n)
{
@@ -2102,10 +2106,6 @@ void StorageInterface::decodeRecord(ucha
if (ptrDiff)
field->move_field_offset(ptrDiff);
- // @todo (HK) Affects Bug#28158: Falcon unique key violation gives wrong error message
- // @todo (KL) Affects Bug#26827: Falcon: column is null after update of a different column
- // both of these bugs need to be fixed in the server, not here.
- // This call to set_null is correct. When they are fixed, we can delete these comments.
if (dataStream->type == edsTypeNull ||
!bitmap_is_set(table->read_set, field->field_index))
field->set_null();
--- 1.329/mysql-test/t/disabled.def 2007-06-16 21:55:15 +02:00
+++ 1.330/mysql-test/t/disabled.def 2007-06-16 21:55:15 +02:00
@@ -69,13 +69,10 @@ falcon_bug_24024 : Bug#24024 2006-12-
falcon_bug_26058 : Bug#26058 2007-05-03 hakank Currently failing
falcon_bug_26433 : Bug#26433 2007-02-16 hakank Currently failing
falcon_bug_26607 : Bug#26607 2007-02-23 hakank Currently failing
-falcon_bug_26827 : Bug#26827 2007-03-04 hakank Currently failing
falcon_bug_27350 : Bug#27350 2007-03-23 hakank Currently failing
falcon_bug_27426 : Bug#27426 2007-03-27 hakank Currently failing
falcon_bug_27997 : Bug#27997 2007-04-21 hakank Currently failing
falcon_bug_28026 : Bug#28026 2007-04-25 hakank Currently failing
-falcon_bug_28158 : Bug#28158 2007-05-02 hakank Currently failing
-falcon_bug_28165 : Bug#28158 2007-05-02 hakank Currently failing because of Bug#28158
#falcon_bug_28500 : Bug#28500 2007-05-20 hakank Currently failing
falcon_bug_28725 : Bug#28725 2007-05-28 hakank Currently failing
falcon_page_size_1 : Bug#23220 2007-02-19 hakank Currently failing
| Thread |
|---|
| • bk commit into 6.0-falcon tree (istruewing:1.2563) BUG#26827 | ingo | 16 Jun |
