Skip to content

executor: add vectorized data access methods to Column#11368

Merged
coocood merged 17 commits into
pingcap:masterfrom
qw4990:vecexpr-column-vec-API
Jul 22, 2019
Merged

executor: add vectorized data access methods to Column#11368
coocood merged 17 commits into
pingcap:masterfrom
qw4990:vecexpr-column-vec-API

Conversation

@qw4990
Copy link
Copy Markdown
Contributor

@qw4990 qw4990 commented Jul 22, 2019

What problem does this PR solve?

Add some vectorized data access methods to Column to let Expression can access its data directly instead of using Row and RowIter, which is prepared for vectorized expression evaluation.

What is changed and how it works?

Two kinds of data access methods are added to Column:

  1. Types() []Type: added for fixed-length types like i64, f64, MyDecimal;
  2. GetType(rowID) Type: added for var-length types like JSON, String;

And IsNull is exposed.

Most of these changes are new methods or unit tests, so it's easy for you to review although there are hundreds of lines changed.

Check List

Tests

  • Unit test

@qw4990 qw4990 changed the title executor: add vectorized data access methods to Column and Chunk executor: add vectorized data access methods to Column Jul 22, 2019
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 22, 2019

Codecov Report

Merging #11368 into master will decrease coverage by 0.3224%.
The diff coverage is 88.8888%.

@@               Coverage Diff                @@
##             master     #11368        +/-   ##
================================================
- Coverage   81.5947%   81.2722%   -0.3225%     
================================================
  Files           423        423                
  Lines         91577      90310      -1267     
================================================
- Hits          74722      73397      -1325     
- Misses        11552      11608        +56     
- Partials       5303       5305         +2

Comment thread util/chunk/column.go Outdated
Comment thread util/chunk/column.go Outdated
Comment thread util/chunk/column.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we consider unsigned float32 and float64?

Comment thread util/chunk/column.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

how about Decimals?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rename it to Decimals;

Comment thread util/chunk/row.go Outdated
Comment thread util/chunk/chunk.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

colIdx is more consistent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

renamed.

Copy link
Copy Markdown
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 22, 2019
@zz-jason
Copy link
Copy Markdown
Member

/run-all-tests

XuHuaiyu
XuHuaiyu previously approved these changes Jul 22, 2019
Copy link
Copy Markdown
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 status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 22, 2019
@qw4990
Copy link
Copy Markdown
Contributor Author

qw4990 commented Jul 22, 2019

/run-all-tests

@coocood
Copy link
Copy Markdown
Member

coocood commented Jul 22, 2019

LGTM

@qw4990
Copy link
Copy Markdown
Contributor Author

qw4990 commented Jul 22, 2019

Please help me to approve this PR @XuHuaiyu @coocood @zz-jason

@coocood coocood merged commit 268be86 into pingcap:master Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sig/execution SIG execution status/LGT2 Indicates that a PR has LGTM 2. type/new-feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants