Sven Sandberg wrote:
> Hi Mats and happy new year!
>
> Thanks for this big piece of work. I have two notes on the architecture
> which I'd like to discuss on IRC:
>
> - Why can't we determine the warnings in decide_logging_format() and
> save ER_ constant numbers in THD, instead of saving all_flags_set?
We could do that.
> - Why is binlog_stmt_flags a bitfield and not an enumeration type?
Because each alternative is a property that can be either enabled or disabled.
Trying to simplify the system by using an enumeration to encode the conjunction
of alternatives makes the code harder to read, harder to extend, and reduces the
flexibility. The fact that some combinations are not realistic *in the current
implementation* is not a good argument for changing it into an enumeration.
Assuming that we add more flags, compare:
enum flags {
NOTHING,
UNSAFE_UNCOMPRESSED_STMT,
SAFE_UNCOMPRESSED_STMT,
UNSAFE_COMPRESSED_STMT,
SAFE_COMPRESSED_ROW_INJECTIONS,
UNSAFE_UNCOMPRESSED_ROW_INJECTION,
SAFE_UNCOMPRESSED_ROW_INJECTION,
UNSAFE_COMPRESSED_ROW_INJECTIONS,
SAFE_COMPRESSED_ROW_INJECTIONS,
}
with
enum flags {
SAFE,
ROW_INJECTION,
COMPRESSED
};
Now, write a portable test to identify all UNCOMPRESSED events and you will see
why it is unflexible.
>
> Other notes:
>
> - I still think there should be a test case.
I'll add test cases to test the combinations, but since we have no
statement-based logging-only engine, it is not possible to test the combinations
that require this.
>
> - I found some typos in the patch, and I also have a suggestion for
> improved naming scheme of error constants: see inline comments.
Sure.
Best wishes,
Mats Kindahl
--
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com
