Two missing API methods in the Criteria API

[archiecobbs]

The following methods are inexplicably missing from the Criteria API:

• CriteriaBuilder.treat(From, Class)
• Subquery.correlate(From)

As a result, programmers have to use a sibling method with a wider type, causing them to have to add unchecked casts to compensate, etc.

[gavinking]

@archiecobbs I'm not sure I understand. Typically when creating a criteria query you will have an instance of Root or Join to work with, since there aren't any important methods which return From.

Do you have a specific motivating code example or scenario which would help justify this change?

[archiecobbs]

Hi @gavinking,

I discovered this omission while developing QueryStream, which is a "wrapper" library around the Criteria API that tries to make coding type-safe Criteria queries simpler. So yes it is probably accurate to say these methods would not be regularly used by normal people, but they do represent "holes" in the API in the sense that support for every other case is provided and these holes cause problems for such a "wrapper" library.

For the first case, check out PathStream.cast(). The method returns the invoked object's type for method chaining and therefore has return type PathStream. Now consider FromStream which is a subtype of PathStream. Ideally, it should also have an overriding method FromStream.cast() that returns FromStream. But this is impossible to implement, because there's no version of Criteria.treat() that accepts a From.

Similarly for Subquery.correlate(), checkout the QueryStream.Builder API and the substream() methods it provides. The others can be implemented in a single line, while the substream(From) variant has to explicitly try all the other cases, and fails if an actual From is ever encountered (even though that may rarely happen):

        @SuppressWarnings("unchecked")
        public <X, Y> FromStream<Y, ? extends From<X, Y>> substream(From<X, Y> from) {
            if (from == null)
                throw new IllegalArgumentException("null join");
            if (from instanceof Root)
                return (FromStream<Y, ? extends From<X, Y>>)this.substream((Root<Y>)from);
            if (from instanceof SetJoin)
                return this.substream((SetJoin<X, Y>)from);
            if (from instanceof ListJoin)
                return this.substream((ListJoin<X, Y>)from);
            if (from instanceof MapJoin)
                return this.substream((MapJoin<X, ?, Y>)from);
            if (from instanceof CollectionJoin)
                return this.substream((CollectionJoin<X, Y>)from);
            if (from instanceof Join)
                return this.substream((Join<X, Y>)from);
            throw new UnsupportedOperationException("substream() from " + from.getClass().getName());
        }

Thanks.

P.S. I'm curious what you think about this library in general.

[gavinking]

@archiecobbs most of that switch is unnecessary, I believe, because Join is a supertype of all the other join types. So you only need two cases in the switch, I believe.

[archiecobbs]

Probably true. Regardless, that code is an ugly and incomplete workaround for the real problem. I look forward to discarding it when this ticket gets fixed :)

[gavinking]

I mean, I gotta be honest: you're not really selling me on this. If the only real payoff here is to eliminate the need for a single one-line function:

static <X,Y> Join<X,Y> correlate(Subquery<?> subquery, From<X,Y> from) {
    return from instanceof Join ? subquery.correlate((Join<X,Y>) from) : subquery.correlate((Root<X,Y>) from);
}

which is only needed in specialized library code, then I would say let's not do this. It just doesn't look to me like a massive burden to maintain that function.

[archiecobbs]

You seem to be asserting that every From instance is always actually either a Root or a Join for every JPA implementation that exists or may someday exist.

Even if that might happen to be true today, it's not safe to just assume that it always will be... unless you write that into the specification.

In any case, my point is not that no workaround exists, but rather that, in principle, a properly designed API should not be forcing workarounds like this in the first place.

That's why this bug is an API specification bug, not a "it doesn't work" type of bug.