Skip to content

expression: use new metadata instead of implicit_args#13962

Merged
sre-bot merged 7 commits intopingcap:masterfrom
TennyZhuang:in-union-metadata
Dec 9, 2019
Merged

expression: use new metadata instead of implicit_args#13962
sre-bot merged 7 commits intopingcap:masterfrom
TennyZhuang:in-union-metadata

Conversation

@TennyZhuang
Copy link
Contributor

@TennyZhuang TennyZhuang commented Dec 7, 2019

Signed-off-by: TennyZhuang zty0826@gmail.com

What problem does this PR solve?

Currently, implicit_args can only pass data with []Datum format, it's not reasonable for structured metadata, such as for in optimization, we need some metadata format like https://github.com/TennyZhuang/tikv/blob/fdb0bb3777f2bb32716a10beb776cc5e1c55125f/components/tidb_query/src/rpn_expr/impl_compare_in.rs#L125

What is changed and how it works?

This PR introduce arbitrary metadata format defined with protobuf in tipb, encoding in TiDB and decoding in TiKV.

Related to pingcap/tipb#161

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

This feature will break the implicit_args format, but will not break anything really, because only cast use implicit_args, and the pushdown switch of cast is never open before.

Related changes

Release note

  • Introduce new implicit_args format

Signed-off-by: TennyZhuang <zty0826@gmail.com>
@TennyZhuang TennyZhuang requested a review from a team as a code owner December 7, 2019 17:33
@ghost ghost requested review from XuHuaiyu and wshwsh12 and removed request for a team December 7, 2019 17:33
@TennyZhuang
Copy link
Contributor Author

I will remove the replace in go.mod after pingcap/tipb#161 merged

@TennyZhuang
Copy link
Contributor Author

@breeswish

Signed-off-by: TennyZhuang <zty0826@gmail.com>
@codecov
Copy link

codecov bot commented Dec 8, 2019

Codecov Report

Merging #13962 into master will decrease coverage by 0.0372%.
The diff coverage is 90%.

@@               Coverage Diff                @@
##             master     #13962        +/-   ##
================================================
- Coverage   80.1896%   80.1524%   -0.0373%     
================================================
  Files           480        480                
  Lines        120316     120196       -120     
================================================
- Hits          96481      96340       -141     
- Misses        16142      16164        +22     
+ Partials       7693       7692         -1

Signed-off-by: TennyZhuang <zty0826@gmail.com>
Signed-off-by: TennyZhuang <zty0826@gmail.com>
Signed-off-by: TennyZhuang <zty0826@gmail.com>
@TennyZhuang TennyZhuang changed the title expression: use new implicit_args format expression: use new metadata instead of implicit_args Dec 8, 2019
Copy link
Member

@breezewish breezewish left a comment

Choose a reason for hiding this comment

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

The rest LGTM

Signed-off-by: TennyZhuang <zty0826@gmail.com>
@breezewish
Copy link
Member

LGTM

@TennyZhuang
Copy link
Contributor Author

@wshwsh12 @XuHuaiyu PTAL

Copy link
Contributor

@lonng lonng left a comment

Choose a reason for hiding this comment

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

LGTM

@lonng
Copy link
Contributor

lonng commented Dec 9, 2019

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 9, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 9, 2019

/run-all-tests

@zz-jason zz-jason added component/expression contribution This PR is from a community contributor. labels Dec 9, 2019
@sre-bot sre-bot merged commit d7206fb into pingcap:master Dec 9, 2019
XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/expression contribution This PR is from a community contributor. status/can-merge Indicates a PR has been approved by a committer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants