Test: support qualified column reference in writing tests#5614
Test: support qualified column reference in writing tests#5614ti-chi-bot merged 15 commits intopingcap:masterfrom
Conversation
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. DetailsReviewer can indicate their review by submitting an approval review. |
|
/run-all-tests |
|
/run-all-tests |
|
/run-all-tests |
|
/run-all-tests |
|
/run-unit-test |
…ified_table_column_in_test
665400c to
748210e
Compare
|
/run-unit-tests |
|
/run-all-tests |
| std::vector<String> executors = { | ||
| "exchange_sender_6 | type:Hash, {<0, String>}\n" | ||
| " table_scan_1 | {<0, String>}", | ||
| "exchange_sender_6 | type:Hash, {<0, String>, <1, String>}\n" |
There was a problem hiding this comment.
because we can distinguish the column name by table_name and column_name, so the columnPrune result is different.
|
/run-unit-tests |
Coverage for changed filesCoverage summaryfull coverage report (for internal network access only) |
| request = left_builder.build(context); | ||
| { | ||
| String expected = "limit_8 | 10\n" | ||
| " Join_7 | LeftOuterJoin, HashJoin. left_join_keys: {<0, String>}, right_join_keys: {<0, String>}\n" |
There was a problem hiding this comment.
why <0, String> or not <3, String>?
There was a problem hiding this comment.
why
<0, String>or not<3, String>?
Related to executorSerializer.cpp:50, toString function print the column from index 0.
…ified_table_column_in_test
|
According to the struct definition struct ColumnName
{
String db_name;
String table_name;
String column_name;
};our qualified format seems to be "db_name.table_name.column_name", can you add some tests about this kind? |
|
/run-all-tests |
|
/run-unit-tests |
Coverage for changed filesCoverage summaryfull coverage report (for internal network access only) |
|
lgtm |
ebe80cd to
8504906
Compare
|
/merge |
|
@ywqzzy: It seems you want to merge this PR, I will help you trigger all the tests: /run-all-tests You only need to trigger If you have any questions about the PR merge process, please refer to pr process. DetailsInstructions 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. |
|
This pull request has been accepted and is ready to merge. DetailsCommit hash: f9f4141 |
Coverage for changed filesCoverage summaryfull coverage report (for internal network access only) |
What problem does this PR solve?
Issue Number: close #5510
Problem Summary:
What is changed and how it works?
Modify qualified name into three parts, add a new struct to hold the column name.
Check List
Tests
Side effects
Documentation
Release note