Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-17673] [SQL] Incorrect exchange reuse with RowDataSourceScan #15273

Closed
wants to merge 3 commits into from

Conversation

ericl
Copy link
Contributor

@ericl ericl commented Sep 28, 2016

What changes were proposed in this pull request?

It seems the equality check for reuse of RowDataSourceScanExec nodes doesn't respect the output schema. This can cause self-joins or unions over the same underlying data source to return incorrect results if they select different fields.

How was this patch tested?

New unit test passes after the fix.

@ericl ericl changed the title [SPARK-17673] [SQL] [WIP] Incorrect exchange reuse with RowDataSourceScan [SPARK-17673] [SQL] Incorrect exchange reuse with RowDataSourceScan Sep 28, 2016
@SparkQA
Copy link

SparkQA commented Sep 28, 2016

Test build #66014 has finished for PR 15273 at commit c0e08b8.

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

@SparkQA
Copy link

SparkQA commented Sep 28, 2016

Test build #66017 has finished for PR 15273 at commit f0df3a0.

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

@SparkQA
Copy link

SparkQA commented Sep 28, 2016

Test build #66018 has finished for PR 15273 at commit e2dfb25.

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

@rxin
Copy link
Contributor

rxin commented Sep 28, 2016

Did the unit test fail without the fix?

@ericl
Copy link
Contributor Author

ericl commented Sep 28, 2016

Yep, it returns 1 row only.

On Wed, Sep 28, 2016, 12:59 AM Reynold Xin notifications@github.com wrote:

Did the unit test fail without the fix?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#15273 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAA6Sjsfx6ug7bh3-hu4ItWjGx5mANC6ks5quh58gaJpZM4KIWY9
.

@rxin
Copy link
Contributor

rxin commented Sep 28, 2016

Thanks - merging in master/2.0.

I think the test is actually in a non-ideal location, but I'm going to merge first in order to make 2.0.1 rc4.

@asfgit asfgit closed this in a6cfa3f Sep 28, 2016
asfgit pushed a commit that referenced this pull request Sep 28, 2016
…ackport)

This backports #15273 to branch-2.0

Also verified the test passes after the patch was applied. rxin

Author: Eric Liang <ekl@databricks.com>

Closes #15282 from ericl/spark-17673-2.
asfgit pushed a commit that referenced this pull request Sep 28, 2016
## What changes were proposed in this pull request?

As a followup for #15273 we should move non-JDBC specific tests out of that suite.

## How was this patch tested?

Ran the test.

Author: Eric Liang <ekl@databricks.com>

Closes #15287 from ericl/spark-17713.
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