feat(ydb): migrate transactions and data queries from Table Client to Query Client API#1062
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 <14202262+asmyasnikov@users.noreply.github.com>
- 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 <14202262+asmyasnikov@users.noreply.github.com>
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 <14202262+asmyasnikov@users.noreply.github.com>
|
@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.
@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 <14202262+asmyasnikov@users.noreply.github.com>
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 <14202262+asmyasnikov@users.noreply.github.com>
|
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.
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 <175728472+Copilot@users.noreply.github.com>
|
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 <14202262+asmyasnikov@users.noreply.github.com>
Done. Converted |
|
@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. |
|
@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. |
|
✅ This pull request is being closed because it has been successfully merged into our internal monorepository. |
…nt to Query Client API ## Replace Table Service with Query Service for YDB Transactions ### 📝 Summary of Changes: #### 1. Transaction Class Header (`transaction.hpp`) - **Line 5**: Changed include to `<ydb-cpp-sdk/client/query/query.h>` for transaction types - **Line 74**: Changed constructor parameter from `NYdb::NTable::TTransaction` to `NYdb::NQuery::TTransaction` - **Line 90**: Changed member variable from `NYdb::NTable::TTransaction ydb_tx_` to `NYdb::NQuery::TTransaction ydb_tx_` #### 2. Transaction Implementation (`transaction.cpp`) - **Constructor (Lines 22-37)**: Updated to accept `NYdb::NQuery::TTransaction` instead of Table transaction - **Commit() (Lines 70-101)**: Uses `ydb_tx_.Commit()` with Query Client settings - **Rollback() (Lines 103-122)**: Uses `ydb_tx_.Rollback()` with Query Client settings - **Execute() (Lines 138-176)**: Uses `ydb_tx_.GetSession().ExecuteQuery()` with Query Client APIs and `impl::ConvertStatsMode()` - **GetBuilder() (Lines 124-126)**: Uses `ydb_tx_.GetSession().GetParamsBuilder()` #### 3. TableClient Implementation (`table.cpp`) - **Begin() (Lines 297-314)**: Changed from Table Client to Query Client (RetryQuery, PrepareQueryTxSettings, BeginTransaction) - **ExecuteDataQuery() (Lines 352-384)**: Migrated to use Query Client: - Uses `RetryQuery` instead of `RetryOperation` - Converts `QuerySettings` to `NYdb::NQuery::TExecuteQuerySettings` using `impl::ConvertStatsMode()` - Uses `PrepareQueryTxSettings` instead of `PrepareTxSettings` - Uses `NYdb::NQuery::TSession`, `TTxControl`, and `ExecuteQuery` #### 4. Implementation Utilities (`impl/operation_settings.hpp`, `impl/operation_settings.cpp`) - **ConvertStatsMode()**: Single shared implementation that converts `NYdb::NTable::ECollectQueryStatsMode` to `NYdb::NQuery::EStatsMode` - Eliminates code duplication between transaction.cpp and table.cpp #### 5. Build Configuration (`CMakeLists.txt`, Line 8) - Added `YDB-CPP-SDK::Query` to YDB\_LIBRARIES list ### 🔑 Key Technical Decisions: 1. **Complete Migration**: Both `Transaction::Execute()` and `TableClient::ExecuteDataQuery()` now use Query Client 2. **No Code Duplication**: Consolidated `ConvertStatsMode()` into shared impl namespace 3. **Transaction Object Model**: Query Client provides `NYdb::NQuery::TTransaction` with similar pattern to Table Client 4. **API Conversion**: Consistent pattern for converting QuerySettings across both implementations 5. **Backward Compatibility**: All public APIs remain unchanged - only internal implementation uses Query Client ### 🎯 Benefits: - Uses modern Query Client API throughout instead of deprecated Table Client - Maintains full backward compatibility with existing code - Follows YDB SDK recommended migration path - No code duplication - single ConvertStatsMode implementation - Consistent implementation pattern across Transaction and TableClient - No changes required to existing code using these APIs ### 📚 References: - Issue: #1061 - YDB Go SDK example: <https://github.com/ydb-platform/ydb-go-sdk/blob/70fd60ba1490c984162574958dd23344cb513f6e/internal/table/session.go#L131-L165> - YDB Java SDK example: <https://github.com/ydb-platform/ydb-java-sdk/blob/3fd06ddcf36ab918eb6a394ee3bbebac6caf15b2/query/src/main/java/tech/ydb/query/impl/TableClientImpl.java#L106-L160> --- Pull Request resolved: <#1062> Co-authored-by: kpavlov00 <kpavlov00@yandex-team.com> Co-authored-by: kpavlov00 <kpavlov00@yandex-team.com> Co-authored-by: kpavlov00 <kpavlov00@yandex-team.com> Co-authored-by: kpavlov00 <kpavlov00@yandex-team.com> Co-authored-by: kpavlov00 <kpavlov00@yandex-team.com> Co-authored-by: kpavlov00 <kpavlov00@yandex-team.com> Co-authored-by: kpavlov00 <kpavlov00@yandex-team.com> Co-authored-by: kpavlov00 <kpavlov00@yandex-team.com> Co-authored-by: kpavlov00 <kpavlov00@yandex-team.com> Co-authored-by: Aleksey Myasnikov \ <asmyasnikov@ydb.tech\> commit_hash:da54645480a88513a7d0f37885598f40a342bbd5
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.