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.ResultSet #30

Merged

Conversation

NathanQingyangXu
Copy link
Contributor

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

A subtask for "load by id" parent ticket (https://jira.mongodb.org/browse/HIBERNATE-20). Compared to DML processing (insertion, updating and deletion), aggregate query brings about more complexity by returning ResultSet. Given we just finished Statement and PreparedStatement, it seems good timing to proceed to the last important JDBC component implementation.

@NathanQingyangXu NathanQingyangXu force-pushed the HIBERNATE-21-implement-resultSet branch from c6a4aa5 to 7c20f05 Compare January 29, 2025 01:35
@jyemin jyemin requested review from katcharov and removed request for jyemin February 4, 2025 16:28
NathanQingyangXu and others added 5 commits February 12, 2025 19:24
# Conflicts:
#	src/main/java/com/mongodb/hibernate/jdbc/MongoConnection.java
#	src/main/java/com/mongodb/hibernate/jdbc/MongoPreparedStatement.java
#	src/main/java/com/mongodb/hibernate/jdbc/MongoStatement.java
@NathanQingyangXu NathanQingyangXu force-pushed the HIBERNATE-21-implement-resultSet branch from 6ecc035 to 64eb6b7 Compare March 13, 2025 17:17
Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

The last reviewed commit is 7b53c42.

I finally got to reviewing the test code.

@NathanQingyangXu
Copy link
Contributor Author

The last reviewed commit is 7b53c42.

I finally got to reviewing the test code.

The jdbc integration testing classes are pretty messy for I hesitated to switch to hibernate-testing (which would simply a lot) for it is way higher level and might hide some low-level issue.

Thanks for your review comments. I appreciate it.

Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

The last reviewed commit is 3bd9b5b.

MongoResultSetTests.java, MongoPreparedStatementTests.java are yet to be reviewed.

Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

The last reviewed commit is 3bd9b5b. All files were reviewed.

Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

The last reviewed commit is 323a09e.

@NathanQingyangXu,

@NathanQingyangXu
Copy link
Contributor Author

The last reviewed commit is 323a09e.

@NathanQingyangXu,

I missed that. Adopted and pushed.

@stIncMale
Copy link
Member

I missed that. Adopted and pushed.

What about the other thread?

@NathanQingyangXu
Copy link
Contributor Author

I missed that. Adopted and pushed.

What about the other thread?

I missed that as well. Now both threads are resolved.

Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

The last reviewed commit is 2daab80.

Copy link
Contributor

@katcharov katcharov left a comment

Choose a reason for hiding this comment

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

LGTM

@NathanQingyangXu NathanQingyangXu merged commit 499e600 into mongodb:main Mar 14, 2025
6 checks passed
@NathanQingyangXu NathanQingyangXu deleted the HIBERNATE-21-implement-resultSet branch March 14, 2025 17:14
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.

4 participants