fix: Disallows dropping duplicate keys when using full outer join

[renato2099]

Which issue does this PR close?

Closes #1305

Rationale for this change

Datafusion-python should follow datafusion implementation and disallow dropping keys when doing a full outer join as both keys are not equivalent thus they can't just be dropped. Users can then decide on how to proceed based on their use case.

What changes are included in this PR?

fix + unit test

Are there any user-facing changes?

yes, disallowing dropping keys when doing a full outer join as that is not semantically correct.

[kosiew]

@renato2099

Thanks for working on this.

Documentation gaps:

The doc string for join in python/datafusion/dataframe.py states:

drop_duplicate_keys: When True, the columns from the right DataFrame
    that have identical names in the ``on`` fields to the left DataFrame
    will be dropped.

It does not mention the full join exception. Users reading this may assume the parameter works the same for all join types.

Similarly, docs/source/user-guide/common-operations/joins.rst should also document for the full join and drop_duplicate_keys behaviour.

[renato2099]

Hi @kosiew , thanks for taking a look at the PR!

I have added some documentation notes. Let me know if this is sufficient, otherwise I can add more explanations.

[renato2099]

I am thinking that we could have a follow up on this path to be more ergonomic though + a more future-proof API (non-breaking path). Basically, we could introduce an enum-like parameter alongside the boolean, deprecating the latter later on, we could have something like:

join_key_behavior: Literal[
    "drop_right",     # current drop_duplicate_keys=True
    "keep_both",      # current drop_duplicate_keys=False
    "coalesce",       # coalesce both columns if that is really what user wants to do
] | None = None

Then

• If join_key_behavior is provided, then we would ignore drop_duplicate_keys
• FULL JOIN would allow only "keep_both"
• INNER / LEFT / RIGHT would allow "drop_right" or "keep_both"

but we could do that in a follow up PR, wdyt @kosiew ?

[timsaucer]

Closing this PR since the consensus has landed on using the coalesce approach instead. Thank you for the PR and helpful discussions!