-
Notifications
You must be signed in to change notification settings - Fork 638
Implemented limit on olap table columnt count #8742
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
Conversation
⚪
🟢
*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 |
@@ -39,6 +39,7 @@ struct TSchemeLimits { | |||
|
|||
// table | |||
ui64 MaxTableColumns = 200; | |||
ui64 MaxOlapTableColumns = 10000; |
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'd suggest MaxColumnTableColumns
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.
Не хотелось чтобы 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 |
⚪ ⚪
⚪
🟢
*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 |
@@ -312,6 +312,28 @@ class TAlterColumnTable: public TSubOperation { | |||
return result; | |||
} | |||
|
|||
NSchemeShard::TPath parentPath = NSchemeShard::TPath::Resolve(parentPathStr, context.SS); |
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.
нужно проверки занести в 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а кто исключение ловит?
⚪
🟢
*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 |
@@ -36,6 +36,21 @@ NKikimr::TConclusionStatus TStandaloneSchemaUpdate::DoInitializeImpl(const TUpda | |||
return TConclusionStatus::Fail("schema update error: " + collector->GetErrorMessage() + ". in alter constructor STANDALONE_UPDATE"); | |||
} | |||
} | |||
|
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.
нужно не только для standalone, но и для in_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 |
auto domainInfo = parentPath.DomainInfo(); | ||
const TSchemeLimits& limits = domainInfo->GetSchemeLimits(); | ||
|
||
for (auto& [_, preset]: alterData->SchemaPresets) { |
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.
Можно, чтобы не дублировать проверки в alter store, create store, update table и create table, один раз написать валидацию апдейта в TColumnTableUpdate и вызывать её в местах, где этот апдейт применяется? Сейчас в четырёх местах переписана почти одинаковая логика, кажется лучше описать это в одном месте
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.
Этот объект доступен только в alter table
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.
Окей, и правда, но дублирование в create_store и alter_store точно можно убрать, потому что там инициализируется одна и та же AlterData.
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.
Но имхо будет лучше определить один валидатор 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Для 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 |
Changelog entry
...
Changelog category
Additional information
...