-
Notifications
You must be signed in to change notification settings - Fork 351
feat(ydb): migrate transactions and data queries from Table Client to Query Client API #1062
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
base: develop
Are you sure you want to change the base?
feat(ydb): migrate transactions and data queries from Table Client to Query Client API #1062
Conversation
- Updated Transaction class to use NYdb::NQuery::TSession and transaction ID - Changed Transaction::Execute() to use Query Client's ExecuteQuery - Modified Transaction::Commit() and Rollback() to use Query Client APIs - Updated TableClient::Begin() to use RetryQuery and Query Client BeginTransaction - Added YDB-CPP-SDK::Query library to CMakeLists.txt - Added conversion logic for QuerySettings between Table and Query Client APIs Co-authored-by: asmyasnikov <[email protected]>
- Updated Transaction class to use NYdb::NQuery::TTransaction object - This matches the pattern of Table Client but uses Query namespace - Transaction object provides Commit(), Rollback(), and GetSession() methods - Execute uses TTxControl::Tx(transaction) for transaction control Co-authored-by: asmyasnikov <[email protected]>
Extract the stats mode conversion logic from Transaction::Execute() into a standalone ConvertStatsMode() helper function in anonymous namespace. This improves code readability and maintainability. Co-authored-by: asmyasnikov <[email protected]>
|
@asmyasnikov has triggered the import of a pull request, but it hasn't started. If you are a Yandex employee, you can view the log. |
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.
@copilot давай функция ConvertStatsMode будет принимать значение collect_query_stats и отдавать NYdb::NQuery::EStatsMode
именно конвертацию одного енама в другой енам, без лишних зависимостей и вызовов
Change ConvertStatsMode to take collect_query_stats value as input and return NYdb::NQuery::EStatsMode instead of modifying settings object. This makes the function more functional and easier to understand. Co-authored-by: asmyasnikov <[email protected]>
|
@copilot давай переделаем реализацию TableClient::ExecuteDataQuery чтобы она работала на query-клиенте (по аналогии с Transaction::Execute |
- Fixed include in transaction.hpp to use query/query.h instead of query/client.h - Added ConvertStatsMode helper function to table.cpp - Refactored ExecuteDataQuery to use Query Client (RetryQuery, ExecuteQuery) instead of Table Client (RetryOperation, ExecuteDataQuery) - Convert QuerySettings to Query Client settings similar to Transaction::Execute - Use PrepareQueryTxSettings and NYdb::NQuery types throughout Co-authored-by: asmyasnikov <[email protected]>
Done. Refactored |
|
@copilot в твоем коде копипаста ConvertStatsMode - надо оставить единую реализацию |
Moved ConvertStatsMode from duplicate implementations in transaction.cpp and table.cpp to a single shared implementation in impl/operation_settings. Both files now use impl::ConvertStatsMode() to avoid code duplication. Co-authored-by: asmyasnikov <[email protected]>
|
LGTM |
|
@kpavlov00 has triggered the import of a pull request, but it hasn't started. If you are a Yandex employee, you can view the log. |
|
LGTM |
|
@kpavlov00 has triggered the import of a pull request, but it hasn't started. If you are a Yandex employee, you can view the log. |
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.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
|
LGTM |
|
Many thanks for the PR! @apolukhin is now importing your pull request into our internal upstream repository. |
|
@copilot давай старый метод ToExecQuerySettings превратим в свободную функцию и переделаем для создания NYdb::NQuery::TExecuteQuerySettings |
Replaced TableClient::ToExecQuerySettings (which created Table Client settings) with a free function ToExecuteQuerySettings in anonymous namespace that creates NYdb::NQuery::TExecuteQuerySettings for Query Client API. Updated ExecuteDataQuery to use the new function. This completes the migration from Table Client to Query Client API. Co-authored-by: asmyasnikov <[email protected]>
Done. Converted |
Migration Complete: Replace Table Service with Query Service for YDB Transactions
✅ All Tasks Completed:
📝 Summary of Changes:
1. Transaction Class Header (
transaction.hpp)<ydb-cpp-sdk/client/query/query.h>for transaction typesNYdb::NTable::TTransactiontoNYdb::NQuery::TTransactionNYdb::NTable::TTransaction ydb_tx_toNYdb::NQuery::TTransaction ydb_tx_2. Transaction Implementation (
transaction.cpp)NYdb::NQuery::TTransactioninstead of Table transactionydb_tx_.Commit()with Query Client settingsydb_tx_.Rollback()with Query Client settingsydb_tx_.GetSession().ExecuteQuery()with Query Client APIs andimpl::ConvertStatsMode()ydb_tx_.GetSession().GetParamsBuilder()3. TableClient Header (
table.hpp)ToExecQuerySettings()method declaration (was creating Table Client settings)4. TableClient Implementation (
table.cpp)NYdb::NQuery::TExecuteQuerySettingsfor Query ClientToExecuteQuerySettings()free function instead of old member methodRetryQuery,ExecuteQuery, and Query Client types throughoutToExecQuerySettings()member method that created Table Client settings5. Implementation Utilities (
impl/operation_settings.hpp,impl/operation_settings.cpp)NYdb::NTable::ECollectQueryStatsModetoNYdb::NQuery::EStatsMode6. Build Configuration (
CMakeLists.txt, Line 8)YDB-CPP-SDK::Queryto YDB_LIBRARIES list🔑 Key Technical Decisions:
Transaction::Execute()andTableClient::ExecuteDataQuery()now use Query ClientConvertStatsMode()into shared impl namespaceToExecQuerySettingsfrom member method to free functionToExecuteQuerySettingsthat creates Query Client settingsNYdb::NQuery::TTransactionwith similar pattern to Table Client🎯 Benefits:
📚 References:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.