Skip to content
This repository has been archived by the owner on Mar 31, 2021. It is now read-only.

Implementation of the execute method in the PreparedStatementImpl class #49

Merged

Conversation

echo-42
Copy link
Contributor

@echo-42 echo-42 commented Jan 26, 2020

Issue: #62

Implementation of "execute" method in the PreparedStatementImpl.
Some tools want to see this method. Since the implementation is already in StatementImpl, in my opinion it is fair to add it to PreparedStatementImpl.
I did this by analogy.

@echo-42 echo-42 requested a review from vinjosh February 4, 2020 18:29
@dai-chen
Copy link
Contributor

@echo-42 Thanks for your PR. Could you please add some unit test for your changes?

@echo-42
Copy link
Contributor Author

echo-42 commented Mar 12, 2020

@echo-42 Thanks for your PR. Could you please add some unit test for your changes?

I did not find an existing test for PreparedStatement. I had to add a new test class.

@dai-chen dai-chen requested review from dai-chen, abbashus and penghuo and removed request for vinjosh March 18, 2020 18:55
@dai-chen
Copy link
Contributor

dai-chen commented Mar 18, 2020

@echo-42 Hmm, it seems missing. I can only find this one: https://github.com/opendistro-for-elasticsearch/sql-jdbc/blob/master/src/test/java/com/amazon/opendistroforelasticsearch/jdbc/StatementTests.java. Thanks for adding a new one. Will review your PR with others.

@dai-chen
Copy link
Contributor

Approved and waiting a little while for others to review. Will merge if looking good. Thanks for your contribution!

@dai-chen dai-chen merged commit 47f4b3e into amazon-archives:master Mar 19, 2020
@echo-42
Copy link
Contributor Author

echo-42 commented Mar 19, 2020

thanks =)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants