OK, looks like the same bug (I can't be sure I do not know where it is repeatable and how). I probably can add DEBUG only test case, and lets hope that it is that bug fixed.
Oleksandr Byelkin
added a comment - OK, looks like the same bug (I can't be sure I do not know where it is repeatable and how). I probably can add DEBUG only test case, and lets hope that it is that bug fixed.
create view v1 as SELECT a as a, a+1 as b FROM t1;
insert into t1 values (1), (2);
update v1 set b= b+1;
do not trigger the error, so high probability that above fix just mask real problem which lead to not updateable items appeared in the list to update.
Oleksandr Byelkin
added a comment -
create table t1 (a int);
create view v1 as SELECT a as a, a+1 as b FROM t1;
insert into t1 values (1), (2);
update v1 set b= b+1;
do not trigger the error, so high probability that above fix just mask real problem which lead to not updateable items appeared in the list to update.
Absolutely! This is correct fix, I just telling that the other bug most probably hidden "up stream". And I am going to investigate it as have finished blocker which returned to me.
Oleksandr Byelkin
added a comment - Absolutely! This is correct fix, I just telling that the other bug most probably hidden "up stream". And I am going to investigate it as have finished blocker which returned to me.
After investigation, it is a false-positive. But it depends on external (with regard to fill_record()) context:
The only way it could be broken is if if (!(field= fld->field_for_view_update())) line case would succeed from the first time.
Consequent checks initialize rfield.
fld may return NULL in field_for_view_update() only for views.
Before fill_record, UPDATE first calls check_fields, where field_for_view_update() result is already checked. INSERT calls check_view_insertability that checks that all view fields are updateable.
That is, the check may be converted into an assertion. I will add rfield= NULL to suppress the warning.
Also I will make unwind_stored_field_offsets more durable. I believe I used a wrong field to check (should have check end against item_field.field_for_view_update() to break)
Nikita Malyavin
added a comment - After investigation, it is a false-positive. But it depends on external (with regard to fill_record() ) context:
The only way it could be broken is if if (!(field= fld->field_for_view_update())) line case would succeed from the first time.
Consequent checks initialize rfield.
fld may return NULL in field_for_view_update() only for views.
Before fill_record, UPDATE first calls check_fields, where field_for_view_update() result is already checked. INSERT calls check_view_insertability that checks that all view fields are updateable.
That is, the check may be converted into an assertion. I will add rfield= NULL to suppress the warning.
Also I will make unwind_stored_field_offsets more durable. I believe I used a wrong field to check (should have check end against item_field.field_for_view_update() to break )
PS I've made sure the tests are passing with the new assertion.
Nikita Malyavin
added a comment - sanja I hope you can review this one in the same pass with MDEV-36507 !
They are on the same branch, bb-10.5-nikita.
Commit 2e88e204 .
PS I've made sure the tests are passing with the new assertion.
Bug
Critical
OK, looks like the same bug (I can't be sure I do not know where it is repeatable and how). I probably can add DEBUG only test case, and lets hope that it is that bug fixed.