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

executor: don't reuse Executor in IndexLookUpJoin, remove doRequestForDatums() #5031

Merged
merged 11 commits into from Nov 10, 2017

Conversation

Projects
None yet
5 participants
@tiancaiamao
Contributor

tiancaiamao commented Nov 6, 2017

I change the DataReader interface, let it return a new Executor object every time:

type DataReader interface {
 doRequestForDatums(goCtx goctx.Context, datums [][]types.Datum) (Executor, error)
}

The returned Executor is not reused...

Remove doRequestForDatums() method from TableReaderExecutor IndexReaderExecutor IndexLookUpExecutor , and implement DataReader by introducing new struct and wrap around them....

@coocood @zz-jason

Show outdated Hide outdated executor/builder.go Outdated
Show outdated Hide outdated executor/builder.go Outdated
Show outdated Hide outdated executor/builder.go Outdated
Show outdated Hide outdated executor/builder.go Outdated
Show outdated Hide outdated executor/builder.go Outdated
@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao
Contributor

tiancaiamao commented Nov 9, 2017

@tiancaiamao tiancaiamao removed the status/WIP label Nov 9, 2017

Show outdated Hide outdated executor/builder.go Outdated
Show outdated Hide outdated executor/builder.go Outdated
Show outdated Hide outdated executor/builder.go Outdated
Show outdated Hide outdated executor/builder.go Outdated
Show outdated Hide outdated executor/builder.go Outdated
Show outdated Hide outdated executor/builder.go Outdated
Show outdated Hide outdated executor/distsql.go Outdated
table: e.table,
tableID: e.tableID,
dagPB: e.tableRequest,
schema: schema,
ctx: e.ctx,
}
err = tableReader.doRequestForHandles(goCtx, task.handles)
}, task.handles)
if err != nil {

This comment has been minimized.

@zz-jason

zz-jason Nov 9, 2017

Member

log this error ?

@zz-jason

zz-jason Nov 9, 2017

Member

log this error ?

This comment has been minimized.

@tiancaiamao

tiancaiamao Nov 9, 2017

Contributor

done

@tiancaiamao

tiancaiamao Nov 9, 2017

Contributor

done

Show outdated Hide outdated executor/index_lookup_join.go Outdated
Show outdated Hide outdated executor/index_lookup_join.go Outdated
Show outdated Hide outdated executor/builder.go Outdated
@coocood

This comment has been minimized.

Show comment
Hide comment
@coocood

coocood Nov 9, 2017

Member

LGTM

Member

coocood commented Nov 9, 2017

LGTM

tiancaiamao and others added some commits Nov 9, 2017

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Nov 9, 2017

Member

/run-all-tests

Member

zz-jason commented Nov 9, 2017

/run-all-tests

@zz-jason

LGTM

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Nov 10, 2017

Contributor

/run-common-tests

Contributor

tiancaiamao commented Nov 10, 2017

/run-common-tests

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Nov 10, 2017

Contributor

/run-integration-common-test

Contributor

tiancaiamao commented Nov 10, 2017

/run-integration-common-test

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Nov 10, 2017

Contributor

/run-common-tests tidb-test=pr/402

Contributor

tiancaiamao commented Nov 10, 2017

/run-common-tests tidb-test=pr/402

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Nov 10, 2017

Contributor

/run-integration-common-test tidb-test=pr/402

Contributor

tiancaiamao commented Nov 10, 2017

/run-integration-common-test tidb-test=pr/402

@iamxy

This comment has been minimized.

Show comment
Hide comment
@iamxy

iamxy Nov 10, 2017

Member

/run-integration-common-test

Member

iamxy commented Nov 10, 2017

/run-integration-common-test

@coocood coocood merged commit ecbee2e into pingcap:master Nov 10, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@tiancaiamao tiancaiamao deleted the tiancaiamao:data-reader branch Nov 10, 2017

dbjoa added a commit to cloud-pi/tidb that referenced this pull request Nov 30, 2017

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