implement java.sql.PreparedStatement #24

NathanQingyangXu · 2025-01-16T17:03:02Z

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

Implementing java.sql.PreparedStataement is the last blocker to go about CRUD MQL translation (R or aggregate has to be postponed later as planned). We decided to use the { $undefined: true } as the MQL parameter placeholder (later PR will deal with how to generate it during MQL translation) as opposed to the standardized JDBC ? marker.

Both unit testing and a simple integration testing case are included.

NathanQingyangXu · 2025-01-16T18:22:18Z

src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java

+        setParameter(parameterIndex, BsonNull.VALUE);
+    }
+
+    private void setBsonDateTimeParameter(int parameterIndex, java.util.@Nullable Date date) throws SQLException {


It is very weird that the following @Nullable is disallowed:

@Nullable java.util.Date date

No clue on jspecify doc.

…ng nullness warnings as well).

… MongoPreparedStatement for we only use its `getTime()` method

NathanQingyangXu · 2025-01-17T16:17:55Z

src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java

+    @Override
+    public int executeUpdate() throws SQLException {
+        checkClosed();
+        return executeUpdate(command);


In POC (and in tech doc), we implemented by invoking parent class or MongoStatement's executeUpdate(String mql) but I think it is not the best for some reasons:

we need to parse mql again unnecessarily; passing command directly to a new protected method accepting a parsed command makes more sense

we had to solve the parameters in Statement class which should know nothing about PreparedStatement parameter concern; passing a command with all parameters resolved makes more sense

stIncMale

The last reviewed commit is 5c63de4.

src/main/java/com/mongodb/hibernate/jdbc/MongoStatement.java

src/main/java/com/mongodb/hibernate/jdbc/PreparedStatementAdapter.java

src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java

build.gradle.kts

src/integrationTest/java/com/mongodb/hibernate/jdbc/MongoPreparedStatementIntegrationTests.java

src/test/java/com/mongodb/hibernate/jdbc/MongoPreparedStatementTests.java

…UpdateCommand

…alendar

…redStatementIntegrationTests.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

…MongoPreparedStatementTests

stIncMale

The last reviewed commit is 17dd554.

…tement

…ementedException

NathanQingyangXu · 2025-01-21T21:19:32Z

The last reviewed commit is 5c63de4.

Thanks. I'd learned to avoid the mistake of missing the middle 'hidden part' so I think I might well not rely on this manual marker any more, :).

NathanQingyangXu · 2025-01-22T14:45:44Z

src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java

+
+    private void checkParameterIndex(int parameterIndex) throws SQLException {
+        if (parameterValueSetters.isEmpty()) {
+            throw new SQLException("No parameter exists");


I found an edge. case above. If the parameter list is empty, the below exception message would be invalid.

stIncMale

The last reviewed commit is 96692d6.

src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java

…ult' at the end)

stIncMale

The last reviewed commit is 412f38f.

src/test/java/org/mockito/configuration/MockitoConfiguration.java

src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java

Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

…ts and MongoStatementTests

jyemin

I don't plan to review this further so LGTM-ing to allow merge when Valentin approves.

stIncMale

The last reviewed commit is 30004df.

There is only one unaddressed concern left.

Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

NathanQingyangXu · 2025-01-27T19:34:59Z

The last reviewed commit is 30004df.

There is only one unaddressed concern left.

I overlooked the comment regarding the removal of protected keyword in MongoStatement#checkClosed(). Now it seems all comments addressed?

stIncMale

The last reviewed commit is 038ec75.

implement java.sql.PreparedStatement

5c70ee8

NathanQingyangXu requested review from jyemin and stIncMale January 16, 2025 17:54

NathanQingyangXu commented Jan 16, 2025

View reviewed changes

NathanQingyangXu added 4 commits January 16, 2025 13:49

disable errorprone scanning on test code (so we can get rid of annoyi…

68bd2ac

…ng nullness warnings as well).

suppress errorprone warnings regarding the usage of java.util.Date in…

88e9d0a

… MongoPreparedStatement for we only use its `getTime()` method

improve MongoPreparedStatementIntegrationTests by making it more complex

c479bf1

simplify logic after references to other JDBC implementation

4ea1aae

NathanQingyangXu commented Jan 17, 2025

View reviewed changes

method renaming to improve readability

5c63de4

stIncMale requested changes Jan 20, 2025

View reviewed changes

NathanQingyangXu and others added 15 commits January 20, 2025 14:22

remove 'protected' visibility keyword usage on MongoStatement#execute…

6a1bf55

…UpdateCommand

made all JDBC adapters as interfaces

bc32a4c

add checkSqlTypeSupported() method to MongoPreparedStatement

43e5a2b

add fail() in MongoPreparedStatement#parseParameters(BsonValue,List)

62c76a0

rename parameters to parameterValueSetters

a34dbf2

fix the wrong usag of fail() in MongoPreparedStatement

82b6194

get rid of stream usage in MongoPreparedStatement

b19d26d

remove exception message format constant usage in MongoPreparedStatement

203d05f

use static import for VisibleForTesting in MongoPreparedStatement

b978c8e

move checkClosed to public methods instead of common private method

75db3d9

make null value setting go through centralized setNull() method

17b91ac

make time setter invoke other overloaded methods accepting nullable C…

2e45866

…alendar

Update src/integrationTest/java/com/mongodb/hibernate/jdbc/MongoPrepa…

62862f5

…redStatementIntegrationTests.java Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

remove unnecessary @MockitoSettings(strictness = Strictness.WARN) in …

d4f691a

…MongoPreparedStatementTests

remove unnecessary ParameterParsingTests in MongoPreparedStatementTests

17dd554

NathanQingyangXu requested a review from stIncMale January 20, 2025 23:07

stIncMale requested changes Jan 21, 2025

View reviewed changes

NathanQingyangXu added 2 commits January 21, 2025 10:46

fix erroneous usage of fail in MongoPreparedStatement

46ecaaf

rename parameters method arguments consistently in MongoPreparedSta…

5609f0f

…tement

NathanQingyangXu requested a review from stIncMale January 21, 2025 20:30

change setNull() for user-typed overloaded method to throw NotYetImpl…

d93dfdc

…ementedException

NathanQingyangXu added 2 commits January 22, 2025 09:28

refactor setNull() as per Javadoc; improve checkParameterIndex() logic

08916c1

refactor setNull() as per Javadoc; improve checkParameterIndex() logic

96692d6

NathanQingyangXu commented Jan 22, 2025

View reviewed changes

stIncMale mentioned this pull request Jan 22, 2025

implement java.sql.DatabaseMetaData #22

Merged

stIncMale requested changes Jan 23, 2025

View reviewed changes

NathanQingyangXu added 4 commits January 23, 2025 09:16

use global configuration to simplify mock answer smart nullness config

939424b

remove Blob and Clob support

cc10c1b

add isWrapper() implementation to be consistent with metadata PR

55b223d

minor change to PreparedStatementAdapter's javadoc (removing 'by defa…

412f38f

…ult' at the end)

stIncMale requested changes Jan 23, 2025

View reviewed changes

NathanQingyangXu and others added 5 commits January 24, 2025 13:37

make time related stuff to be implemented in their own ticket

d1c4006

improve fail() message in MongoPreparedStatement#parseParameters

1073bf0

Update src/test/java/org/mockito/configuration/MockitoConfiguration.java

89429ab

Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

remove unnecessary RETURNS_SMART_NULLS from MongoPreparedStatementTes…

c71b5d5

…ts and MongoStatementTests

add isWrapperFor for MongoConnection and MongoStatement

30004df

jyemin previously approved these changes Jan 24, 2025

View reviewed changes

NathanQingyangXu requested a review from stIncMale January 24, 2025 21:05

stIncMale requested changes Jan 27, 2025

View reviewed changes

Update src/main/java/com/mongodb/hibernate/jdbc/MongoStatement.java

bdad28d

Co-authored-by: Valentin Kovalenko <valentin.male.kovalenko@gmail.com>

NathanQingyangXu dismissed jyemin’s stale review via bdad28d January 27, 2025 19:33

NathanQingyangXu requested a review from stIncMale January 27, 2025 19:35

Merge branch 'main' into HIBERNATE-13-preparedStatement

038ec75

stIncMale approved these changes Jan 27, 2025

View reviewed changes

NathanQingyangXu merged commit 193f25f into mongodb:main Jan 27, 2025
6 checks passed

NathanQingyangXu deleted the HIBERNATE-13-preparedStatement branch January 27, 2025 21:33

NathanQingyangXu mentioned this pull request Mar 13, 2025

implement java.sql.ResultSet #30

Merged

implement java.sql.PreparedStatement #24

implement java.sql.PreparedStatement #24

Uh oh!

Conversation

NathanQingyangXu commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NathanQingyangXu Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NathanQingyangXu Jan 17, 2025

Choose a reason for hiding this comment

Uh oh!

stIncMale left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stIncMale left a comment

Choose a reason for hiding this comment

Uh oh!

NathanQingyangXu commented Jan 21, 2025

Uh oh!

NathanQingyangXu Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

stIncMale left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stIncMale left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jyemin left a comment

Choose a reason for hiding this comment

Uh oh!

stIncMale left a comment

Choose a reason for hiding this comment

Uh oh!

NathanQingyangXu commented Jan 27, 2025

Uh oh!

stIncMale left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NathanQingyangXu commented Jan 16, 2025 •

edited

Loading

NathanQingyangXu Jan 16, 2025 •

edited

Loading