Index impl table is not private #18159
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the handling of direct access to index implementation tables by changing their accessibility from public to restricted. Key changes include a switch in return values in the scheme cache, updated unit tests to reflect altered index behaviors (including vector index handling), and propagation of an internal call flag across multiple components.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ydb/core/tx/scheme_board/cache.cpp | Changed return value for specific index impl table types to restrict access. |
| ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp | Updated unit test “AlterIndexImplTable” to handle both vector and secondary index scenarios. |
| ydb/core/kqp/ut/indexes/kqp_indexes_ut.cpp | Modified test expectation for index creation to now assert a successful outcome. |
| ydb/core/kqp/session_actor/kqp_session_actor.cpp | Passed through a new IsInternalCall flag in the request. |
| ydb/core/kqp/gateway/kqp_gateway.h | Added an explicit IsInternalCall field in gateway requests. |
| ydb/core/kqp/executer_actor/kqp_data_executer.cpp | Refactored index table validation by extracting error handling into CheckDatashardIndexTables. |
| … | Other test and UT changes updated accordingly. |
Comments suppressed due to low confidence (2)
ydb/core/kqp/ut/indexes/kqp_indexes_ut.cpp:1615
- [nitpick] The test now expects a successful result (with a specific formatted outcome) rather than a scheme error as before; verify that this revised expectation fully reflects the new design requirements for index creation.
UNIT_ASSERT_C(result.IsSuccess(), result.GetIssues().ToString());
ydb/core/tx/scheme_board/cache.cpp:896
- Changing the return value from true to false for these index implementation table types alters access control behavior; ensure that this change is fully aligned with the intended design to restrict direct access.
return false;
|
🔴 The changelog entry is less than 20 characters or missing. |
|
⚪ 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
⚪ 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
⚪ 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
⚪ 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
⚪ 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
⚪ 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 |
azevaykin
added
the
rebase-and-check
github-actions
bot
removed
the
rebase-and-check
|
⚪ 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
⚪ 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 |
azevaykin
added
the
rebase-and-check
github-actions
bot
removed
the
rebase-and-check
|
⚪ 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 |
|
⚪ 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 |
ijon
left a comment
ijon
left a comment
There was a problem hiding this comment.
Cautious approval on condition of completing necessary next steps
|
⚪ 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 |
|
⚪ 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 |
5 participants
Changelog entry
Direct access to index impl tables .
Change the settings of index impl tables tables via the ALTER TABLE.
For the vector index
For example:
ALTER TABLE table/index/indexImplLevelTable.For the secondary index
Leave the
ALTER TABLE table ALTER INDEXindex for backward compatibility.At the same time, add in a new format:
ALTER TABLE
table/index/indexImplTableDQL & DML
Select, Update, Delete operations via YQL are allowed only for cluster admins.
Changelog category
Description for reviewers
In following PRs we should:
ShowPrivatePathis usedShow create tableshould support changes in index impl tables #18347