-
Notifications
You must be signed in to change notification settings - Fork 738
Support snapshot isolation mode in ydb cli #29537
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
Conversation
|
Hi! Thank you for contributing! |
|
🟢 |
|
⚪
🟢 |
|
⚪
🟢 |
…ecute commands Co-authored-by: asmyasnikov <14202262+asmyasnikov@users.noreply.github.com>
|
…x' and 'notx', add changelog entry Co-authored-by: pnv1 <2571062+pnv1@users.noreply.github.com>
Addressed in 524f543. Changed help text to use |
|
⚪ ⚪ 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 |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
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
This pull request adds support for snapshot isolation transaction modes (snapshot-ro and snapshot-rw) to the YDB CLI commands. It extends the --tx-mode option to the ydb sql command (previously only available in ydb table query execute) and adds the new snapshot isolation modes to both commands.
- Added
--tx-modeoption toydb sqlcommand with default valueno-tx - Extended both
ydb sqlandydb table query executecommands withsnapshot-roandsnapshot-rwtransaction modes - Maintained backward compatibility by supporting both
no-txandnotx(deprecated) values
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ydb/public/lib/ydb_cli/commands/ydb_sql.h | Added TxMode field to store transaction mode configuration |
| ydb/public/lib/ydb_cli/commands/ydb_sql.cpp | Implemented --tx-mode option with transaction control logic supporting all modes including snapshot isolation |
| ydb/public/lib/ydb_cli/commands/ydb_service_table.cpp | Added snapshot isolation modes to both data and generic query execution, updated help text |
| ydb/apps/ydb/CHANGELOG.md | Documented the new features in the changelog |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else if (TxMode == "snapshot-ro") { | ||
| txSettings = NTable::TTxSettings::SnapshotRO(); | ||
| } else if (TxMode == "snapshot-rw") { | ||
| txSettings = NTable::TTxSettings::SnapshotRW(); |
Copilot
AI
Nov 25, 2025
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.
The ExecuteDataQuery function doesn't handle "no-tx" or "notx" transaction modes, even though the help text at line 370 mentions "no-tx (generic queries only)". While this is consistent with the help text's indication that no-tx is only for generic queries, the function should still validate that the user hasn't provided these invalid modes for data queries. Consider adding a check:
} else if (TxMode == "no-tx" || TxMode == "notx") {
throw TMisuseException() << "Transaction mode 'no-tx' is only supported for generic queries.";This would provide a more helpful error message than "Unknown transaction mode."
| txSettings = NTable::TTxSettings::SnapshotRW(); | |
| txSettings = NTable::TTxSettings::SnapshotRW(); | |
| } else if (TxMode == "no-tx" || TxMode == "notx") { | |
| throw TMisuseException() << "Transaction mode 'no-tx' is only supported for generic queries."; |
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.
ExecuteDataQuery does not support NoTx tx-control
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.
no-tx не должно быть в ydb table query execute
|
Docs issue: #29605 |
Changelog entry
Added
--tx-modeoption toydb sqlcommand with support for all transaction modes including snapshot isolation (snapshot-ro,snapshot-rw). Also added snapshot isolation modes toydb table query execute.Changelog category
Description for reviewers
Adds
--tx-modesupport toydb sqlcommand (previously only available inydb table query execute) and extends both commands with snapshot isolation modes.Supported modes:
serializable-rwonline-rostale-rosnapshot-ro— new snapshot read-only isolationsnapshot-rw— new snapshot read-write isolationno-tx— no transaction (default forydb sql, also acceptsnotxfor backward compatibility)Changes:
ydb_sql.h/cpp: Added--tx-modeoption with defaultno-txand transaction control logicydb_service_table.cpp: Addedsnapshot-ro/snapshot-rwtoExecuteDataQuery,StreamExecuteQuery, and help textydb/apps/ydb/CHANGELOG.md: Added changelog entryUsage:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.