Re: [Maria-developers] c776ed4d6ec: MDEV-19848 Server crashes in check_vcol_forward_refs upon INSERT DELAYED into table with long blob key
Hi, Sachin! On Aug 20, Sachin Setiya wrote:
revision-id: c776ed4d6ec (mariadb-10.4.5-153-gc776ed4d6ec) parent(s): 4ca016237f1 author: Sachin <sachin.setiya@mariadb.com> committer: Sachin <sachin.setiya@mariadb.com> timestamp: 2019-07-30 03:42:21 +0530 message:
MDEV-19848 Server crashes in check_vcol_forward_refs upon INSERT DELAYED into table with long blob key
There are 2 issues
1st:- in make_new_field when we & into new field flag we forget LONG_UNIQUE_HASH_FIELD Flag.
2nd:- We are calling parse_vcol_defs on keyinfo , but they are not in right form. We should call setup_keyinfo_hash_all before calling parse_vcol_defs
This looks quite suspicious. Ideally, Delayed_insert::get_local_table() should just create a copy of the table, but not modify the original table. In your case it constantly modifies KEY's of the original table. It's kind of dirty. As for the real bug, see what parse_vcol_defs() is doing for long unique fields. In particular, it sets keypart->field->vcol_info= table->field[keypart->field->field_index]->vcol_info; but the keypart here is in the original Delayed_insert::table, while field and vcol_info is in a copy (created in Delayed_insert::get_local_table). This seems to be just wrong. I'd really prefer Delayed_insert::get_local_table to not change anything in the Delayed_insert::table. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi Serg!, Thanks for the review On Wed, Aug 21, 2019 at 2:03 AM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Sachin!
On Aug 20, Sachin Setiya wrote:
revision-id: c776ed4d6ec (mariadb-10.4.5-153-gc776ed4d6ec) parent(s): 4ca016237f1 author: Sachin <sachin.setiya@mariadb.com> committer: Sachin <sachin.setiya@mariadb.com> timestamp: 2019-07-30 03:42:21 +0530 message:
MDEV-19848 Server crashes in check_vcol_forward_refs upon INSERT DELAYED into table with long blob key
There are 2 issues
1st:- in make_new_field when we & into new field flag we forget LONG_UNIQUE_HASH_FIELD Flag.
2nd:- We are calling parse_vcol_defs on keyinfo , but they are not in right form. We should call setup_keyinfo_hash_all before calling parse_vcol_defs
This looks quite suspicious.
Ideally, Delayed_insert::get_local_table() should just create a copy of the table, but not modify the original table. In your case it constantly modifies KEY's of the original table. It's kind of dirty.
Agree , To solve this I created a new key_info copy for delayed_local table.
As for the real bug, see what parse_vcol_defs() is doing for long unique fields. In particular, it sets
keypart->field->vcol_info= table->field[keypart->field->field_index]->vcol_info;
To solve this I added a if condition + if (!keypart->field->vcol_info || + !keypart->field->vcol_info->expr) Although it is still dirty , I was unable to come up with better solution. Please let me know if I can solve it using some other way.
but the keypart here is in the original Delayed_insert::table, while field and vcol_info is in a copy (created in Delayed_insert::get_local_table). This seems to be just wrong.
I'd really prefer Delayed_insert::get_local_table to not change anything in the Delayed_insert::table.
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Patch link http://lists.askmonty.org/pipermail/commits/2019-October/014029.html Buildbot bb-10.4-sachin -- Regards Sachin Setiya Software Engineer at MariaDB
participants (2)
-
Sachin Setiya -
Sergei Golubchik