Skip to content
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

implement java.sql.Statement #18

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

NathanQingyangXu
Copy link
Contributor

https://jira.mongodb.org/browse/HIBERNATE-16

java.sql.Statement is the parent class of java.sql.PreparedStatement, which mainly focused on parameterization. Internally after parameters passed into PreparedStatement are collected and resolved, the resulting SQL would still be executed in java.sql.Statement, including:

  • ResultSet executeQuery(String sql)
  • int executeUpdate(String sql)
  • boolean execute(String sql)

We have separate tickets for the first and third entry above, but the second entry is in the scope of this PR. The method parameter or sql could come from HQL or native MQL. Regardless, for now we simply feed it (parameterization from HQL will be done in another ticket implementing java.sql.PreparedStatement) to Mongo Java driver.

The implementation internally is based on MongoDatabase#runCommand(ClientSession, Bson), so we need to pass MongoDatabase all the way from MongoConnectionProvider. Currently a @Nullable MongoDatabase will be used for now and it could be changed later (when we finished discussion on how to get database somewhere).

Most of the unit testing focused on the edge cases of int executeUpdate(String) by mocking. After evergreen integration work is done, we might later add integration testing cases on it.

@@ -76,8 +76,6 @@ tasks.withType<JavaCompile>().configureEach {
options.errorprone {
disableWarningsInGeneratedCode.set(true)
option("NullAway:AnnotatedPackages", "com.mongodb.hibernate")
option("NullAway:ExcludedFieldAnnotations", "org.mockito.Mock")
option("NullAway:ExcludedFieldAnnotations", "org.mockito.InjectMocks")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found the above two options not taking effect

@@ -272,11 +273,11 @@ public void clearWarnings() throws SQLException {
@Override
public <T> T unwrap(Class<T> unwrapType) throws SQLException {
checkClosed();
throw new SQLFeatureNotSupportedException("Unwrap() unsupported");
throw new SQLFeatureNotSupportedException("unwrap unsupported");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simulating the message pattern elsewhere

}

@Override
public boolean isWrapperFor(Class<?> iface) throws SQLException {
public boolean isWrapperFor(Class<?> iface) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems we'd better eliminate unused exceptions from method signature to avoid troubling method user to catch the checked exception which will never be thrown.

default:
throw new SQLFeatureNotSupportedException(
"Unsupported (supported commands include 'insert', 'update' and 'delete') command: "
+ commandName);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might need dicusson on what commands we support. The above three was copied from POC, not necessarily the best.

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

Successfully merging this pull request may close these issues.

1 participant