Incompatibility between v2 and v3 when using superfluous parameters map

[sarxos]

Hello @jdbi Team :)

In JDBI v2 we were using construction like this:

@SqlQuery("SELECT COUNT(*) FROM account a WHERE <where>")
int count(@Define("where") String where, @BindMap Map<String, Object> params);

The where is being build in runtime. In some cases it's empty (simply 1 = 1) and in some cases it contains some values provided by user on GUI filter. For example:

SELECT COUNT(*) FROM account a WHERE 1 = 1;

With:

where = '1 = 1'
params = { }

Or:

SELECT COUNT(*) FROM account a WHERE 1 = 1 AND a.status = :status;

When user provided status in a GUI filter:

where = '1 = 1 AND a.status = :status'
params = { status = 2 }

Right now, when empty params argument is provided, this throws an exception. For example:

com.vividgames.swim.akka.exception.MessageProcessingException: org.jdbi.v3.core.statement.UnableToCreateStatementException: Superfluous named parameters provided while the query declares none: '{positional:{}, named:{}, finder:[{}]}'. [statement:"SELECT count(*) FROM test t WHERE <where>", rewritten:"/* templated */ SELECT count(*) FROM test t WHERE value > 0", parsed:"ParsedSql{sql='/* CountDao.count */ SELECT count(*) FROM `dev-abc.1`.test t WHERE value > 0', parameters=ParsedParameters{positional=false, parameterNames=[]}}", arguments:{positional:{}, named:{}, finder:[{}]}]
	at org.jdbi.v3.core.statement.ArgumentBinder.bindNamed(ArgumentBinder.java:58)
	at org.jdbi.v3.core.statement.ArgumentBinder.bind(ArgumentBinder.java:29)
	at org.jdbi.v3.core.statement.SqlStatement.internalExecute(SqlStatement.java:1429)
	at org.jdbi.v3.core.result.ResultProducers.lambda$getResultSet$2(ResultProducers.java:68)
	at org.jdbi.v3.core.result.ResultIterable.lambda$of$0(ResultIterable.java:53)
	at org.jdbi.v3.core.result.ResultIterable.findFirst(ResultIterable.java:116)
	at org.jdbi.v3.sqlobject.statement.internal.ResultReturner$CollectedResultReturner.mappedResult(ResultReturner.java:266)
	at org.jdbi.v3.sqlobject.statement.internal.SqlQueryHandler.lambda$configureReturner$0(SqlQueryHandler.java:54)
	at org.jdbi.v3.sqlobject.statement.internal.CustomizingStatementHandler.invoke(CustomizingStatementHandler.java:155)
	at org.jdbi.v3.sqlobject.statement.internal.SqlQueryHandler.invoke(SqlQueryHandler.java:26)
	at org.jdbi.v3.sqlobject.SqlObjectFactory.lambda$null$13(SqlObjectFactory.java:163)
	at org.jdbi.v3.core.ConstantHandleSupplier.invokeInContext(ConstantHandleSupplier.java:52)
	at org.jdbi.v3.sqlobject.SqlObjectFactory.lambda$createInvocationHandler$14(SqlObjectFactory.java:162)

I found a flag in SqlStatements which can be used to allow unused bindings:

/**
 * Sets whether or not an exception should be thrown when any arguments are given to a query but not actually used in it. Unused bindings tend to be bugs or oversights, but can also just be convenient. Defaults to false: unused bindings are not allowed.
 *
 * @see org.jdbi.v3.core.argument.Argument
 */
public SqlStatements setUnusedBindingAllowed(boolean allowUnusedBindings) {
    this.allowUnusedBindings = allowUnusedBindings;
    return this;
}

But I cannot find any way to use this method :( In JDBI source code I found only usages, but no place where it's being set.

I can rewrite our DAOs and introduce new methods which takes no params, e.g.:

@SqlQuery("SELECT COUNT(*) FROM account a")
int countAll();

@SqlQuery("SELECT COUNT(*) FROM account a WHERE <where>")
int countFiltered(@Define("where") String where, @BindMap Map<String, Object> params);

But this would have to be done in a number of existing DAOs and to be honest I would rather like to change one line and magically make it work again.

[leaumar]

You'll have to set the flag on the Jdbi/Handle that creates the SqlObject.

jdbi.useHandle(h -> {
    Dao dao = h.configure(SqlStatements.class, stmts -> stmts.setUnusedBindingAllowed(true))
        .attach(Dao.class);
});

As far as I remember, there's no statement customizing annotation like @AllowUnusedBindings to do it. Could easily be contributed if you really want it.

[sarxos]

Hi @TheRealMarnes,

Your code works perfectly :) Thank you!

I think this could be described in a JDBI documentation available from www, or in setUnusedBindingAllowed javadoc (because these were the first places where I started looking on how to use it).

I don't know how to edit documentation, but I can update javadoc if you like.

[leaumar]

You're right, the manual lacks any mention of it. Maybe the exception should mention it as well.

I'm not sure why you bring up the javadoc as lacking though; you literally pasted the javadoc, so it exists.

If you'd like to start a manual section about it, the manual file is index.adoc. It's written in asciidoc, which really explains itself when you read it, it's kind of markdown-ish. I never really learned it, yet I've been writing manual sections just fine :) Any IDE should also give you a live preview to work with.

[sarxos]

@TheRealMarnes,

Thank you. Will take a look at the index.

In regards to:

I'm not sure why you bring up the javadoc as lacking though; you literally pasted the javadoc, so it exists.

Yes, it's there, but it does not describe anything more than what can be read directly from the code. Especially it does not describe how to configure it, and this is the core point of this issue. I found the method in code, I read javadoc, but that's all and after struggling for some time with the code, finding usages etc, I had to ask you, guys, for help. If handle.configure method is referenced/mentioned in this javadoc (best with example) it would greatly increase its value in regards to understanding how to enable this specific feature.

[leaumar]

I think you're missing the bigger picture. The handle.configure thing is standard for all configuration on Configurable objects like Jdbi and Handle (along with getConfig), it's not something special just for this. allowUnusedBindings takes a boolean, and with just that it should be pretty obvious. Calling the method and how it propagates follows our standard configuration paradigm.

[sarxos]

Well, indeed, it seems I missing it, but the other users are probably missing it as well. It looks to me as quite advanced and kind of a tribal knowledge obvious for core developers but not as obvious for us here in a wild. After you describe it as a standard configuration paradigm I went do JDBI documentation, and after reading it in regards to configuration stuff, I still have just a brief idea how it works. The configure method is used only once in regards to setting TupleMappers of Vavr.

It's a part of a bigger issue with not all features described in doc in enough details, but we, a JDBI users, are here to help pointing what is missing and help filling the gaps.

Therefore, I propose to change javadoc to contain more details, e.g.:

/**
 * Sets whether or not an exception should be thrown when any arguments are
 * given to a query but not actually used in it. Unused bindings tend to be bugs or
 * oversights, but can also just be convenient. Defaults to false: unused bindings 
 * are not allowed.<br>
 * <br>
 * It can be changed by configuring {@link Configurable} object (e.g. {@link Jdbi}
 * or {@link Handle}), for example:
 * <pre>
 * handle.configure(SqlStatements.class, s -> s.setUnusedBindingAllowed(true))
 * </pre>
 *
 * @see org.jdbi.v3.core.argument.Argument
 * @see Handle.configure
 * @see Jdbi.configure
 */

I will gladly add it, but please let me know if what I wrote is ok.

[stevenschlansker]

I ran into this exact same deficiency myself this morning. I'm going to prepare a PR that adds @AllowUnusedBindings on a sqlobject.

[leaumar]

tribal knowledge obvious for core developers but not as obvious for us here in a wild

I'd just like to point out I'm a core member since only like 2 months and I was just a regular user like yourself for over a year before then, so it's not like I'm savvy on the arcane knowledge. 😛 I know most things I know about jdbi by being in your same situation and just exploring and clicking-through on the API and interfaces, letting intellij hint methods at me and such.

But yes, it should be clearly documented because we can't expect everyone to inspect jdbi's internals to figure out basic usage. :)

I don't think we'll be adding the javadoc the way you suggest though. The config mantra should be dcoumented in its own right, not repeated on every single config setting or included on any one particularly chosen config setting. Each config setting should explain its own effects only.

[stevenschlansker]

We have a section on JdbiConfig: http://jdbi.org/#_jdbiconfig but it's more focused on making your own implementations and it's stuffed into the Advanced section near the end. Maybe we should add a "config from the user's perspective" section in the docs in an earlier section? Would that address this need better?

[sarxos]

I was reading through JdbiConfig and I was under the same impression. Things cleared after @TheRealMarnes gave me an example he provided in the comment.

Thank you for the @AllowUnusedBindings :)

[sarxos]

Guys, I know this is against having configuration paradigm documented in one place, but please consider adding information in the exception message the same way it's done here:

jdbi/core/src/main/java/org/jdbi/v3/core/Handle.java

Line 173 in 1c80a39

+ "This check may be disabled by calling getConfig(Handles.class).setForceEndTransactions(false).");

This would save a person who was hit by this exception A LOT of digging in a code :)

[leaumar]

I agree with that much, the exception itself should hint at it.

[sarxos]

@TheRealMarnes hint added in #1428

[stevenschlansker]

We've added both a hint to the error message, as well as general config documentation with this as a specific example. Hopefully that's a sufficient fix; thanks for using jdbi :)

[sarxos]

Thank you Steven! Take care :)