search

embulk / embulk-output-jdbc

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

Make temp tables suffix length flexible #301

Closed civitaspo closed 2 years ago

civitaspo commented 2 years ago

close #299

dmikurube commented 2 years ago

How about extracting that number-formatting part as a method, and adding some quick tests for that?

civitaspo commented 2 years ago

@dmikurube Thanks for pointing that out. I extracted the method and wrote tests for it.

civitaspo commented 2 years ago

I don't see so much benefit from predefining namePrefix. No big performance benefit, and less testable.

I agree with your opinion. But, I think it would be better to do this with a different PR because, as you say, the amount of changes would be a bit much to stop predefining namePrefix. So I would like to release this PR as it is.

civitaspo commented 2 years ago

@hito4t

How about defining generateIntermediateTableNameFormat method, which returns namePrefix + "%0" + suffixLength + "d" ?

I think this is a good idea. I agree with this way as it simplifies the implementation.

So I implemented this in efac12e. Could you review this?

civitaspo commented 2 years ago

@hito4t san Thank you for reviewing !!!

@hito4t san or @dmikurube san Could you merge this PR and release it as a new version?

dmikurube commented 2 years ago

Thanks for your contribution! Merging.

civitaspo commented 2 years ago

😄 😄 😄 😄 😄