-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use execute() instead of executeUpdate() for afterLoad/beforeLoad #313
Use execute() instead of executeUpdate() for afterLoad/beforeLoad #313
Conversation
424c74b
to
71560a2
Compare
Add execute method.
71560a2
to
80e6eb2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% confident about the difference between execute
and executeUpdate
, but it looks good from @hito4t's comment on Zulip.
One inline comment left about naming.
Also, how about naming this pull request's subject: Use execute() instead of executeUpdate() for afterLoad/beforeLoad
@@ -383,6 +383,19 @@ protected void executeSql(String sql) throws SQLException | |||
} | |||
} | |||
|
|||
protected void executeSqlStatement(String sql) throws SQLException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be hard to tell the difference between executeSql
and executeSqlStatement
...
How about:
- Name this method
executeInNewStatement
- Create another method
executeUpdateInNewStatement
to be the same asexecuteSql
- Mark
executeSql
@Deprecated
, and change it to just callexecuteUpdateInNewStatement
@dmikurube Thank you for your comment! I'll change the code. |
@hiroyuki-sato Once you address the comment, I'll include it in the next v0.10.3. |
@dmikurube Thanks. I'll fix as far as possible. |
Okay, I tagged the |
I think I'll release v0.10.3 next Tuesday. I'll include it if it's ready by then. |
@dmikurube Could you check when you have time? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! |
Fixing #312
I added test code to embulk-output-postgresql in #319.
@dmikurube and @hito4t
Could you check when you get a chance?
Ref: https://embulk-dev.zulipchat.com/#narrow/stream/352993-embulk/topic/output-postgresql.20.3A.20support.20select.20in.20the.20after_load/near/345032157