Skip to content

Conversation

@cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

remove the table from AlterTable/DescribeTable.children.

Why are the changes needed?

For DDL/DML commands, we only want to get the v2 table from DataSourceV2Relation. Things can go wrong if other rules transform the query plan and mistakenly convert DataSourceV2Relation in DDL/DML commands to something else.

As discussed in #26091 , we should exclude the table from AlterTable/DescribeTable.children, so that they can't be transformed unintentionally. This is also what we did for the writing plans (AppendData for example).

The code is mostly extracted from #25955 , to ease the review of that PR. All credits go to @rdblue

Does this PR introduce any user-facing change?

no

How was this patch tested?

existing tests

@cloud-fan
Copy link
Contributor Author

cc @rdblue @brkyvz

@cloud-fan cloud-fan added the SQL label Oct 30, 2019
@SparkQA
Copy link

SparkQA commented Oct 30, 2019

Test build #112909 has finished for PR 26318 at commit fda91c7.

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

@rdblue
Copy link
Contributor

rdblue commented Oct 30, 2019

@cloud-fan, this is included in #25955. Why resubmit this separately?

@cloud-fan
Copy link
Contributor Author

At the time I was reviewing #25955 , it was excluded. I thought it's intentional. Let me close it.

@cloud-fan cloud-fan closed this Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants