SqmSelection registry

[sebersole]

At the moment, resolution of selections in the SQM tree (for order-by and group-by handling) are handled through SqmPathRegistry; specifically this part of it:

public interface SqmPathRegistry {
    ...
    void register(SqmSelection selection);
    SqmSelection findSelectionByAlias(String alias);
    SqmSelection findSelectionByPosition(int position);
}

This set-up causes trouble in at least one valid use-case...

Consider the query select new Zoo( z.name as zname, z.address as zaddress) from Zoo z order by zname, zaddress. It is trying to order by the aliases assigned to the dynamic-instantiation args. Ultimately this tries to locate an SqmSelection based on the alias specified in the order-by.

The way this query is modeled in the SQM tree however is that there is just a single SqmSelection which is for the dynamic-instantiation expression (with no alias). So the attempt to resolve zname and zaddress against the registry will fail.

I think there are 2 considerations here...

Use of SqmSelection

Assuming we don't want to force the arguments to be SqmSelection refs also (I don't think we should), we aren't going to be able to use SqmSelection in the registry contract. Actually, I think the best would be to use SqmAliasedNode which has exactly 2 impls: SqmSelection and SqmDynamicInstantiationArgument, the exact 2 node types we want to deal with here:

public interface SqmPathRegistry {
    ...
    void register(SqmAliasedNode selection);
    SqmAliasedNode findSelectionByAlias(String alias);
    SqmAliasedNode findSelectionByPosition(int position);
}

Regarding the SqmDynamicInstantiation, I think we should simply not register it. Considering the scope of this registry is resolving order-by and group-by references - I don't think a query like this is valid : select new Zoo( z.name, z.address) as ctor from Zoo z order by ctor. So the registration of the SqmDynamicInstantiation is (1) unnecessary and (2) actually problematic for positional resolution.

Separate registry?

For background, I cannot remember why we decided to combine the notion of a "path registry" and an "aliased selection registry".

This is not so much a functional consideration, but I wonder if we want to split this part of SqmPathRegistry into its own contract, e.g.

/**
 * Track aliased nodes defined within the select-clause
 */
public interface SqmAliasedNodeRegistry {
    void register(SqmAliasedNode selection);
    SqmAliasedNode findNodeByAlias(String alias);
    SqmAliasedNode findNodeByPosition(int position);
}

public interface SqmPathRegistry {
    ...
}


public interface SqmCreationProcessingState {
    ...

    // used to just be this method
    SqmPathRegistry getPathRegistry();

    // proposed addition
    SqmAliasedNodeRegistry getAliasedNodeRegistry();
}

Like I said though, this is not a functional consideration - this all works fine with just the changes to SqmPathRegistray outlined in Use of SqmSelection section above. This is more about better "functional encapsulation".

[beikov]

Using SqmAliasedNode instead of SqmSelection sounds like a good approach to me.