ConstructorMapper doesn't throw an exception if no record fields at all match any columns in the result set

[davidboden]

In the class org.jdbi.v3.core.mapper.reflect.ConstructorMapper we have a code block:

    if (!matchedColumns) {
      return Optional.empty();
    } else {
      paramData.sort(Comparator.comparing((p) -> {
        return p.propagateNull ? 1 : 0;
      }));
      if (!unmatchedParameters.isEmpty()) {
        throw new IllegalArgumentException(String.format("Instance factory '%s' parameter '%s' has no matching columns in the result set. Verify that the Java compiler is configured to emit parameter names, that your result set has the columns expected, annotate the parameter names explicitly with @ColumnName, or annotate nullable parameters as @Nullable", this.factory, unmatchedParameters));
      } else {

If the fields on the Java constructor match at least one column but there are also fields which don't match columns, we'll get an IllegalArgumentException. However, if none of the fields match columns at all, the code quietly returns Optional.empty(). This behaviour feels inconsistent and I don't know what the genuine use of it would be.

If there's general agreement that we should throw an IllegalArgumentException instead of returning Optional.empty() then I'm happy to write a couple of tests to demonstrate the problem and a fix in a PR.

[stevenschlansker]

Hi @davidboden, I think your snippet is not painting the entire picture. That code is from a private helper method, and the constructor will check for an empty optional and if it did not match, return a mapper that throws an exception.

Now, this exception is a little bit strange in that it is not thrown until you try to actually use the mapper. But, could you please share a bit more context about what use case you have that would be improved? A small code example of how you try to use it, showing unintuitive behavior, would help.

[hgschmie]

@davidboden do you have any updates for us here?