Review of MDEV-33675 Assertion(reclength < vreclength) in setup_vcols_for_repair()

Hi! Review of MDEV-33675 Assertion(reclength < vreclength) in setup_vcols_for_repair() Thanks for the clear explanation of the problem. Please add that this problem only occurs with tables where there is no data for the main record outside of the null bits. As slightly smaller patch is (same idea applies for the Aria patch): +++ b/storage/myisam/ha_myisam.cc @@ -367,13 +367,13 @@ int table2myisam(TABLE *table_arg, MI_KEYDEF **keydef_out, while (recpos < (uint) share->stored_rec_length) { Field **field, *found= 0; - minpos= share->reclength; + minpos= share->stored_rec_length; length= 0; for (field= table_arg->field; *field; field++) { if ((fieldpos= (*field)->offset(record)) >= recpos && - fieldpos <= minpos) + fieldpos < minpos) { /* skip null fields */ if (!(temp_length= (*field)->pack_length_in_rec())) The above code fixes the issue as all generated fields are always at the end of the record, ie after 'share->stored_rec_ength'. There is no reason to test vcol_info->stored_in_db as these are already covered by the above code and pack_length_in_rec() At least the above patch fixes all your test cases and some additional test cases that I tested like: create table t1 (c1 bit, c2 long as (c1) virtual, c3 char(10)) engine=myisam; insert into t1 (c1,c3) values (1, "a"); check table t1; insert into t1 values(); select hex(c1), hex(c2) from t1; drop table t1; Please add the above test to your commit. Please also add tests that shows that things works with stored and not stored virtual columns. Please also avoid generating warnings in your test that is not part of the problem: select cast(c1 as unsigned) c1, 0 + c2 from t; -> select hex(c1), hex(c2) from t; Please fix and I will do a quick new review. Regards, Monty
participants (1)
-
Michael Widenius