Implemented limit on olap table columnt count#8742
Implemented limit on olap table columnt count#8742aavdonkin merged 7 commits intoydb-platform:mainfrom
Conversation
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ Details
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
|
||
| // table | ||
| ui64 MaxTableColumns = 200; | ||
| ui64 MaxOlapTableColumns = 10000; |
There was a problem hiding this comment.
I'd suggest MaxColumnTableColumns
There was a problem hiding this comment.
Не хотелось чтобы 2 раза слово Column было в названии
|
⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ Details
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
| return result; | ||
| } | ||
|
|
||
| NSchemeShard::TPath parentPath = NSchemeShard::TPath::Resolve(parentPathStr, context.SS); |
There was a problem hiding this comment.
нужно проверки занести в TInStoreSchemaUpdate.
это объект, который занимается модификацией схемы.
|
|
||
| if (Transaction.HasAlterColumnTable()) { | ||
| auto& alterSchema = Transaction.GetAlterColumnTable().GetAlterSchema(); | ||
| TTablesStorage::TTableReadGuard table = context.SS->ColumnTables.at(path->PathId); |
There was a problem hiding this comment.
а кто исключение ловит?
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
| return TConclusionStatus::Fail("schema update error: " + collector->GetErrorMessage() + ". in alter constructor STANDALONE_UPDATE"); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
нужно не только для standalone, но и для in_store
|
⚪ ⚪ Details
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
| auto domainInfo = parentPath.DomainInfo(); | ||
| const TSchemeLimits& limits = domainInfo->GetSchemeLimits(); | ||
|
|
||
| for (auto& [_, preset]: alterData->SchemaPresets) { |
There was a problem hiding this comment.
Можно, чтобы не дублировать проверки в alter store, create store, update table и create table, один раз написать валидацию апдейта в TColumnTableUpdate и вызывать её в местах, где этот апдейт применяется? Сейчас в четырёх местах переписана почти одинаковая логика, кажется лучше описать это в одном месте
There was a problem hiding this comment.
Этот объект доступен только в alter table
There was a problem hiding this comment.
Окей, и правда, но дублирование в create_store и alter_store точно можно убрать, потому что там инициализируется одна и та же AlterData.
There was a problem hiding this comment.
Но имхо будет лучше определить один валидатор TColumnTableSchema, который принимает TSchemeLimits и возвращает TConclusion и использовать его повсюду, чтобы этот код валидации был написан один раз (а не 4 :] ):
if (columnCount > limits.MaxColumnTableColumns) {
TString errStr = TStringBuilder()
<< "Too many columns"
<< ". new: " << columnCount
<< ". Limit: " << limits.MaxColumnTableColumns;
result->SetError(NKikimrScheme::StatusSchemeError, errStr);
return result;
}
There was a problem hiding this comment.
Для create store и alter store переместил проверку в одно место
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
4 participants
Changelog entry
...
Changelog category
Additional information
...