C#: Fix join order in InterpretedCallable characteristic predicate.

[michaelnebel]

The original solution joined on Callable and elementSpec based on name, which lead to a quite large intermediate relation.
The tuples counts in all the screenshots below are based on the rappayne_webgoat database.

The tuple counts for the original solution can be seen here:

To improve the join ordering and to keep the intermediate relations as small as possible, we use the following observations

• When we want to use specific specs (ie. a spec that directly targets a callable), we would like to join Callable and elementSpec on three columns in one go (namespace, type and name).
• When we want to use specs that are valid for all overrides (subtypes = true), we need project the elementSpec relation (on subtypes = true as this is relatively small subset) and join on Callable name. Afterwards we can inspect the types.

The new predicates and tuples counts can be seen here.

[michaelnebel]

It looks like DCA is somewhat broken ATM when it comes to reporting some of the data for join order badness (we can't see the detail of a specific join order problem, only that there might be one).
In any case, the following DCA executions indicates that the changes on this PR solves the join order problem for InterpretedCallable:

Running DCA against main has 281 issues reported in the join order badness section (33 related to InterpretedCallable):
https://github.com/github/codeql-dca-main/blob/data/michaelnebel/main__nightly__nightly/reports/summaries/any.md#join-order-badness-per-source-and-predicate-v0

Running DCA against the branch on this PR has 248 issues reported in the join order badness secion (0 related to InterpretedCallable)
https://github.com/github/codeql-dca-main/blob/data/michaelnebel/fix-joinorder-i__nightly__nightly/reports/summaries/any.md#join-order-badness-per-source-and-predicate-v0

Also note that math says: 281 - 33 = 248

[hvitved]

First join-fix LGTM, I think the second can be improved.

[hvitved]

This looks like a potentially bad join, as we will relate all calls with matching names. How about first joining on getASubTypeUnbound+() instead?

pragma[nomagic]
private predicate subtypeSpecCandidate(string name, UnboundValueOrRefType t) {
  exists(UnboundValueOrRefType t0 |
    elementSpec(_, _, true, name, _, _, t0) and
    t = t0.getASubTypeUnbound+()
  )
}

pragma[nomagic]
private predicate callableInfo(Callable c, string name, UnboundValueOrRefType decl) {
  name = c.getName() and
  decl = c.getDeclaringType()
}

and then use it in InterpretedCallable as

exists(string name, UnboundValueOrRefType t |
  callableInfo(this, name, t) and
  subtypeSpecCandidate(name, t)
)

[michaelnebel]

Yes, I think you are right! However, in this particular case the join is worsened slightly.
Let us try with your solution and run DCA.

[hvitved]

However, in this particular case the join is worsened slightly.

I think that is only because we are currently lucky that elementSpec(_, _, true, name, _, _, t) is rather small. But even so, if, say, elementSpec(_, _, "ToString", _, _, t) held, we would join with all ToString methods, which can be a large set.

[michaelnebel]

Yes - I totally agree.

[michaelnebel]

Weird actions error on last DCA execution. Will try again.

[michaelnebel]

DCA looks good
(1) InterpretedCallable has disappeared from the join order badness list.
(2) None of the newly introduced predicates have turned up on the join order badness list