amazon-archives / sql-jdbc

🔍 Open Distro for Elasticsearch JDBC Driver
Apache License 2.0
111 stars 49 forks source link

Implementation of the execute method in the PreparedStatementImpl class #49

Closed echo-42 closed 4 years ago

echo-42 commented 4 years ago

Issue: https://github.com/opendistro-for-elasticsearch/sql-jdbc/issues/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.

dai-chen commented 4 years ago

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

echo-42 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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

echo-42 commented 4 years ago

thanks =)