embulk / embulk-output-jdbc

MySQL, PostgreSQL, Redshift and generic JDBC output plugins for Embulk
Other
88 stars 86 forks source link

Swap name + drop instead of drop + rename in replace mode #336

Closed rajyan closed 6 months ago

rajyan commented 6 months ago

https://github.com/embulk/embulk-output-jdbc/tree/master/embulk-output-mysql#modes

This pull request aims to enhance the safety of the replace mode for embulk-output-mysql, which can inadvertently drop the target table on failure, due to MySQL's implicit commit of DDL.

It changes to swap the names of intermediate and target table at first, and then drop the "intermediate" table.

In MySQL, the table rename operation can be executed in a single query, as outlined in the documentation here:

Therefore, The target table would never be dropped. The DDL is transactional for other plugins, so it shouldn't change the behavior of replace mode.

I'm not sure enough whether this can be a breaking change, if in case, it might be better to implement it as a new mode.

rajyan commented 6 months ago

Confirmed that test passes for MySQL and PostgreSQL https://github.com/rajyan/embulk-output-jdbc/actions/runs/7852811639

not sure enough about Redshift and SQL Server.

rajyan commented 6 months ago

@hiroyuki-sato

Thank you for your review.

As you pointed out, this should be only applied for MySQL and I'll reopen another pull request.