-
Notifications
You must be signed in to change notification settings - Fork 735
Handle new prefixes in prefixed vector index update #25505
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
Handle new prefixes in prefixed vector index update #25505
Conversation
|
🟢 |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
| fullPrefixColumns.push_back(NTableIndex::NKMeans::IdColumn); | ||
| auto fullPrefixColumnList = BuildColumnsList(fullPrefixColumns, pos, ctx); | ||
|
|
||
| auto cnSequencer = Build<TKqpCnSequencer>(ctx, pos) |
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.
что будет при обновлении индекса созданного ранее (без sequencer?)
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.
посмотри в kqp_opt_phy_insert_index и kqp_opt_phy_upsert_index - там смотрится, если у колонки __ydb_id в префиксной таблице нет DEFAULT_FROM_SEQUENCE, то используется старый алгоритм, который игнорит неизвестные префиксы. точнее я это только что ещё поправил т.к. оказалось что LookupMode=Join в StreamLookup то же самое что SemiJoin т.е. сам он строки с null не фильтрует. я добавил фильтр)
в тесты добавить это было бы интересно, но это видимо в тесты на совместимость как раз т.к. сейчас создать так индексную таблицу не получится по идее.
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.
конкретно этот сценарий (и другие про обновление во время построения) чуть проще чем на совместимость, и можно прямо юнит-тестом написать через BlockEvents
а так да, надо бы
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.
Тут не делал, т.к. сходу пока хз, как такое написать)
ydb/core/tx/schemeshard/schemeshard__operation_create_build_index.cpp
Outdated
Show resolved
Hide resolved
ydb/core/tx/schemeshard/schemeshard__operation_drop_sequence.cpp
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
ydb/core/tx/schemeshard/schemeshard__operation_create_sequence.cpp
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
f3b876b to
58ae4a2
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
58ae4a2 to
3d7b628
Compare
|
Всё, #26046 смержился, тут осталось только KQP |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ 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 |
Changelog category
Description for reviewers
Для поддержки вставки новых префиксов:
Таким образом, даже при вставке новых префиксов в индекс таблица level всегда остаётся по-настоящему иммутабельной.