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

*: change the way transfer handle. #4348

Merged
merged 41 commits into from Sep 27, 2017

Conversation

Projects
None yet
6 participants
@winoros
Member

winoros commented Aug 28, 2017

Don't transfer handle if we don't need it, so remove rowMeta in selectResponse.
Implement it first on mock-tikv.
PTAL @hanfei1991 @shenli @zimulala

Show outdated Hide outdated store/tikv/coprocessor.go
Show outdated Hide outdated store/tikv/mock-tikv/cop_handler_dag.go
Show outdated Hide outdated executor/executor_test.go
@@ -88,7 +93,14 @@ func (p *PhysicalTableScan) ToPB(ctx context.Context) (*tipb.Executor, error) {
func (p *PhysicalIndexScan) ToPB(ctx context.Context) (*tipb.Executor, error) {
columns := make([]*model.ColumnInfo, 0, p.schema.Len())
for _, col := range p.schema.Columns {
columns = append(columns, p.Table.Columns[col.Position])
if col.ID == model.ExtraHandleID {

This comment has been minimized.

@zimulala

zimulala Aug 29, 2017

Member

Do we need to check TypeSupported here?

@zimulala

zimulala Aug 29, 2017

Member

Do we need to check TypeSupported here?

This comment has been minimized.

@winoros

winoros Aug 29, 2017

Member

added

@winoros

winoros Aug 29, 2017

Member

added

This comment has been minimized.

@winoros

winoros Sep 7, 2017

Member

removed check.

@winoros

winoros Sep 7, 2017

Member

removed check.

Show outdated Hide outdated plan/plan_to_pb.go
Show outdated Hide outdated distsql/new_distsql.go
Show outdated Hide outdated distsql/new_distsql.go
Show outdated Hide outdated distsql/new_distsql.go
Show outdated Hide outdated distsql/new_distsql.go
Show outdated Hide outdated distsql/new_distsql.go

winoros added some commits Aug 30, 2017

@hanfei1991

This comment has been minimized.

Show comment
Hide comment
@hanfei1991

hanfei1991 Sep 5, 2017

Member

We can remove the switch to update at once

Member

hanfei1991 commented Sep 5, 2017

We can remove the switch to update at once

winoros added some commits Sep 7, 2017

@winoros

This comment has been minimized.

Show comment
Hide comment
@winoros
Member

winoros commented Sep 7, 2017

@zimulala zimulala added status/DNM and removed status/WIP labels Sep 8, 2017

winoros added some commits Sep 8, 2017

Show outdated Hide outdated executor/distsql.go
Show outdated Hide outdated executor/builder.go
Show outdated Hide outdated plan/new_physical_plan_builder.go
@winoros

This comment has been minimized.

Show comment
Hide comment
@winoros

winoros Sep 12, 2017

Member

/build

Member

winoros commented Sep 12, 2017

/build

@winoros winoros removed the status/DNM label Sep 12, 2017

@winoros winoros changed the title from *: change the way transfer handle when using mock-tikv. to *: change the way transfer handle. Sep 12, 2017

@winoros winoros added the status/DNM label Sep 12, 2017

@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli

shenli Sep 25, 2017

Member

@winoros Please resolve the conflicts.

Member

shenli commented Sep 25, 2017

@winoros Please resolve the conflicts.

Show outdated Hide outdated plan/column_pruning.go
Show outdated Hide outdated executor/new_distsql.go
Show outdated Hide outdated executor/builder.go

winoros added some commits Sep 26, 2017

winoros added some commits Sep 26, 2017

@zimulala

This comment has been minimized.

Show comment
Hide comment
@zimulala

zimulala Sep 26, 2017

Member

LGTM

Member

zimulala commented Sep 26, 2017

LGTM

@winoros

This comment has been minimized.

Show comment
Hide comment
@winoros

winoros Sep 26, 2017

Member

We should merge this pr with tikv/tikv#2302 at the same time.
So DNM currently.

Member

winoros commented Sep 26, 2017

We should merge this pr with tikv/tikv#2302 at the same time.
So DNM currently.

indexReq.OutputOffsets = []uint32{uint32(len(is.Index.Columns))}
var (
handleCol *expression.Column
tableReaderSchema *expression.Schema

This comment has been minimized.

@hanfei1991

hanfei1991 Sep 26, 2017

Member

Can we construct table schema in physical plan builder ? We can set table schema to table plan.

@hanfei1991

hanfei1991 Sep 26, 2017

Member

Can we construct table schema in physical plan builder ? We can set table schema to table plan.

This comment has been minimized.

@winoros

winoros Sep 27, 2017

Member

add a TODO

@winoros

winoros Sep 27, 2017

Member

add a TODO

@winoros

This comment has been minimized.

Show comment
Hide comment
@winoros

winoros Sep 27, 2017

Member

/run-integration-test tikv=pr/yiding/handle

Member

winoros commented Sep 27, 2017

/run-integration-test tikv=pr/yiding/handle

@winoros winoros removed the status/DNM label Sep 27, 2017

coocood and others added some commits Sep 27, 2017

@winoros winoros merged commit 0fd5b51 into master Sep 27, 2017

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
ci/circleci Your tests passed on CircleCI!
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@winoros winoros deleted the yiding/handle branch Sep 27, 2017

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