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

executor, expression: support Chunk in ProjectionExec #5178

Merged
merged 4 commits into from
Nov 22, 2017

Conversation

zz-jason
Copy link
Member

To simplify code, I haven't support vectorized expression evaluation here, to support it, we have to check whether there exists some special expressions, I'll do it in the near future.

Here is an example that we have to evaluate expressions row by row:

create table t(a bigint);
insert into t values(1), (2), (3);
select @a, @a := a+1 from t;

Main changes:

  1. Let ProjectionExec support chunk and add implement function NextChunk.
  2. Implemented EvalExprsToChunk in package expression to evaluate a list of expression based on an input Chunk and append all the result to an output Chunk. We can further improve it to evaluate output Chunk column by column, ie. support vectorized expression evaluation.
  3. Add a new function named newChunk to interface executor

@zz-jason
Copy link
Member Author

/run-all-tests

func EvalExprsToChunk(ctx context.Context, exprs []Expression, input, output *chunk.Chunk) error {
sc := ctx.GetSessionVars().StmtCtx
for rowID, length := 0, input.NumRows(); rowID < length; rowID++ {
for colID, expr := range exprs {
Copy link
Member

Choose a reason for hiding this comment

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

It would be more cache friendly If we move exprs range to outer for loop.

}

func evalOneCell(sc *variable.StatementContext, expr Expression, input, output *chunk.Chunk, rowID, colID int) error {
switch fieldType, evalType := expr.GetType(), expr.GetType().EvalType(); evalType {
Copy link
Member

Choose a reason for hiding this comment

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

The EvalType is constant for every row, so we can loop all the rows under a certain eval type case.

@coocood
Copy link
Member

coocood commented Nov 21, 2017

LGTM

XuHuaiyu
XuHuaiyu previously approved these changes Nov 22, 2017
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

LGTM

@XuHuaiyu XuHuaiyu added the status/LGT2 Indicates that a PR has LGTM 2. label Nov 22, 2017
@zz-jason zz-jason merged commit 55b8f9f into pingcap:master Nov 22, 2017
res, isNull, err := expr.EvalInt(input.GetRow(rowID), sc)
if err != nil {
return errors.Trace(err)
} else if isNull {
Copy link
Member

Choose a reason for hiding this comment

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

Save this else.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, I'll do it in the next pr

}
case types.ETReal:
res, isNull, err := expr.EvalReal(input.GetRow(rowID), sc)
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@zz-jason zz-jason deleted the dev/chunk/projection branch November 22, 2017 05:36
@zz-jason zz-jason mentioned this pull request Nov 29, 2017
41 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants