-
Notifications
You must be signed in to change notification settings - Fork 740
Unique index validation stage #20133
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
Unique index validation stage #20133
Conversation
|
⚪ 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 |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ DetailsTest 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 |
|
🟢 |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ |
|
⚪ 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 |
b2e1d86 to
ce1b363
Compare
|
⚪ DetailsTest 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 |
|
⚪ 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 |
snaury
left a comment
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.
По даташардам от меня окей (изменение на вид минимально инвазивное). Однако учитывайте, что дизайн уникальных индексов, которые не уникальные на уровне реализации в даташарде / локальной базе может быть ошибкой с точки зрения эффективности. Дело в том, что тогда любой point read в таком индексе на самом деле выливается в range read, который дороже в реализации. Но кажется я это уже говорил на каком-то из прошлых обсуждений.
vitalif
left a comment
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.
Ну в общем я за то, чтобы ПР разделить, а статусы вторые убрать :-)
ce1b363 to
362c4cc
Compare
|
⚪ DetailsTest 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 |
|
⚪ 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
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ DetailsTest 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 |
| Y_ENSURE(found); | ||
| } | ||
|
|
||
| // Arrange shards in ascending order by shard index |
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.
а чего PerformCrossShardUniqIndexValidation сам внутри не посортирует как ему надо
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.
и это не by shard index, а по ключу видимо
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.
и что-то не пойму где оно сортируется вообще сейчас
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.
Я спрашивал у Ильназа и он сказал мне, что нужный мне порядок (по ключу) содержится в table->GetPartitions(). Сортировать отдельно смысла нет. Написал коммент, что "in ascending order by shard index"
|
⚪ 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 |
I refactored according to the request
(cherry picked from commit 6771cfb)
(cherry picked from commit 6771cfb)
Changelog entry
...
Changelog category
Description for reviewers
In this PR I add a special stage to index build process: validation of built index. This is used only as a unique index build stage.
Also here is the fix of the bug: in KQP index build case we don't fail query if index build process fails.