Hard to debug batch bind parameters

[stev...@gmail.com]

Hi-

I've already fixed my problem, but I have run across this a few times, and its always confusing.  It's confused a couple other people that use Jooq on my team too so I'm curious if there are plans to improve it.  Also, I'm on 3.3.x so maybe its already better and I just need to upgrade!

Here's the query:

AmlBsaReports abr = Tables.AML_BSA_REPORTS;
BatchBindStep batch = create.batch(create
  .update(abr)
  .set(abr.STATUS, statusType.getCode())
  .set(abr.REF_MERGED_REPORT_ID, mergedReportId)
  .where(abr.REPORT_ID.eq((Long) null))
);

for (..something...) {
  batch.bind(report.getId());  // add another query to the batch with a particular parameter
}
batch.execute();

So there's a mix of literals and "null" placeholders for bind parameters.  The above fails with an unpleasant:

java.lang.ArrayIndexOutOfBoundsException: 1
at org.jooq.impl.BatchSingle.executePrepared(BatchSingle.java:145)
at org.jooq.impl.BatchSingle.execute(BatchSingle.java:111)

Obviously, when I pass all of the params in the bind method it works, but it's inconvenient.  Another place where this was even more confusing is in a MERGE query that uses the "dual" table -- apparently under the covers (at least for tsql) that generates a select 1 -- and apparently that '1' needs to be passed in to the bind otherwise it throws a similar exception.  Can we get named parameters in the future? 

Also a helper method to pass the nulls without the casting would be nice.  It gets a little ugly with a few different types.  Some method even just called param() or something that passed some type that would unambiguously resolve so we didn't have to keep casting null?

Lastly, any update on the feature to allow you to "collect" updates in a batch until a transaction boundary. i.e. some kind of "automatic" buffering & batching?  I know you have some thoughts around it and have a github issue.  Just curious on any expectation of what version you were targeting.

[Lukas Eder]

Hi Steve,

Thank you very much for your detailed feedback. It's true that the batch API deserves some review. It is indeed a bit confusing. There currently isn't any improvement between jOOQ 3.3 and 3.5 that you could profit from, immediately. I will comment on the particular issues inline...

2015-01-13 3:42 GMT+01:00 <stev...@gmail.com>:

Hi-

I've already fixed my problem, but I have run across this a few times, and its always confusing.  It's confused a couple other people that use Jooq on my team too so I'm curious if there are plans to improve it.  Also, I'm on 3.3.x so maybe its already better and I just need to upgrade!

Here's the query:

AmlBsaReports abr = Tables.AML_BSA_REPORTS;
BatchBindStep batch = create.batch(create
  .update(abr)
  .set(abr.STATUS, statusType.getCode())
  .set(abr.REF_MERGED_REPORT_ID, mergedReportId)
  .where(abr.REPORT_ID.eq((Long) null))
);

for (..something...) {
  batch.bind(report.getId());  // add another query to the batch with a particular parameter
}
batch.execute();

So there's a mix of literals and "null" placeholders for bind parameters.  The above fails with an unpleasant:

java.lang.ArrayIndexOutOfBoundsException: 1
at org.jooq.impl.BatchSingle.executePrepared(BatchSingle.java:145)
at org.jooq.impl.BatchSingle.execute(BatchSingle.java:111)

Obviously, when I pass all of the params in the bind method it works, but it's inconvenient.

I understand, so you would like to keep the first two bind values for STATUS = ? and REF_MERGED_REPORT_ID = ? identical for all batched statements, and re-assign only the REPORT_ID = ? one. The way this can be done already today is by using DSL.inline() for the values that you don't want to change. I.e. the following would work:

AmlBsaReports abr = Tables.AML_BSA_REPORTS;

BatchBindStep batch = create.batch(create

  .update(abr)

  .set(abr.STATUS, DSL.inline(statusType.getCode()))

  .set(abr.REF_MERGED_REPORT_ID, DSL.inline(mergedReportId))

  .where(abr.REPORT_ID.eq((Long) null))

);

for (..something...) {

  batch.bind(report.getId());  // add another query to the batch with a particular parameter

}

batch.execute();

With the current BatchBindStep.bind() methods, it will not be possible to "bypass" the first two bind values by index.

 Another place where this was even more confusing is in a MERGE query that uses the "dual" table -- apparently under the covers (at least for tsql) that generates a select 1 -- and apparently that '1' needs to be passed in to the bind otherwise it throws a similar exception.

That sounds like a bug. Users shouldn't expect such synthetic bind values to be generated under the hood. I have registered an issue for this:

https://github.com/jOOQ/jOOQ/issues/3940

This issue leaves a couple of additional open questions. jOOQ emulates quite a few expressions that are not available in a given SQL dialect, or at least not available in exactly the way the jOOQ API expresses the feature. Some emulations are not strictly equivalent to what the user has in mind, e.g. with respect to bind value order (string functions are a typical example) or even with respect to the number of repetitions of an expression.

In most cases, these questions are irrelevant as users do not get exposed to the rendered SQL and bound values - but when batching statements, this is certainly an important issue!

In order to solve this thoroughly, jOOQ will need to keep track of the actual user values passed to the various API elements in their correct order, and BatchBindStep.bind(Object...) will need to consider exactly that order. I'm currently not sure if this is feasible, as the lexical order in Java code does not necessarily correspond to any order in the AST.

Of course...

 Can we get named parameters in the future? 

This would be a more transparent solution. Issue #2905 will fix this:

https://github.com/jOOQ/jOOQ/issues/2905

 

Also a helper method to pass the nulls without the casting would be nice.  It gets a little ugly with a few different types.  Some method even just called param() or something that passed some type that would unambiguously resolve so we didn't have to keep casting null?

Yes, you're right. There are already param() methods to create named parameters. It would certainly make sense to review those and also add the possibility to create unnamed param placeholders, possibly generic. I'm not 100% sure yet that making those methods generic is a good idea:

<T> Field<T> param() {

    return ...

}

While this certainly compiles when used in the jOOQ API, e.g.

.where(abr.REPORT_ID.eq(param()))

The <T> type information might not be available and it might not correspond to what users expect (e.g. Long). In the worst case, this could lead to ClassCastExceptions. I will need to think about this. It's certainly worth improving. I have registered #3941 for this:

https://github.com/jOOQ/jOOQ/issues/3941

Lastly, any update on the feature to allow you to "collect" updates in a batch until a transaction boundary. i.e. some kind of "automatic" buffering & batching?  I know you have some thoughts around it and have a github issue.  Just curious on any expectation of what version you were targeting.

This feature hasn't been targeted to any specific release so far. Personally, I'm a bit reluctant to add this feature too early without a lot of consideration as it makes a lot of assumptions about the transaction model that is used.

Already today, I believe it is possible to implement this feature at the client side using ExecuteListeners that abort query execution if they're executed in such a batch context. jOOQ already uses this technique for 2 use-cases, e.g. batchStore(UpdatableRecord):

http://blog.jooq.org/2013/04/28/rare-uses-of-a-controlflowexception/

I am sorry, I cannot make any promises right now with respect to this feature - but I will think about it again while implementing the above batch API issues.

Thanks again for your valuable input. You've reported quite a few interesting issues, which we will address in 3.6.

Best Regards,

Lukas

[Lukas Eder]

Hi Steve,

Named bind variable support for batch statements is now implemented for jOOQ 3.6.0, as well as unnamed param() support:

- https://github.com/jOOQ/jOOQ/issues/2905

- https://github.com/jOOQ/jOOQ/issues/3941

I still need to verify if we can really make methods like Field<Object> param() generic, such that they have a signature like <T> Field<T> param(). This will be tracked as:

- https://github.com/jOOQ/jOOQ/issues/3972

Best Regards,

Lukas