Fix test framework's groupby expression support #5419

Willendless · 2022-07-19T22:45:52Z

What problem does this PR solve?

Issue Number: close #5418

Problem Summary:

What is changed and how it works?

check whether agg/groupby ASTPtr is null before pushing them into expression list

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

Release note

None

ti-chi-bot · 2022-07-19T22:45:53Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

SeaRise
ywqzzy

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.

Willendless · 2022-07-19T23:00:50Z

/run-all-tests

sre-bot · 2022-07-19T23:17:16Z

Coverage for changed files

Filename                           Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/tests/gtest_executor.cpp         119                50    57.98%           3                 0   100.00%          52                 0   100.00%          32                18    43.75%
TestUtils/mockExecutor.cpp              76                 4    94.74%          43                 2    95.35%         280                27    90.36%          38                 2    94.74%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                  195                54    72.31%          46                 2    95.65%         332                27    91.87%          70                20    71.43%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18626      9373             49.68%    212372  94729        55.39%

full coverage report (for internal network access only)

Willendless · 2022-07-19T23:44:35Z

/run-integration-test

SeaRise

LGTM

SeaRise · 2022-07-20T02:32:28Z

dbms/src/Flash/tests/gtest_executor.cpp

@@ -60,5 +60,29 @@ try
 }
 CATCH

+// todo: move this test to gtest_aggregation_executor.cpp


maybe you can do now?

ywqzzy

LGTM

Willendless · 2022-07-20T22:29:24Z

/run-all-tests

Willendless · 2022-07-20T22:31:10Z

/cc @xzhangxian1008

feel free to modify my test case when you support more aggregation expressions and add corresponding tests

:)

sre-bot · 2022-07-20T22:46:13Z

Coverage for changed files

Filename                                       Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/tests/gtest_aggregation_executor.cpp         289                97    66.44%           7                 0   100.00%         179                 1    99.44%          70                35    50.00%
TestUtils/mockExecutor.cpp                          76                 4    94.74%          43                 2    95.35%         280                27    90.36%          38                 2    94.74%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                              365               101    72.33%          50                 2    96.00%         459                28    93.90%         108                37    65.74%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18626      9374             49.67%    212376  94710        55.40%

full coverage report (for internal network access only)

SeaRise · 2022-07-21T01:10:57Z

/merge

ti-chi-bot · 2022-07-21T01:10:59Z

@SeaRise: 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 · 2022-07-21T01:11:01Z

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

Commit hash: c20e757

sre-bot · 2022-07-21T01:28:17Z

Coverage for changed files

Filename                                       Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/tests/gtest_aggregation_executor.cpp         289                97    66.44%           7                 0   100.00%         179                 1    99.44%          70                35    50.00%
TestUtils/mockExecutor.cpp                          76                 4    94.74%          43                 2    95.35%         280                27    90.36%          38                 2    94.74%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                              365               101    72.33%          50                 2    96.00%         459                28    93.90%         108                37    65.74%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18632      9372             49.70%    212524  94696        55.44%

full coverage report (for internal network access only)

sre-bot · 2022-07-21T03:37:15Z

Coverage for changed files

Filename                                       Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/tests/gtest_aggregation_executor.cpp         289                97    66.44%           7                 0   100.00%         179                 1    99.44%          70                35    50.00%
TestUtils/mockExecutor.cpp                          76                 4    94.74%          43                 2    95.35%         280                27    90.36%          38                 2    94.74%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                              365               101    72.33%          50                 2    96.00%         459                28    93.90%         108                37    65.74%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18632      9374             49.69%    212524  94731        55.43%

full coverage report (for internal network access only)

ti-chi-bot · 2022-07-21T03:57:51Z

@Willendless: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

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.

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.

sre-bot · 2022-07-21T04:17:40Z

Coverage for changed files

Filename                                       Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/tests/gtest_aggregation_executor.cpp         289                97    66.44%           7                 0   100.00%         179                 1    99.44%          70                35    50.00%
TestUtils/mockExecutor.cpp                          76                 4    94.74%          43                 2    95.35%         280                27    90.36%          38                 2    94.74%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                              365               101    72.33%          50                 2    96.00%         459                28    93.90%         108                37    65.74%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18632      9373             49.69%    212519  94684        55.45%

full coverage report (for internal network access only)

Willendless added 2 commits July 19, 2022 18:43

fix: avoid pushing back nullptr for group by and agg func

3088468

chore: add simple tests for null agg or group_by expr

f79858f

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-triage-completed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 19, 2022

chore: format

e2d0489

SeaRise approved these changes Jul 20, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 20, 2022

ywqzzy approved these changes Jul 20, 2022

View reviewed changes

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 Jul 20, 2022

Willendless changed the title ~~Fix test framework's groupby expression support~~ [WIP] Fix test framework's groupby expression support Jul 20, 2022

ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 20, 2022

chore: move agg null test to aggregation executor file

c20e757

ti-chi-bot removed the do-not-merge/needs-triage-completed label Jul 20, 2022

Willendless changed the title ~~[WIP] Fix test framework's groupby expression support~~ Fix test framework's groupby expression support Jul 20, 2022

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 20, 2022

ti-chi-bot requested a review from xzhangxian1008 July 20, 2022 22:31

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 21, 2022

Merge branch 'master' into fix-issue-5418

208a55e

Merge branch 'master' into fix-issue-5418

29c9fd7

Merge branch 'master' into fix-issue-5418

c927e7e

ti-chi-bot merged commit 15478d7 into pingcap:master Jul 21, 2022

Willendless deleted the fix-issue-5418 branch July 21, 2022 20:31

ywqzzy mentioned this pull request Aug 25, 2022

Interpreter & Executor & MPP unit test framework #4609

Closed

54 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix test framework's groupby expression support #5419

Fix test framework's groupby expression support #5419

Willendless commented Jul 19, 2022

ti-chi-bot commented Jul 19, 2022 •

edited

Loading

Willendless commented Jul 19, 2022

sre-bot commented Jul 19, 2022

Willendless commented Jul 19, 2022

SeaRise left a comment

SeaRise Jul 20, 2022

Willendless Jul 20, 2022

ywqzzy left a comment

Willendless commented Jul 20, 2022

Willendless commented Jul 20, 2022

sre-bot commented Jul 20, 2022

SeaRise commented Jul 21, 2022

ti-chi-bot commented Jul 21, 2022

ti-chi-bot commented Jul 21, 2022

sre-bot commented Jul 21, 2022

sre-bot commented Jul 21, 2022

ti-chi-bot commented Jul 21, 2022

sre-bot commented Jul 21, 2022

Fix test framework's groupby expression support #5419

Fix test framework's groupby expression support #5419

Conversation

Willendless commented Jul 19, 2022

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jul 19, 2022 • edited Loading

Willendless commented Jul 19, 2022

sre-bot commented Jul 19, 2022

Coverage for changed files

Coverage summary

Willendless commented Jul 19, 2022

SeaRise left a comment

Choose a reason for hiding this comment

SeaRise Jul 20, 2022

Choose a reason for hiding this comment

Willendless Jul 20, 2022

Choose a reason for hiding this comment

ywqzzy left a comment

Choose a reason for hiding this comment

Willendless commented Jul 20, 2022

Willendless commented Jul 20, 2022

sre-bot commented Jul 20, 2022

Coverage for changed files

Coverage summary

SeaRise commented Jul 21, 2022

ti-chi-bot commented Jul 21, 2022

ti-chi-bot commented Jul 21, 2022

sre-bot commented Jul 21, 2022

Coverage for changed files

Coverage summary

sre-bot commented Jul 21, 2022

Coverage for changed files

Coverage summary

ti-chi-bot commented Jul 21, 2022

sre-bot commented Jul 21, 2022

Coverage for changed files

Coverage summary

ti-chi-bot commented Jul 19, 2022 •

edited

Loading