Unifying column names validation for both row and column tables #14722 #15453
Conversation
|
🟢 |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
| TString tableDescr = R"( | ||
| Name: "TestTable" | ||
| Schema { | ||
| Columns { |
There was a problem hiding this comment.
Add Column with name containing all of allowed symbols
|
⚪ Test history | Ya make output | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ |
|
|
||
| status = annoyingClient.CreateColumnTable("/Root/OlapStore", R"( | ||
| Name: "Test" | ||
| ColumnShardCount : 4 |
There was a problem hiding this comment.
Без явного указания запрос падал с ошибкой -
2025-03-07T12:05:20.003260Z node 1 :TX_PROXY ERROR: Actor# [1:7479037519220164375:2896] txid# 281474976710659, issues: { message: "Cannot create table with 64 column shards, only 4 are available" severity: 1 }
Error 128: Cannot create table with 64 column shards, only 4 are available
|
⚪ Test history | Ya make output | Test bloat
⚪
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ Test history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*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 |
| @@ -176,6 +177,10 @@ bool TOlapColumnsDescription::ValidateForStore(const NKikimrSchemeOp::TColumnTab | |||
| return false; | |||
| } | |||
| const TString& colName = colProto.GetName(); | |||
| if (!IsValidColumnName(colName, false)) { | |||
There was a problem hiding this comment.
Зачем эта валидация? Кажется, она здесь не обязательна, потому что все колонки из tableSchema должны присутствовать в схеме стора (следующая проверка), а их имена валидируются в TOlapColumnBase::ParseFromRequest.
Я обратил внимание, потому что одинаковая валидация описана в двух местах, что потом может привести к ошибкам (например, кто-то добавит другую валидацию только в одном месте и из-за этого не покроет часть сценариев).
There was a problem hiding this comment.
Тесты схемных операций можно писать без KQP слоя, как в schemeshard/ut_olap/.... Лучше написать такой тест, чтобы, когда он падал, было очевидно, что проблема в схемшарде.
Emgariko
changed the title
Changelog entry
Unifying column names validation for both row and column tables #14722
Changelog category
Description for reviewers
Added name validation call used in row tables to column tables(link). Covered with tests - testing Create/Alter TABLE/ColumnStore behavior.