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

Batch Return Does not work with SQL Server #1269

Open
deusaquilus opened this issue Dec 23, 2018 · 1 comment
Open

Batch Return Does not work with SQL Server #1269

deusaquilus opened this issue Dec 23, 2018 · 1 comment
Labels

Comments

@deusaquilus
Copy link
Collaborator

deusaquilus commented Dec 23, 2018

Version: (e.g. 3.0.0-SNAPSHOT)
Module: (e.g. quill-jdbc)
Database: (e.g. sql server)

Expected behavior

This should return a list of IDs

testContext.run(liftQuery(productEntries).foreach(e => productInsert(e)))

Actual behavior

Instead SQL shows the following error:

The statement must be executed before any results can be obtained.

Workaround

No good workaround. Need to insert records one-by-one.

Solution:

This is happening because getGeneratedKeys won't work for SQL server unless you do:

conn.prepareStatement(query, Statement.RETURN_GENERATED_KEYS)

Fix this in the SQL Server context. Then change this:

productEntries.map(product => testContext.run(productInsert(lift(product))))

Back to this:

testContext.run(liftQuery(productEntries).foreach(e => productInsert(e)))

In ProductJdbcSpec (in jdbc context, and monix jdbc context)

(I noticed the solution here)

Edit:
Actually it looks like getGeneratedKeys is not supported after executeBatch (mssql issue is here). That means that we have to do just execute for now.

@getquill/maintainers

@deusaquilus deusaquilus changed the title Batch Insert Does not work with SQL Server Batch Return Does not work with SQL Server Dec 23, 2018
@fwbrasil fwbrasil added the bug label Dec 24, 2018
@deusaquilus
Copy link
Collaborator Author

So I figured this stuff out with SQL Server in ProtoQuill. The issue is that SQL Server does not work correctly (here) with the combination of preparedStatement.addBatch -> preparedStatement.executeBatch -> preparedStatement.getGeneratedKeys as has been mentioned above.

However, there is one possibly recourse which is to use a combination of the OUTPUT clause together with preparedStatement.executeQuery after each call to preparedStatement.addBatch as I have implemented in ProtoQuill here. This approach has also been mentioned here on a post in a related mssql-jdbc issue.

The problem is doing preparedStatement.executeQuery after every single-row-insert is the very antithesis of doing batching. You're literally executing a query on every single row-insert which is the very definition of N+1! That means that the only way that this workaround makes any sense is to implement it together with VALUES-clause batch optimization i.e. make every addBatch call insert MORE than one row (1000 rows works bet in practice). That way, the preparedStataement after each addBatch will return 1000 ids instead of one.

This functionality was already implemented in ProtoQuill here: zio/zio-protoquill#172. I am investigating the possibility of implementing it in Scala2-Quill.

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

No branches or pull requests

2 participants