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

stats: fix bug when merge sample collectors #4752

Merged
merged 6 commits into from Oct 12, 2017

Conversation

Projects
None yet
4 participants
@lamxTyler
Member

lamxTyler commented Oct 11, 2017

Count should be the total number of non-null rows, not sampled rows.
PTAL @coocood @hanfei1991 @winoros

@coocood

This comment has been minimized.

Show comment
Hide comment
@coocood

coocood Oct 11, 2017

Member

collect is also used in SampleBuilder in which case, Count should be used.

Member

coocood commented on statistics/sample.go in b332ab4 Oct 11, 2017

collect is also used in SampleBuilder in which case, Count should be used.

This comment has been minimized.

Show comment
Hide comment
@lamxTyler

lamxTyler Oct 11, 2017

Member

It used at here.

Member

lamxTyler replied Oct 11, 2017

It used at here.

lamxTyler added some commits Oct 11, 2017

@coocood

This comment has been minimized.

Show comment
Hide comment
@coocood

coocood Oct 11, 2017

Member

Add a field isMerger in SampleCollector, then remove argument insertSketch would be more clear.

Member

coocood commented Oct 11, 2017

Add a field isMerger in SampleCollector, then remove argument insertSketch would be more clear.

@coocood

This comment has been minimized.

Show comment
Hide comment
@coocood

coocood Oct 12, 2017

Member

LGTM

Member

coocood commented Oct 12, 2017

LGTM

@jackysp

This comment has been minimized.

Show comment
Hide comment
@jackysp

jackysp Oct 12, 2017

Member

/run-all-tests

Member

jackysp commented Oct 12, 2017

/run-all-tests

@winoros winoros merged commit 182476e into master Oct 12, 2017

11 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 72.624%
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
jenkins-ci-tidb/common-test Jenkins job succeeded.
Details
jenkins-ci-tidb/integration-common-test Jenkins job succeeded.
Details
jenkins-ci-tidb/integration-ddl-test Jenkins job succeeded.
Details
jenkins-ci-tidb/mybatis-test Jenkins job succeeded.
Details
jenkins-ci-tidb/sqllogic-test Jenkins job succeeded.
Details
jenkins-ci-tidb/unit-test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@winoros winoros deleted the xhb/fic branch Oct 12, 2017

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