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.

Sign up for GitHub

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

Jump to bottom

add GeneratedColumnPlaceholderInputStream #6796

Merged
merged 26 commits into from Feb 13, 2023
Merged

add GeneratedColumnPlaceholderInputStream #6796

merged 26 commits into from Feb 13, 2023

Conversation

guo-shaoge
Copy link
Contributor

@guo-shaoge guo-shaoge commented Feb 10, 2023 •
edited

Signed-off-by: guo-shaoge shaoge1994@163.com

What problem does this PR solve?

Issue Number: close #6801 , ref pingcap/tidb#40663

Problem Summary: Check description in pingcap/tidb#41261

What is changed and how it works?

Add GeneratedColumnPlaceholderBlockInputStream.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

None

@guo-shaoge
add GeneratedColumnPlaceholderInputStream
ade7a44 
Signed-off-by: guo-shaoge <shaoge1994@163.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Feb 10, 2023 •
edited

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • SeaRise
  • windtalker

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added do-not-merge/needs-linked-issue release-note-none size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 10, 2023
@guo-shaoge
clear gtest change
b58875e 
Signed-off-by: guo-shaoge <shaoge1994@163.com>
@guo-shaoge
fix log
fddbabe 
Signed-off-by: guo-shaoge <shaoge1994@163.com>
@ti-chi-bot ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 10, 2023
@guo-shaoge guo-shaoge mentioned this pull request Feb 10, 2023
Merged
12 tasks
@guo-shaoge
add case
93a4c11 
Signed-off-by: guo-shaoge <shaoge1994@163.com>
@ti-chi-bot ti-chi-bot added do-not-merge/needs-triage-completed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 10, 2023
ywqzzy and others added 14 commits February 10, 2023 13:53
@ywqzzy @guo-shaoge
least/greatest function for string (pingcap#6662)
a6daf46 
close pingcap#6660
@ywqzzy @guo-shaoge
fix tls ipv6 metric (pingcap#6736)
5b033a1 
close pingcap#6734
@windtalker @guo-shaoge
Refine spiller (pingcap#6780)
6a1bee4 
ref pingcap#6528
@lidezhu @guo-shaoge
remove legacy code in RegionPersister (pingcap#6781)
9f8724e 
ref pingcap#6728
@flowbehappy @guo-shaoge
fix RegionKVStoreTest (pingcap#6768)
3e8ce03 
close pingcap#6767
@SeaRise @guo-shaoge
Collect request unit for tiflash compute (pingcap#6649)
8981dfa 
close pingcap#6678
@JaySon-Huang @guo-shaoge
Remove useless IDAsPathUpgrader (pingcap#6782)
c6493d4 
ref pingcap#6233
@SeaRise @guo-shaoge
fix tsan fail in unit test JoinExecutorTestRunner.MultiJoin (pingca…
76e725e 
…p#6770)

close pingcap#6765
@SeaRise @guo-shaoge
Refine the hostname backlist in MetricsPrometheus (pingcap#6784)
c5ea678 
ref pingcap#6678
@windtalker @guo-shaoge
Add more information to the error message in MPPTunnel (pingcap#6787)
0d25d4d 
ref pingcap#6233
@LittleFall @guo-shaoge
fix the issue that decimal divide not round. (pingcap#6471)
206b807 
close pingcap#4488, close pingcap#6393, close pingcap#6462
@windtalker @guo-shaoge
Refine the spill threshold for aggregation/sort, refine the memory us…
ff2949f 
…age calculation for aggregation (pingcap#6708)

ref pingcap#6528
@guo-shaoge
fix format
8de0167 
Signed-off-by: guo-shaoge <shaoge1994@163.com>
@guo-shaoge
Merge branch 'master' into gen_col_placeholder
8eb8ab7
@SeaRise SeaRise self-requested a review February 13, 2023 04:00
AilinKid
AilinKid reviewed Feb 13, 2023
View reviewed changes
dbms/src/Flash/Coprocessor/DAGQueryBlockInterpreter.cpp Outdated
@@ -185,6 +185,16 @@ void DAGQueryBlockInterpreter::handleMockTableScan(const TiDBTableScan & table_s
analyzer = std::make_unique<DAGExpressionAnalyzer>(std::move(names_and_types), context);
pipeline.streams.insert(pipeline.streams.end(), mock_table_scan_streams.begin(), mock_table_scan_streams.end());
}
std::vector<std::pair<UInt64, DataTypePtr>> generated_column_infos;
Copy link
Contributor

Choose a reason for hiding this comment

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

should we do it also in handleTableScan?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean DAGQueryBlockInterpreter::handleTableScan?
DAGQueryBlockInterpreter::handleTableScan will call DAGStorageInterpter::execute(). so no need to handle.

@guo-shaoge
add getGeneratedColumnInfos
2c24e58 
Signed-off-by: guo-shaoge <shaoge1994@163.com>
@guo-shaoge
del getGeneratedColumnInfos
ec1914d 
Signed-off-by: guo-shaoge <shaoge1994@163.com>
@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 13, 2023
@guo-shaoge
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

@guo-shaoge: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: ec1914d

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 13, 2023
@ti-chi-bot
Copy link
Member

@AilinKid: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@guo-shaoge
Copy link
Contributor Author

/test integration-test

@guo-shaoge
Copy link
Contributor Author

/run-integration-test

This was referenced Feb 13, 2023
Merged
Merged
@ti-chi-bot ti-chi-bot merged commit e84ed48 into pingcap:master Feb 13, 2023
ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Feb 13, 2023
@guo-shaoge @ti-chi-bot
This is an automated cherry-pick of pingcap#6796
3834650 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot ti-chi-bot mentioned this pull request Feb 13, 2023
Merged
12 tasks
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-6.1: #6812.

@ti-chi-bot ti-chi-bot mentioned this pull request Feb 13, 2023
Merged
12 tasks
ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Feb 13, 2023
@guo-shaoge @ti-chi-bot
This is an automated cherry-pick of pingcap#6796
631499f 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-6.5: #6813.

ti-chi-bot added a commit that referenced this pull request Mar 29, 2023
@ti-chi-bot
add GeneratedColumnPlaceholderInputStream (#6796) (#6812)
9b3e36d 
close #6801, ref pingcap/tidb#40663
ti-chi-bot added a commit that referenced this pull request Apr 4, 2023
@ti-chi-bot
add GeneratedColumnPlaceholderInputStream (#6796) (#6813)
16223d8 
close #6801, ref pingcap/tidb#40663
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@windtalker windtalker windtalker approved these changes

@AilinKid AilinKid AilinKid approved these changes

@SeaRise SeaRise SeaRise approved these changes

Assignees
No one assigned
Labels
needs-cherry-pick-release-6.1 Type: Need cherry pick to release-6.1 needs-cherry-pick-release-6.5 release-note-none size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Got cannot find column error for generated column
10 participants
@guo-shaoge @ti-chi-bot @windtalker @AilinKid @SeaRise @ywqzzy @lidezhu @flowbehappy @JaySon-Huang @LittleFall