Hi Jason!
I have two comments inline below.
He Zhenxing wrote:
[snip]
> === modified file 'sql/log.cc'
> --- a/sql/log.cc 2008-07-25 17:21:55 +0000
> +++ b/sql/log.cc 2008-09-20 09:17:36 +0000
[snip]
> @@ -4048,14 +4053,21 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
> if (event_info->write(file))
> goto err;
>
> + error=0;
> if (file == &log_file) // we are writing to the real log (disk)
> {
> - if (flush_and_sync())
> + bool synced;
> + if (flush_and_sync(&synced))
> goto err;
If you take the branch here, it will mean that the "err" label will be reached
with error=0, which indicates that there were no error.
> +
> + if (RUN_HOOK(binlog_storage, after_flush,
> + (thd, log_file_name, file->pos_in_file, synced))) {
> + goto err;
Same here.
> + }
> +
> signal_update();
> rotate_and_purge(RP_LOCK_LOG_IS_ALREADY_LOCKED);
> }
> - error=0;
>
> err:
> if (error)
Here is the label to handle the error. Note that the above code falls through to
this label if there were no errors, so that if 'error' is 0, it means that the
"no error code" is executed. However, by moving the assignment, you have made it
so that error will always be 0.
[snip]
> === modified file 'sql/transaction.cc'
> --- a/sql/transaction.cc 2008-08-12 22:30:55 +0000
> +++ b/sql/transaction.cc 2008-09-20 09:17:36 +0000
> @@ -20,6 +20,7 @@
>
> #include "transaction.h"
> #include "mysql_priv.h"
> +#include "rpl_handler.h"
>
> #ifdef WITH_MARIA_STORAGE_ENGINE
> #include "../storage/maria/ha_maria.h"
> @@ -192,6 +193,13 @@ bool trans_commit_stmt(THD *thd)
> int res= FALSE;
> if (thd->transaction.stmt.ha_list)
> res= ha_commit_trans(thd, FALSE);
> + else
> + {
> + /* call hooks for statement commit of non-transactional tables,
> + for transactional tables, the hooks will be called in
> + ha_commit_trans */
> + RUN_HOOK(transaction, after_commit, (thd, FALSE));
> + }
> DBUG_RETURN(test(res));
> }
>
> @@ -215,6 +223,13 @@ bool trans_rollback_stmt(THD *thd)
> if (thd->transaction_rollback_request && !thd->in_sub_stmt)
> ha_rollback_trans(thd, TRUE);
> }
> + else
> + {
> + /* call hooks for statement rollback of non-transactional tables,
> + for transactional tables, the hooks will be called in
> + ha_rollback_trans */
> + RUN_HOOK(transaction, after_rollback, (thd, FALSE));
> + }
I agree with Davi: keep the hook calls either in the trans_* functions or in the
ha_* functions.
Just my few cents,
Mats Kindahl
--
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com
