You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "dataroaring (via GitHub)" <gi...@apache.org> on 2023/04/19 10:00:33 UTC
[GitHub] [doris] dataroaring opened a new pull request, #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
dataroaring opened a new pull request, #18822:
URL: https://github.com/apache/doris/pull/18822
LSC updates tablet's schema in writing. Be optimizes adding columns via linked schema change and it distinguishes adding by compare column name. e.g. if new column's name is not found in old schema, then it is a added column.
When a table is under schema-changing, it adds __doris_shadow_ prefix in name of columns in shadow index.
# Proposed changes
Issue Number: close #xxx
## Problem summary
Describe your changes.
## Checklist(Required)
* [ ] Does it affect the original behavior
* [ ] Has unit tests been added
* [ ] Has document been added or modified
* [ ] Does it need to update dependencies
* [ ] Is this PR support rollback (If NO, please explain WHY)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring commented on pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring commented on PR #18822:
URL: https://github.com/apache/doris/pull/18822#issuecomment-1516143451
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18822:
URL: https://github.com/apache/doris/pull/18822#issuecomment-1523477489
clang-tidy review says "All clean, LGTM! :+1:"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring commented on pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring commented on PR #18822:
URL: https://github.com/apache/doris/pull/18822#issuecomment-1523534776
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring commented on pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring commented on PR #18822:
URL: https://github.com/apache/doris/pull/18822#issuecomment-1525336630
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18822:
URL: https://github.com/apache/doris/pull/18822#discussion_r1171121009
##########
be/src/olap/schema_change.cpp:
##########
@@ -1681,23 +1681,29 @@ Status SchemaChangeHandler::_parse_request(const SchemaChangeParams& sc_params,
}
}
- int32_t column_index = base_tablet_schema->field_index(column_name);
+ int32_t column_index = base_tablet_schema->field_index( std::string_view(column_name));
Review Comment:
warning: no matching member function for call to 'field_index' [clang-diagnostic-error]
```cpp
int32_t column_index = base_tablet_schema->field_index( std::string_view(column_name));
^
```
**be/src/olap/tablet_schema.h:187:** candidate function not viable: no known conversion from 'std::string_view' (aka 'basic_string_view<char>') to 'const std::string' (aka 'const basic_string<char>') for 1st argument
```cpp
int32_t field_index(const std::string& field_name) const;
^
```
**be/src/olap/tablet_schema.h:188:** candidate function not viable: no known conversion from 'std::string_view' (aka 'basic_string_view<char>') to 'int32_t' (aka 'int') for 1st argument
```cpp
int32_t field_index(int32_t col_unique_id) const;
^
```
##########
be/src/olap/schema_change.cpp:
##########
@@ -1681,23 +1681,29 @@
}
}
- int32_t column_index = base_tablet_schema->field_index(column_name);
+ int32_t column_index = base_tablet_schema->field_index( std::string_view(column_name));
if (column_index >= 0) {
column_mapping->ref_column = column_index;
continue;
}
+ if (column_name.starts_with("__doris_shadow_")) {
Review Comment:
warning: no member named 'starts_with' in 'std::basic_string<char>' [clang-diagnostic-error]
```cpp
if (column_name.starts_with("__doris_shadow_")) {
^
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring commented on pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring commented on PR #18822:
URL: https://github.com/apache/doris/pull/18822#issuecomment-1522640227
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18822:
URL: https://github.com/apache/doris/pull/18822#discussion_r1171194613
##########
be/src/olap/schema_change.cpp:
##########
@@ -1681,12 +1681,16 @@ Status SchemaChangeHandler::_parse_request(const SchemaChangeParams& sc_params,
}
}
- int32_t column_index = base_tablet_schema->field_index(column_name);
+ int32_t column_index = base_tablet_schema->field_index(std::string_view(column_name));
Review Comment:
warning: no matching member function for call to 'field_index' [clang-diagnostic-error]
```cpp
int32_t column_index = base_tablet_schema->field_index(std::string_view(column_name));
^
```
**be/src/olap/tablet_schema.h:187:** candidate function not viable: no known conversion from 'std::string_view' (aka 'basic_string_view<char>') to 'const std::string' (aka 'const basic_string<char>') for 1st argument
```cpp
int32_t field_index(const std::string& field_name) const;
^
```
**be/src/olap/tablet_schema.h:188:** candidate function not viable: no known conversion from 'std::string_view' (aka 'basic_string_view<char>') to 'int32_t' (aka 'int') for 1st argument
```cpp
int32_t field_index(int32_t col_unique_id) const;
^
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18822:
URL: https://github.com/apache/doris/pull/18822#issuecomment-1529041860
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18822:
URL: https://github.com/apache/doris/pull/18822#issuecomment-1525339557
clang-tidy review says "All clean, LGTM! :+1:"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman commented on a diff in pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "morningman (via GitHub)" <gi...@apache.org>.
morningman commented on code in PR #18822:
URL: https://github.com/apache/doris/pull/18822#discussion_r1181235747
##########
fe/fe-core/src/main/java/org/apache/doris/planner/OlapTableSink.java:
##########
@@ -212,7 +212,7 @@ private TOlapTableSchemaParam createSchema(long dbId, OlapTable table) {
List<String> columns = Lists.newArrayList();
List<TColumn> columnsDesc = Lists.newArrayList();
List<TOlapTableIndex> indexDesc = Lists.newArrayList();
- columns.addAll(indexMeta.getSchema().stream().map(Column::getName).collect(Collectors.toList()));
+ columns.addAll(indexMeta.getSchema().stream().map(Column::getNonShadowName).collect(Collectors.toList()));
Review Comment:
The column with `__doris_shadow_` prefix can only be created when doing `modify column`.
And most of `modify column` operation is not a "light schema change" operation.
But there is an exception, which is doing "modify varchar column's length", which is a `modify column` operation
and will do "light schema change".
So I think we should handle it in `SchemaChangeHandler.processModifyColumn()`, to avoid creating column with
shadow prefix if it is a light schema change, instead of modifying here.
And on BE side, there will still be column with shadow prefix, eg, modify column without light schema change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman merged pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "morningman (via GitHub)" <gi...@apache.org>.
morningman merged PR #18822:
URL: https://github.com/apache/doris/pull/18822
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18822:
URL: https://github.com/apache/doris/pull/18822#issuecomment-1515724962
clang-tidy review says "All clean, LGTM! :+1:"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman commented on a diff in pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "morningman (via GitHub)" <gi...@apache.org>.
morningman commented on code in PR #18822:
URL: https://github.com/apache/doris/pull/18822#discussion_r1181243070
##########
fe/fe-core/src/main/java/org/apache/doris/planner/OlapTableSink.java:
##########
@@ -212,7 +212,7 @@ private TOlapTableSchemaParam createSchema(long dbId, OlapTable table) {
List<String> columns = Lists.newArrayList();
List<TColumn> columnsDesc = Lists.newArrayList();
List<TOlapTableIndex> indexDesc = Lists.newArrayList();
- columns.addAll(indexMeta.getSchema().stream().map(Column::getName).collect(Collectors.toList()));
+ columns.addAll(indexMeta.getSchema().stream().map(Column::getNonShadowName).collect(Collectors.toList()));
Review Comment:
Oh, I see.
The `columns` here is for loading task, and the schema will be saved in rowset's meta(not tablet's meta).
And for rowset meta, there should be no column with `shadow` prefix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18822:
URL: https://github.com/apache/doris/pull/18822#issuecomment-1529041867
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18822:
URL: https://github.com/apache/doris/pull/18822#issuecomment-1521455907
clang-tidy review says "All clean, LGTM! :+1:"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring commented on pull request #18822: [fix](schema_change) remove shadow prefix of schema for tablesink
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring commented on PR #18822:
URL: https://github.com/apache/doris/pull/18822#issuecomment-1523528766
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org