-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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.
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
[Feature](Schema Change)light schema change support add key column #42911
base: master
Are you sure you want to change the base?
[Feature](Schema Change)light schema change support add key column #42911
Conversation
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
clang-tidy review says "All clean, LGTM! 👍" |
0fe8150
to
9a99b34
Compare
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
clang-tidy review says "All clean, LGTM! 👍" |
9a99b34
to
72f1191
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
be/src/vec/olap/block_reader.cpp
Outdated
@@ -15,8 +15,6 @@ | |||
// specific language governing permissions and limitations | |||
// under the License. | |||
|
|||
#include "vec/olap/block_reader.h" | |||
|
|||
#include <gen_cpp/olap_file.pb.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'gen_cpp/olap_file.pb.h' file not found [clang-diagnostic-error]
#include <gen_cpp/olap_file.pb.h>
^
72f1191
to
98197ee
Compare
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 41363 ms
|
TPC-DS: Total hot run time: 194832 ms
|
ClickBench: Total hot run time: 32.08 s
|
ec7d59a
to
79d0dc6
Compare
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
clang-tidy review says "All clean, LGTM! 👍" |
79d0dc6
to
4c5aefd
Compare
clang-tidy review says "All clean, LGTM! 👍" |
4c5aefd
to
fbdeccd
Compare
run buildall |
TPC-H: Total hot run time: 32617 ms
|
TeamCity be ut coverage result: |
TPC-DS: Total hot run time: 196644 ms
|
ClickBench: Total hot run time: 31.8 s
|
fbdeccd
to
324eed6
Compare
run buildall |
0ab8b9a
to
a7be770
Compare
run buildall |
TPC-H: Total hot run time: 32512 ms
|
TeamCity be ut coverage result: |
TPC-DS: Total hot run time: 197498 ms
|
ClickBench: Total hot run time: 30.93 s
|
ALTER TABLE add_keys_light_schema_change ADD COLUMN new_key_column INT default "2" AFTER user_id PROPERTIES ("timeout"="604800") | ||
""" | ||
|
||
def jobStateResult = waitJobFinish(""" SHOW ALTER TABLE COLUMN WHERE IndexName='add_keys_light_schema_change' ORDER BY createtime DESC LIMIT 1 """, 9) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not need wait, because light weight schema change is sync process. Not need background process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jobStateResult is used to check schema change is use light schema change or not.
`sex` TINYINT COMMENT "用户性别", | ||
|
||
`cost` BIGINT DEFAULT "0" COMMENT "用户总消费") | ||
UNIQUE KEY(`user_id`, `date`, `city`, `age`, `sex`) DISTRIBUTED BY HASH(`user_id`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also need test unique key merge on read
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌🏻,i will add case 4.1 for this scene
BUCKETS 4 | ||
PROPERTIES ( | ||
"replication_num" = "1", | ||
"light_schema_change" = "true", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all test cases should disable compaction
INSERT INTO add_keys_light_schema_change VALUES | ||
(1, '2017-10-01', 'Beijing', 10, 1, 20) | ||
""" | ||
qt_sc_12 """ select * from add_keys_light_schema_change order by user_id """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test case should follow this process:
- insert values
- add key column with default value
- insert values
- check select value
qt_sc_12 """ select * from add_keys_light_schema_change order by user_id """ | ||
|
||
sql """ | ||
ALTER TABLE add_keys_light_schema_change ADD COLUMN new_mv_key1 INT KEY default "2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should also test delete condition:
- insert values
- add key column with default value
- delete where new keycolumn = xxx
- insert values
- check select value
qt_sc_12 """ select * from add_keys_light_schema_change order by user_id """ | ||
|
||
sql """ | ||
ALTER TABLE add_keys_light_schema_change ADD COLUMN new_mv_key1 INT KEY default "2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the org table is key(col1,col2)
you add col3, the new key is (col1,col2,col3) or (col3,col1,col2) or (col1,col3,col2) ?
I think you should test all these cases.
sql """ DROP TABLE IF EXISTS add_keys_light_schema_change """ | ||
sql """ | ||
CREATE TABLE IF NOT EXISTS add_keys_light_schema_change ( | ||
`user_id` LARGEINT NOT NULL COMMENT "用户id", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should test all typed key columns such as string,date,int.
For example, the original key column could be one of string,date,int
and you add a new key column, it could also be one of string,date,int
jobStateResult = waitJobFinish(""" SHOW ALTER TABLE COLUMN WHERE IndexName='add_keys_light_schema_change' ORDER BY createtime DESC LIMIT 1 """, 9) | ||
assertNotEquals(jobStateResult[0][8], "-1") | ||
|
||
// case 2.1: light schema change aggreage : with multi version rowset and compaction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should check these query values:
- select * from new table
- select * from newtable where key=xxx
""" | ||
table_tablets = sql """ SHOW TABLETS FROM add_keys_light_schema_change ORDER BY RowCount DESC LIMIT 1 """ | ||
|
||
qt_41_unique_multi_rowset """ SELECT * FROM add_keys_light_schema_change; """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For all test cases, you should check the compaction logic:
- first disable compaction and run all cases.
- call compaction API to do compaction for the table, and then run all test cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
a7be770
to
23f7672
Compare
run buildall |
TPC-H: Total hot run time: 32354 ms
|
TeamCity be ut coverage result: |
TPC-DS: Total hot run time: 196160 ms
|
ClickBench: Total hot run time: 30.82 s
|
23f7672
to
2c7af74
Compare
run buildall |
TPC-H: Total hot run time: 32826 ms
|
TPC-DS: Total hot run time: 197143 ms
|
ClickBench: Total hot run time: 31.48 s
|
TeamCity be ut coverage result: |
Proposed changes
Background
Currently, the LightSchemaChange feature in Doris only supports adding value columns, not supports the regular key column adding. If we add a regular key column to a simple table that not in the short key columns, it will do with HardLinkedSchemaChange; But for the cooldown tablets that save on remote system like S3, it will degrade to DirectSchemaChange, The schema change time is very slow. This modification is intended to replace HardLinkedSchemaChange with LightSchemaChange for normal key column adding.
Modification
Modify LightSchemaChange check logic in FE, adding key column adding scene.