[api-change] add soft limit of String type length #8567
Conversation
yangzhg
added
the
kind/api-change
morningman
added
dev/1.0.0-deprecated
morningman
left a comment
morningman
left a comment
There was a problem hiding this comment.
also need to modify schema change
yangzhg
force-pushed
the
fix_string
branch
4 times, most recently
from
6505552 to
e7b8239
Compare
be/src/exec/tablet_sink.cpp
Outdated
| case TYPE_STRING: { | ||
| StringValue* str_val = (StringValue*)slot; | ||
| if (str_val->len > OLAP_STRING_MAX_LENGTH) { | ||
| if (str_val->len > ACTRUAL_STRING_TYPE_MAX_LENGTH) { |
There was a problem hiding this comment.
| if (str_val->len > ACTRUAL_STRING_TYPE_MAX_LENGTH) { | |
| if (str_val->len > ACTUAL_STRING_TYPE_MAX_LENGTH) { |
be/src/olap/olap_define.h
Outdated
| // bloom filter fpp | ||
| static const double BLOOM_FILTER_DEFAULT_FPP = 0.05; | ||
|
|
||
| #define ACTRUAL_STRING_TYPE_MAX_LENGTH \ |
There was a problem hiding this comment.
string_type_length_soft_limit_bytes can be modified by runtime.
If you defined a macro here, then we can't modify it anymore.
the config supports pre-check, see compaction_task_num_per_disk.
We can use pre-check to check if the config value exceed the OLAP_STRING_MAX_LENGTH.
And then, we can use config::string_type_length_soft_limit_bytes directly.
be/src/runtime/types.h
Outdated
| TypeDescriptor ret; | ||
| ret.type = TYPE_STRING; | ||
| ret.len = MAX_STRING_LENGTH; | ||
| ret.len = ACTRUAL_STRING_TYPE_MAX_LENGTH; |
There was a problem hiding this comment.
Is this compatible with the String that has been created before which len is already larger than ACTRUAL_STRING_TYPE_MAX_LENGTH?
| throw new AnalysisException("Create olap table should contain distribution desc"); | ||
| } | ||
| distributionDesc.analyze(columnSet); | ||
| distributionDesc.analyze(columnSet, columnDefs); |
There was a problem hiding this comment.
There is another place we need to modify in this CreateTableStmt.
When user create a DUPLICATE KEY table without specify the key column, Doris will auto
select part of columns as KEY column.
And if it select a string column as KEY column, the error will return to user.
But this is not what user expected.
We should:
- stop selecting KEY column when met String column.
- return error msg to user when the first column is String and give a suggestion to user.
yangzhg
force-pushed
the
fix_string
branch
2 times, most recently
from
9dc19f9 to
88691eb
Compare
1. add a config string_type_soft_limit to soft limit max length of string type 2. disable using String type in Key column, partition column and distribution column 3. remove String type alias BLOB for futrue use
github-actions
bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
morningman
removed
the
dev/1.0.0-deprecated
1. add a config string_type_soft_limit to soft limit max length of string type 2. disable using String type in Key column, partition column and distribution column 3. remove String type alias BLOB for futrue use
morningman
added
dev/merged-1.0.1-deprecated
2 participants
Proposed changes
distribution column
Problem Summary:
Describe the overview of changes.
Checklist(Required)
Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...