fix(join): skip substitution of non-field references in join chains

[coroa]

Description of changes

As alternative to #9594, we here avoid the same signature error raised in #9561, when a Literal or more general Value in a JoinChain fails to be included in a DerefMap.

A statement like
t3 = t1.inner_join(t2, "i").select("i", lit=1, val=2 * t1.x) creates a JoinChain looking like

JoinChain[r0]
  JoinLink[inner, r1]
    r0.i == r1.i
  values:
    i:     r0.i
    lit:   1
    val:   2 * r0.x

with one of the reference "fields" in values actually pointing to a Literal and another one to a more general Value.

Adding a subsequent JoinLink to this JoinChain like by
t5 = t3.inner_join(t4, "i") fails in prepare_predicates, since the DerefMap.from_targets(..., extra=reverse) call tries to add a substitute from the new lit-field to the Literal(1) object and from val to the Value-like expression, which both do not fit the Field type, that the subs attribute of the DerefMap are expected to have.

Here we skip those as direct substitution possibilities.

Issues closed

• Resolves bug: SignatureValidationError from DerefMap during repeated joins #9561

[github-actions]

ACTION NEEDED

Ibis follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message.

Please update your PR title and description to match the specification.

[cpcloud]

@coroa I pushed up a small test.

[coroa]

@coroa I pushed up a small test.

Thanks! But note that the SignatureValidationError was only triggered by adding another JoinLink.

Should i extend the test for that?

[cpcloud]

Ah, yes!

[coroa]

@cpcloud What do you think about that addition to the test?

[cpcloud]

LGTM, I have another case I want to add, pushing that here in a sec.

[cpcloud]

Thanks for the PR!

[coroa]

Doctest failure looks unrelated (table ordering instability).

Thanks for the quick turn-around, it's a delight.

[cpcloud]

Yep, failure is unrelated, and fixed in #9597.

[coroa]

FYI for debugging i found it very helpful to add some simple __repr__ methods to

class Field(Value):
    [...]
    def __repr__(self):
        return f"<Field name={self.name} rel={self.rel}>"

and

class Relation(Node, Coercible):
    [...]
    def __repr__(self):
        return f"<{self.__class__.__name__} fields={','.join(self.schema)}>"

I saw that you have some formatting facilities with pretty and so on, but having a simple Node representation would make the error messages quite a lot clearer.