Brandon Nesterenko <brandon.nestere...@mariadb.com> writes:
> I always appreciate learning the history of why some things exist as they
> are :)
Oh yes, there's a _lot_ of history in the replication code...
> is logged. Reading the whole comment really solidified it. I'll incorporate
> your feedback.
Thanks Brandon!
I think generally the new patch looks good. It's a nice cleanup of the
Query_log_event::begin_event() to simply pad with Q_DUMMY instead of
special-casing different lengths (which then was buggy due to not being
updated as GTID_EVENT was extended).
Just one small but important oversight:
> case Q_GTID_FLAGS3:
> {
> CHECK_SPACE(pos, end, 1);
> gtid_flags_extra= *pos++;
> if (gtid_flags_extra & (Gtid_log_event::FL_COMMIT_ALTER_E1 |
> Gtid_log_event::FL_ROLLBACK_ALTER_E1))
> {
> CHECK_SPACE(pos, end, 8);
> sa_seq_no = uint8korr(pos);
> pos+= 8;
> }
> }
> case Q_DUMMY:
> {
> + /*
> + At some point, this query event was translated from a GTID event,
> with
> + these Q_DUMMY bytes added to pad the end of the header. We can skip
> the
> + rest of processing these vars. Note this is a separate case from the
> + default to avoid the DBUG_PRINT of an unknown status var.
> + */
> + pos= (const uchar*) end;
> break;
> }
The `break` at the end of the prior case Q_GTID_FLAGS3 was lost, that needs
to be restored.
This part looks a bit odd:
> + int2store(q + Q_STATUS_VARS_LEN_OFFSET, dummy_bytes);
> + for (size_t i= 0; i < dummy_bytes; i++)
> + q[Q_DATA_OFFSET + i]= Q_DUMMY;
> + q+= dummy_bytes;
> + q[Q_DATA_OFFSET]= 0; /* Zero terminator for empty db */
> + q+= Q_DATA_OFFSET + 1;
The line `q+= dummy_bytes;` leaves q with a strange value, which is
Q_DATA_OFFSET before the end of the usevar section. I would have done
something like this:
q+= dummy_bytes + Q_DATA_OFFSET;
*q+= 0; /* Zero terminator for empty db */
Or this:
q[Q_DATA_OFFSET + dummy_bytes]= 0; /* Zero terminator for empty db */
q+= Q_DATA_OFFSET + dummy_bytes + 1;
But I think the current code is correct, so it's up to you what you prefer.
- Kristian.
_______________________________________________
developers mailing list -- developers@lists.mariadb.org
To unsubscribe send an email to developers-le...@lists.mariadb.org
