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

infoschema: add initial support for views #8892

Merged
merged 8 commits into from Jan 4, 2019

Conversation

6 participants
@morgo
Copy link
Member

morgo commented Dec 30, 2018

What problem does this PR solve?

Fixes #8889

What is changed and how it works?

  • Fixed a compatibility bug: s/CHECK_SUM/CHECKSUM/
  • Added correct info for information_schema.tables.
  • Added correct info for information_schema.views.

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change
  • Has persistent data change

Side effects

  • Increased code complexity
  • Breaking backward compatibility (renamed column CHECK_SUM to CHECKSUM for compatibility)

Related changes

  • Need to update the documentation
  • Need to be included in the release note

This change is Reviewable

@morgo morgo added the status/DNM label Dec 30, 2018

morgo added some commits Dec 30, 2018

@morgo morgo removed the status/DNM label Dec 30, 2018

@morgo

This comment has been minimized.

Copy link
Member

morgo commented Dec 30, 2018

@morgo

This comment has been minimized.

Copy link
Member

morgo commented Dec 30, 2018

@AndrewDi There is one difference from MySQL, which is the that table.View.CheckOption.String() returns CASCADED | LOCAL. MySQL just returns YES | NO. I am not sure, which is the best way to proceed.. :-)

@morgo morgo added this to In progress in Support View via automation Dec 30, 2018

Show resolved Hide resolved infoschema/tables.go Outdated
@zz-jason
Copy link
Member

zz-jason left a comment

LGTM

@morgo morgo added the status/LGT1 label Jan 3, 2019

@XuHuaiyu XuHuaiyu changed the title infoschema: Add initial support for views infoschema: add initial support for views Jan 3, 2019

Support View automation moved this from In progress to Reviewer approved Jan 3, 2019

@XuHuaiyu
Copy link
Contributor

XuHuaiyu left a comment

LGTM

@XuHuaiyu XuHuaiyu added status/LGT2 and removed status/LGT1 labels Jan 3, 2019

@zz-jason

This comment has been minimized.

Copy link
Member

zz-jason commented Jan 3, 2019

/run-all-tests

@XuHuaiyu

This comment has been minimized.

Copy link
Contributor

XuHuaiyu commented Jan 3, 2019

@morgo It seems that we need to update tidb-test repo as well.

@morgo

This comment has been minimized.

Copy link
Member

morgo commented Jan 3, 2019

/run-all-tests tidb-test=pr/712

@morgo

This comment has been minimized.

Copy link
Member

morgo commented Jan 3, 2019

This test fails, but I believe it is a false positive:

FAIL: ddl_test.go:478: testSuite3.TestSetDDLReorgWorkerCnt

ddl_test.go:483:
    c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(1))
... obtained int32 = 16
... expected int32 = 1
@XuHuaiyu

This comment has been minimized.

Copy link
Contributor

XuHuaiyu commented Jan 4, 2019

@winkyao @zimulala PTAL, do you have any idea for this?

@crazycs520

This comment has been minimized.

Copy link
Contributor

crazycs520 commented Jan 4, 2019

let me take a look.

@zz-jason

This comment has been minimized.

Copy link
Member

zz-jason commented Jan 4, 2019

/run-all-tests tidb-test=pr/712

@XuHuaiyu XuHuaiyu merged commit cef6e75 into pingcap:master Jan 4, 2019

11 of 12 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
ci/circleci Your tests passed on CircleCI!
Details
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/common-test job succeeded
Details
idc-jenkins-ci-tidb/integration-common-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-compatibility-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-ddl-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/mybatis-test job succeeded
Details
idc-jenkins-ci-tidb/sqllogic-test-1 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/sqllogic-test-2 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/unit-test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

Support View automation moved this from Reviewer approved to Done Jan 4, 2019

@morgo morgo deleted the morgo:infoschema branch Jan 4, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 4, 2019

Codecov Report

Merging #8892 into master will increase coverage by 0.02%.
The diff coverage is 93.4%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8892      +/-   ##
==========================================
+ Coverage   67.52%   67.54%   +0.02%     
==========================================
  Files         363      363              
  Lines       74928    74984      +56     
==========================================
+ Hits        50593    50648      +55     
  Misses      19877    19877              
- Partials     4458     4459       +1
Impacted Files Coverage Δ
executor/show.go 41.05% <100%> (ø) ⬆️
infoschema/tables.go 52.32% <93.33%> (+3.8%) ⬆️
ddl/delete_range.go 77.71% <0%> (-1.15%) ⬇️
store/tikv/lock_resolver.go 41.7% <0%> (-0.95%) ⬇️
infoschema/builder.go 75.74% <0%> (+0.99%) ⬆️
infoschema/infoschema.go 79.57% <0%> (+1.4%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d098d4...df99bc6. Read the comment docs.

zimulala added a commit that referenced this pull request Jan 4, 2019

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