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 binding is broken if type is not constant for all bindings for the same name #1901

Closed
merzbird opened this issue Jul 8, 2021 · 6 comments · Fixed by #1940
Closed
Labels

Comments

@merzbird
Copy link

merzbird commented Jul 8, 2021

Reproduced with the latest 3.20.1 version.

When using prepared batch and bind the same binding with different types (e.g. boolean and NULL type), exception is thrown.

Code:

jdbi.useHandle(handle -> {
    handle.execute("CREATE TABLE record (b bool)");
    handle.prepareBatch("INSERT INTO record (b) VALUES (:bVal)")
        .bind("bVal", false).add()
        .bindNull("bVal", Types.BOOLEAN).add()
        .execute();
});

Exception:

java.lang.ClassCastException: class org.jdbi.v3.core.argument.NullArgument cannot be cast to class java.lang.Boolean (org.jdbi.v3.core.argument.NullArgument is in unnamed module of loader 'app'; java.lang.Boolean is in module java.base of loader 'bootstrap')

	at org.jdbi.v3.core.argument.internal.strategies.LoggableBinderArgument.apply(LoggableBinderArgument.java:39)
	at org.jdbi.v3.core.statement.ArgumentBinder$Prepared.lambda$prepareBinder$12(ArgumentBinder.java:231)
	at org.jdbi.v3.core.statement.ArgumentBinder.lambda$wrapExceptions$6(ArgumentBinder.java:153)
	at org.jdbi.v3.core.statement.ArgumentBinder$Prepared.lambda$prepareBinder$13(ArgumentBinder.java:234)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1510)
	at org.jdbi.v3.core.statement.ArgumentBinder$Prepared.lambda$prepareBinder$14(ArgumentBinder.java:234)
	at org.jdbi.v3.core.statement.ArgumentBinder$Prepared.bindNamed(ArgumentBinder.java:240)
	at org.jdbi.v3.core.statement.ArgumentBinder.bind(ArgumentBinder.java:60)
	at org.jdbi.v3.core.statement.PreparedBatch.internalBatchExecute(PreparedBatch.java:204)
	at org.jdbi.v3.core.statement.PreparedBatch.execute(PreparedBatch.java:108)
	at so.jdbi.BoolNullTst.lambda$test$0(BoolNullTst.java:36)
	at org.jdbi.v3.core.HandleConsumer.lambda$asCallback$0(HandleConsumer.java:32)
	at org.jdbi.v3.core.Jdbi.withHandle(Jdbi.java:342)
	at org.jdbi.v3.core.Jdbi.useHandle(Jdbi.java:358)
@stevenschlansker
Copy link
Member

Hi, generally PreparedBatch gains efficiency by assuming that all bound parameter types for each batch is the same.
That said, it should be possible to bind some that are null and some that are not. I'll see if there is some improvement that can be made here.

@The-Funk
Copy link

The-Funk commented Jul 9, 2021

I solved this issue in my code temporarily by using multiple PreparedBatches, but definitely subscribing to this thread because as I add complexity to my entities I don't know if separating the batches to account for nulls will still be efficient. At the moment, I have 3 nullable SQL bindings. In the future, I could have 10 or 20 or 30, and for every nullable binding I would need an individually crafted batch, which quickly adds up.

This is my temporary workaround. It's a little ugly...

PreparedBatch batchB = handle.prepareBatch("UPDATE dbo.TagValues SET BoolValue = :boolValue, NumericValue = NULL, StringValue = NULL WHERE TagID = :tagID");

PreparedBatch batchN = handle.prepareBatch("UPDATE dbo.TagValues SET BoolValue = NULL, NumericValue = :numericValue, StringValue = NULL WHERE TagID = :tagID");

PreparedBatch batchS = handle.prepareBatch("UPDATE dbo.TagValues SET BoolValue = NULL, NumericValue = NULL, StringValue = :stringValue WHERE TagID = :tagID");

PreparedBatch batchE = handle.prepareBatch("UPDATE dbo.TagValues SET BoolValue = NULL, NumericValue = NULL, StringValue = NULL WHERE TagID = :tagID");

@mikebell90
Copy link

We are pretty much stuck on older JDBIs (around 3.9) for the same reason fwiw @stevenschlansker - any mixtures of bindNull and bind anything else seems to blow up. All with a classcast exception to whatever type it thinks would be best (probably the first bound type)

@stevenschlansker
Copy link
Member

stevenschlansker commented Aug 24, 2021

I haven't forgotten about this issue - I had some vacation run into a busy time at work. I'll try to get some attention here soon. Sorry it's taking a bit.

@stevenschlansker
Copy link
Member

@merzbird @The-Funk @mikebell90 this should be fixed in 3.24.0 thanks for your patience!

@The-Funk
Copy link

Aaaaah! Thank you so much for everything y'all do! I can't wait to implement this in my code! This is going to make a handful of methods much cleaner looking and more readable.

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

Successfully merging a pull request may close this issue.

4 participants