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

Started to implement the Driver, Connection, Statement and ResultSet class #45

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

Conversation

flipflopsen
Copy link
Contributor

What did I do?
Firstly I implemented the connect method of the SurrealJDBDriver which returns a SurrealJDBCConnection with params passed through a JDBC-Connection String which gets parsed into an URI for easier usage.
Furthermore the constrcutor of SurrealJDBCConnection is implemented which connects through the Websocket and Sync-/AsnycSurrealDriver with the database to establish an authenticated connection and uses a namespace and a dbName.
In addition to this I started to implement the executeQuery()-Method of the SurrealJDBCStatement with support of arbitrary queries (with params as well (see problems section)) and the executeUpdate()-Method.
In terms of classes, the SurrealJDBCResultSet got the next().Method and the getObject(int columnIndex, Class<?> type)-Method implemented, to retrieve objects which are getting instantiated via. the gson library.

Tests
SurrealJDBCStatementTest: executeQuery, executeQueryWithArgs, executeUpdate
SurrealJDBCDriverTest: connect

Problems
The biggest problem at the moment is the retrieval of objects with arguments because the executeQuery override of the Statement only accepts a raw SQL String as an input. As a workaround I used this scheme (you can see it in the code as well):
"select * from person where name.first = $firstName withargs [firstName, "Name"];

Another suggestion is the structure of the jdbc url: jdbc:surrealdb://host:port/dbname;namespace;optionalParam2=SomeValue2;

Would love to discuss these topics with you and appreciate any form of feedback!

Greetings

…le to connect initially.

Copied the TestUtils to the jdbc tests and added 2 dev-wise params

Added the exception package for jdbc, could come in handy in the future
Started to implement some Parts of the connection and Driver to be able to connect initially.

Copied the TestUtils to the jdbc tests and added 2 dev-wise params

Added the exception package for jdbc, could come in handy in the future

Implemented the connect method of the driver, getObject of the Statement and some other and the query method.

Added a few tests for the SurrealJDBCDriverTest and SurrealJDBCStatementTest

Still WIP.
@flipflopsen
Copy link
Contributor Author

It seems like the TestUtils class doesn't get the enviroment variables, I added a new task in the build.gradle called "jdbcTest", which is working.

Don't really now how to fix it in the pipeline, does it take the config from the build.gradle? If yes we could add the vars here:
test {
// -- Env Vars --
useJUnitPlatform()
}

@phenzler
Copy link

Problems
The biggest problem at the moment is the retrieval of objects with arguments because the executeQuery override of the Statement only accepts a raw SQL String as an input. As a workaround I used this scheme (you can see it in the code as well):
"select * from person where name.first = $firstName withargs [firstName, "Name"];

JDBC Statement does only execute an SQL query without parameters.

When using parameters a PreparedStatement must be created and the syntax of the query is defined:
Either
Select * from person where name.first = ? (and setting values in the PreparedStatement by index)
or
Select * from person where name.first = :firstName (and setting values in the PreparedStatement by parameter names)

@flipflopsen
Copy link
Contributor Author

flipflopsen commented May 17, 2023

Generally speaking u are right, I will move this to the PreparedStatement.
The question is now if we want to do this with '?' like normal SQL, because in the SurrealQL it's with $sth like:
"select * from person where name.first = $firstname" and then we pass a Map<String, String> as params to the Sync/Async-Driver.

The JDBC Standard isn't covering this exactly, it's more like a
"select * from person where name.first = ?"
and then a statement.setString(1, "firstname");

EDIT:
After reading some JDBC docs I think I've got it now, I'll implement it tomorrow. :)

@phenzler
Copy link

After reading some JDBC docs I think I've got it now

Did you finde, that it is possible to set parameters by parameter name?
Here an example:

String sql = "SELECT * FROM your_table WHERE column1 = :param1 AND column2 = :param2";
PreparedStatement pstmt = connection.prepareStatement(sql);
pstmt.setObject("param1", value1);
pstmt.setObject("param2", value2);

Copy link
Contributor

@phughk phughk left a comment

Choose a reason for hiding this comment

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

I gave this a quick read over and it looks really solid. Thanks for writing this Filip!

@phughk
Copy link
Contributor

phughk commented May 25, 2023

Regarding the above discussion - we can fix things and add functionality iteratively

@phughk
Copy link
Contributor

phughk commented May 25, 2023

@flipflopsen could you have a look at the tests please? Do we need a separate task for them? They can run as part of normal driver

@phughk phughk self-assigned this May 25, 2023
@flipflopsen
Copy link
Contributor Author

@phughk
Thank you really much for reviewing!
I'll fix the test and the part with the statement as pointed out in the discussion asap so we can merge this. :)

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

Successfully merging this pull request may close these issues.

None yet

3 participants