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
_______________________________________________
commits mailing list -- commits@lists.mariadb.org
To unsubscribe send an email to commits-le...@lists.mariadb.org

Reply via email to