[FLINK-38211][table-planner] Refactor explain for MultiJoin

[gustavodemorais]

What is the purpose of the change

It'd be useful to have some changes to the explain to make debugging and looking at the job graph easier:

• Add inputUniqueKeys
• Add commonJoinKey
• Add names instead of indices for joinFilter and joinConditions
• Remove joinAttributeMap since it's not easily readable and really long

Brief change log

• Refactor explain in physical node
• Update tests accordingly

Verifying this change

Plan and restore tests updated (this is ok because the operator is in an experimental state)

Does this pull request potentially affect one of the following parts:

• Dependencies (does it add or upgrade a dependency): (no)
• The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
• The serializers: (no)
• The runtime per-record code paths (performance sensitive): (no)
• Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
• The S3 file system connector: (no)

Documentation

• Does this pull request introduce a new feature? (no)
• If yes, how is the feature documented? (not applicable)

[flinkbot]

CI report:

• 0f83263 UNKNOWN
• 278ef9b Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot run azure re-run the last Azure build

[davidradl]

@gustavodemorais looks like a planner test is failing in the are you have changed.

[gustavodemorais]

I've rebased it twice already so I'll rebase it after getting a review

[twalthr]

we should make sure that this is insync with other explains, I would suggest to keep it rowType

[twalthr]

remove joinFilter=[true]

[twalthr]

remove as it is contained in joinConditions already

[gustavodemorais]

Thanks for the review, @twalthr! I've addressed your comments.

I've also moved the only single plan test that was outside the MultiJoinTest file so that it's simpler to regenerate plans in the future. (The test wasn't changed).

[gustavodemorais]

On another note, do you think it makes sense to remove the position 0 from all arrays related to joins? Right now, for N inputs, all arrays in the explain are of length N. However, for N inputs, we have N - 1 joins.

The first position of all arrays in the MultiJoin node related to joins are a special case. Each position in the array is related to the join between inputs i and i - 1. The position 0 has some default values since these are the values related to joining input 0 and "input -1". For example, a regular two way join has arrays with a length of two for the properties

joinTypes=[INNER, INNER],
inputUniqueKeys=[(user_id), (order_id, user_id)] 
joinConditions=[true, =(user_id, user_id0)]

Calcite creates as a default value for pos 0. The same thing with the joinTypes, the first position is always INNER. In general, I think it either makes sense keep the position 0 like above for all attributes so they have the same length or remove for it only for the join-related attributes. What do you think?

[twalthr]

I agree to simplify the explain here. A binary join should be:

joinTypes=[INNER],
inputUniqueKeys=[(user_id), (order_id, user_id)] 
joinConditions=[=(user_id, user_id0)]

So inputUniqueKeys relates to inputs and thus N input == N values. Others are join related and thus N-1.

[gustavodemorais]

I've updated both joinTypes and joinConditions to skip the first position. It looks cleaner 👍

I've also moved up stateTtlHints to the beginning of the list next to inputUniqueKeys, since it's also related to inputs and is a short and a more relevant information than the lengthy outputType.

I think we have everything now. Can you take a look @twalthr?

[twalthr]

LGTM