Skip to content

Conversation

@HeartSaVioR
Copy link
Contributor

What changes were proposed in this pull request?

This PR proposes to fix a bug on calling DataType.equalsIgnoreCompatibleNullability with mistakenly swapped parameters in AppendData.outputResolved. The order of parameters for DataType.equalsIgnoreCompatibleNullability are from and to, which says that the right order of matching variables are inAttr and outAttr.

Why are the changes needed?

Although the problematic part is a dead code, once we know there's a bug, preferably we'd like to fix that.

Does this PR introduce any user-facing change?

No, as this fixes the dead code.

How was this patch tested?

This patch fixes the dead code, which is not easy to craft a test. (The test in original commit is no longer valid here.)

@HeartSaVioR
Copy link
Contributor Author

cc. @dongjoon-hyun
I just ported back only the fix, not test, because the test requires catalog implementation which is only available in 3.0.

@dongjoon-hyun
Copy link
Member

Thank you so much, @HeartSaVioR !

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-33136][SQL][BRANCH-2.4] Fix mistakenly swapped parameter in V2WriteCommand.outputResolved [SPARK-33136][SQL][2.4] Fix mistakenly swapped parameter in V2WriteCommand.outputResolved Oct 15, 2020
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. Merged to branch-2.4

dongjoon-hyun pushed a commit that referenced this pull request Oct 15, 2020
…mmand.outputResolved

### What changes were proposed in this pull request?

This PR proposes to fix a bug on calling `DataType.equalsIgnoreCompatibleNullability` with mistakenly swapped parameters in `AppendData.outputResolved`. The order of parameters for `DataType.equalsIgnoreCompatibleNullability` are `from` and `to`, which says that the right order of matching variables are `inAttr` and `outAttr`.

### Why are the changes needed?

Although the problematic part is a dead code, once we know there's a bug, preferably we'd like to fix that.

### Does this PR introduce _any_ user-facing change?

No, as this fixes the dead code.

### How was this patch tested?

This patch fixes the dead code, which is not easy to craft a test. (The test in original commit is no longer valid here.)

Closes #30043 from HeartSaVioR/SPARK-33136-branch-2.4.

Authored-by: Jungtaek Lim (HeartSaVioR) <kabhwan.opensource@gmail.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
@HeartSaVioR
Copy link
Contributor Author

Thanks for reviewing and merging!

@HeartSaVioR HeartSaVioR deleted the SPARK-33136-branch-2.4 branch October 15, 2020 03:10
@SparkQA
Copy link

SparkQA commented Oct 15, 2020

Test build #129785 has finished for PR 30043 at commit 162f6fe.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants