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

fix view #1174

Merged
merged 1 commit into from
Oct 29, 2019
Merged

fix view #1174

merged 1 commit into from
Oct 29, 2019

Conversation

zhexuany
Copy link
Contributor

What problem does this PR solve?

This PR fixes TiSpark cannot parse view table's json.

What is changed and how it works?

ViewCols could be null. If such case happens, we do not need parse viewCols.

Tests

  • Integration test

@zhexuany
Copy link
Contributor Author

/run-all-tests

@@ -4,23 +4,29 @@ class ViewTestSuite extends BaseTiSparkTest {
private val table = "test_view"

test("Test View") {
tidbStmt.execute(s"drop table if exists $table")
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if there's some dirty data (e.g. a table named test_view) in database?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

afterAll will drop it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

but we should still drop it before each test

Copy link
Collaborator

Choose a reason for hiding this comment

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

see this
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it partitionTableSuite? If you take a look at afterAll, you will find that table t is not dropped at afterAll.

try {
tidbStmt.execute("drop view if exists v")
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if there's some dirty data (e.g. a view named v) in database?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

afterAll will drop it.

Copy link
Contributor

@birdstorm birdstorm left a comment

Choose a reason for hiding this comment

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

LGTM

@birdstorm birdstorm merged commit 2467720 into pingcap:master Oct 29, 2019
@marsishandsome
Copy link
Collaborator

@birdstorm would u please add more information to the commit message?

@zhexuany zhexuany deleted the fix_view_bug branch October 30, 2019 02:25
zhexuany added a commit to zhexuany/tispark that referenced this pull request Oct 30, 2019
zhexuany added a commit that referenced this pull request Oct 31, 2019
wfxxh pushed a commit to wanfangdata/tispark that referenced this pull request Jun 30, 2023
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