YT-22930: Make ModifyRows more strongly typed (part 2, remove old api)

* Changelog entry
Type: feature
Component: dynamic-tables

Change NYT::NApi::IDynamicTableTransaction::ModifyRows api: TRowModification can represent less invalid states.
The change is breaking: use of old-style ModifyRows will not compile, however migration is purely mechanical and direct use of ModifyRows should be rare.
commit_hash:c62df38ba1c02a2c48cdb1eeb8f39cfb816cf2cb

10 files changed, 34 insertions, 135 deletions

diff --git a/yt/yt/client/api/delegating_transaction.cpp b/yt/yt/client/api/delegating_transaction.cpp
index 06c9c6c8ab8..06f5e3d68e9 100644
--- a/yt/yt/client/api/delegating_transaction.cpp
+++ b/yt/yt/client/api/delegating_transaction.cpp
@@ -275,13 +275,6 @@ DELEGATE_METHOD(void, ModifyRows, (
     const TModifyRowsOptions& options),
     (path, nameTable, modifications, options))
 
-DELEGATE_METHOD(void, FutureModifyRows, (
-    const NYPath::TYPath& path,
-    NTableClient::TNameTablePtr nameTable,
-    TSharedRange<NFuture::TRowModification> modifications,
-    const TModifyRowsOptions& options),
-    (path, nameTable, modifications, options))
-
 DELEGATE_METHOD(void, AdvanceConsumer, (
     const NYPath::TRichYPath& consumerPath,
     const NYPath::TRichYPath& queuePath,
diff --git a/yt/yt/client/api/delegating_transaction.h b/yt/yt/client/api/delegating_transaction.h
index 27c70a89050..0c66fa034c0 100644
--- a/yt/yt/client/api/delegating_transaction.h
+++ b/yt/yt/client/api/delegating_transaction.h
@@ -223,12 +223,6 @@ public:
         TSharedRange<TRowModification> modifications,
         const TModifyRowsOptions& options) override;
 
-    void FutureModifyRows(
-        const NYPath::TYPath& path,
-        NTableClient::TNameTablePtr nameTable,
-        TSharedRange<NFuture::TRowModification> modifications,
-        const TModifyRowsOptions& options) override;
-
     // Queues
     void AdvanceConsumer(
         const NYPath::TRichYPath& consumerPath,
diff --git a/yt/yt/client/api/dynamic_table_transaction.h b/yt/yt/client/api/dynamic_table_transaction.h
index f0e76dbcf5f..6071bb6c3a1 100644
--- a/yt/yt/client/api/dynamic_table_transaction.h
+++ b/yt/yt/client/api/dynamic_table_transaction.h
@@ -19,10 +19,6 @@ namespace NYT::NApi {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-namespace NFuture {
-
-////////////////////////////////////////////////////////////////////////////////
-
 namespace NRowModifications {
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -80,21 +76,6 @@ using TRowModification = std::variant<
 
 ////////////////////////////////////////////////////////////////////////////////
 
-} // namespace NFuture
-
-////////////////////////////////////////////////////////////////////////////////
-
-//! Either a write or delete.
-struct TRowModification
-{
-    //! Discriminates between writes and deletes.
-    ERowModificationType Type;
-    //! Either a row (for write; versioned or unversioned) or a key (for delete; always unversioned).
-    NTableClient::TTypeErasedRow Row;
-    //! Locks.
-    NTableClient::TLockMask Locks;
-};
-
 struct TModifyRowsOptions
 {
     //! If this happens to be a modification of a replicated table,
@@ -182,12 +163,6 @@ struct IDynamicTableTransaction
         NTableClient::TNameTablePtr nameTable,
         TSharedRange<TRowModification> modifications,
         const TModifyRowsOptions& options = {}) = 0;
-
-    virtual void FutureModifyRows(
-        const NYPath::TYPath& path,
-        NTableClient::TNameTablePtr nameTable,
-        TSharedRange<NFuture::TRowModification> modifications,
-        const TModifyRowsOptions& options = {}) = 0;
 };
 
 DEFINE_REFCOUNTED_TYPE(IDynamicTableTransaction)
diff --git a/yt/yt/client/api/dynamic_table_transaction_mixin.cpp b/yt/yt/client/api/dynamic_table_transaction_mixin.cpp
index a68c6bb2576..b47e012a6dc 100644
--- a/yt/yt/client/api/dynamic_table_transaction_mixin.cpp
+++ b/yt/yt/client/api/dynamic_table_transaction_mixin.cpp
@@ -23,13 +23,13 @@ void TDynamicTableTransactionMixin::WriteRows(
     THROW_ERROR_EXCEPTION_UNLESS(IsWriteLock(lockType), "Inappropriate lock type %Qlv given for write modification",
         lockType);
 
-    std::vector<NFuture::TRowModification> modifications;
+    std::vector<TRowModification> modifications;
     modifications.reserve(rows.Size());
 
     switch (lockType) {
         case ELockType::Exclusive: {
             for (auto row : rows) {
-                modifications.push_back(NFuture::NRowModifications::TWriteRow(row));
+                modifications.push_back(NRowModifications::TWriteRow(row));
             }
 
             break;
@@ -57,7 +57,7 @@ void TDynamicTableTransactionMixin::WriteRows(
                     }
                 }
 
-                modifications.push_back(NFuture::NRowModifications::TWriteAndLockRow(row, lockMask));
+                modifications.push_back(NRowModifications::TWriteAndLockRow(row, lockMask));
             }
 
             break;
@@ -67,7 +67,7 @@ void TDynamicTableTransactionMixin::WriteRows(
             YT_ABORT();
     }
 
-    FutureModifyRows(
+    ModifyRows(
         path,
         std::move(nameTable),
         MakeSharedRange(std::move(modifications), std::move(rows.ReleaseHolder())),
@@ -80,14 +80,14 @@ void TDynamicTableTransactionMixin::WriteRows(
     TSharedRange<TVersionedRow> rows,
     const TModifyRowsOptions& options)
 {
-    std::vector<NFuture::TRowModification> modifications;
+    std::vector<TRowModification> modifications;
     modifications.reserve(rows.Size());
 
     for (auto row : rows) {
-        modifications.push_back(NFuture::NRowModifications::TVersionedWriteRow(row.ToTypeErasedRow()));
+        modifications.push_back(NRowModifications::TVersionedWriteRow(row.ToTypeErasedRow()));
     }
 
-    FutureModifyRows(
+    ModifyRows(
         path,
         std::move(nameTable),
         MakeSharedRange(std::move(modifications), std::move(rows.ReleaseHolder())),
@@ -100,13 +100,13 @@ void TDynamicTableTransactionMixin::DeleteRows(
     TSharedRange<TLegacyKey> keys,
     const TModifyRowsOptions& options)
 {
-    std::vector<NFuture::TRowModification> modifications;
+    std::vector<TRowModification> modifications;
     modifications.reserve(keys.Size());
     for (auto key : keys) {
-        modifications.push_back(NFuture::NRowModifications::TDeleteRow(key));
+        modifications.push_back(NRowModifications::TDeleteRow(key));
     }
 
-    FutureModifyRows(
+    ModifyRows(
         path,
         std::move(nameTable),
         MakeSharedRange(std::move(modifications), std::move(keys.ReleaseHolder())),
@@ -119,14 +119,14 @@ void TDynamicTableTransactionMixin::LockRows(
     TSharedRange<TLegacyKey> keys,
     TLockMask lockMask)
 {
-    std::vector<NFuture::TRowModification> modifications;
+    std::vector<TRowModification> modifications;
     modifications.reserve(keys.Size());
 
     for (auto key : keys) {
-        modifications.push_back(NFuture::NRowModifications::TWriteAndLockRow(key, lockMask));
+        modifications.push_back(NRowModifications::TWriteAndLockRow(key, lockMask));
     }
 
-    FutureModifyRows(
+    ModifyRows(
         path,
         std::move(nameTable),
         MakeSharedRange(std::move(modifications), std::move(keys)),
@@ -164,49 +164,6 @@ void TDynamicTableTransactionMixin::LockRows(
     LockRows(path, nameTable, keys, lockMask);
 }
 
-void TDynamicTableTransactionMixin::ModifyRows(
-    const NYPath::TYPath& path,
-    TNameTablePtr nameTable,
-    TSharedRange<TRowModification> modifications,
-    const TModifyRowsOptions& options)
-{
-    std::vector<NFuture::TRowModification> newModifications;
-    newModifications.reserve(modifications.Size());
-
-    for (const auto& modification : modifications) {
-        THROW_ERROR_EXCEPTION_IF(modification.Type != ERowModificationType::WriteAndLock && !modification.Locks.IsNone(),
-            "Cannot perform lock by %Qlv modification type, use %Qlv",
-            modification.Type,
-            ERowModificationType::WriteAndLock);
-
-        switch (modification.Type) {
-            case ERowModificationType::Write:
-                newModifications.push_back(NFuture::NRowModifications::TWriteRow(TUnversionedRow(modification.Row)));
-                break;
-
-            case ERowModificationType::Delete:
-                newModifications.push_back(NFuture::NRowModifications::TDeleteRow(TLegacyKey(modification.Row)));
-                break;
-
-            case ERowModificationType::VersionedWrite:
-                newModifications.push_back(NFuture::NRowModifications::TVersionedWriteRow(modification.Row));
-                break;
-
-            case ERowModificationType::WriteAndLock:
-                newModifications.push_back(NFuture::NRowModifications::TWriteAndLockRow(
-                    TUnversionedRow(modification.Row),
-                    modification.Locks));
-                break;
-        }
-    }
-
-    FutureModifyRows(
-        path,
-        std::move(nameTable),
-        MakeSharedRange(std::move(newModifications), std::move(modifications.ReleaseHolder())),
-        options);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 
 } // namespace NYT::NApi
diff --git a/yt/yt/client/api/dynamic_table_transaction_mixin.h b/yt/yt/client/api/dynamic_table_transaction_mixin.h
index ce72a916757..ab94daaa77d 100644
--- a/yt/yt/client/api/dynamic_table_transaction_mixin.h
+++ b/yt/yt/client/api/dynamic_table_transaction_mixin.h
@@ -45,12 +45,6 @@ public:
         TSharedRange<NTableClient::TLegacyKey> keys,
         const std::vector<std::string>& locks,
         NTableClient::ELockType lockType = NTableClient::ELockType::SharedStrong) override;
-
-    void ModifyRows(
-        const NYPath::TYPath& path,
-        NTableClient::TNameTablePtr nameTable,
-        TSharedRange<TRowModification> modifications,
-        const TModifyRowsOptions& options = {}) override;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/yt/yt/client/api/persistent_queue.cpp b/yt/yt/client/api/persistent_queue.cpp
index 6ef35c704fc..b91750bbaac 100644
--- a/yt/yt/client/api/persistent_queue.cpp
+++ b/yt/yt/client/api/persistent_queue.cpp
@@ -1021,11 +1021,7 @@ TFuture<void> UpdatePersistentQueueTabletsState(
                 auto rowToDelete = rowBuffer->AllocateUnversioned(2);
                 rowToDelete[0] = MakeUnversionedInt64Value(tabletIndex, nameTableTabletIndexColumnId);
                 rowToDelete[1] = MakeUnversionedInt64Value(rowIndex, nameTableRowIndexColumnId);
-                modifications.push_back(TRowModification{
-                    ERowModificationType::Delete,
-                    rowToDelete.ToTypeErasedRow(),
-                    TLockMask()
-                });
+                modifications.push_back(NRowModifications::TDeleteRow(rowToDelete));
             }
 
             for (const auto& [tabletIndex, tabletUpdate] : tabletMap) {
@@ -1035,11 +1031,7 @@ TFuture<void> UpdatePersistentQueueTabletsState(
                     rowToWrite[0] = MakeUnversionedInt64Value(tabletIndex, nameTableTabletIndexColumnId);
                     rowToWrite[1] = MakeUnversionedInt64Value(tabletUpdate.FirstUnconsumedRowIndex - 1, nameTableRowIndexColumnId);
                     rowToWrite[2] = MakeUnversionedInt64Value(static_cast<i64>(ERowState::ConsumedAndTrimmed), nameTableStateColumnId);
-                    modifications.push_back(TRowModification{
-                        ERowModificationType::Write,
-                        rowToWrite.ToTypeErasedRow(),
-                        TLockMask()
-                    });
+                    modifications.push_back(NRowModifications::TWriteRow(rowToWrite));
                 }
             }
 
diff --git a/yt/yt/client/api/rpc_proxy/transaction_impl.cpp b/yt/yt/client/api/rpc_proxy/transaction_impl.cpp
index a7717daed09..f6465fd6fcc 100644
--- a/yt/yt/client/api/rpc_proxy/transaction_impl.cpp
+++ b/yt/yt/client/api/rpc_proxy/transaction_impl.cpp
@@ -387,10 +387,10 @@ TFuture<void> TTransaction::Abort(const TTransactionAbortOptions& options)
     return DoAbort(&guard, options);
 }
 
-void TTransaction::FutureModifyRows(
+void TTransaction::ModifyRows(
     const TYPath& path,
     TNameTablePtr nameTable,
-    TSharedRange<NFuture::TRowModification> modifications,
+    TSharedRange<TRowModification> modifications,
     const TModifyRowsOptions& options)
 {
     ValidateTabletTransactionId(GetId());
@@ -398,10 +398,10 @@ void TTransaction::FutureModifyRows(
     for (const auto& modification : modifications) {
         // TODO(sandello): handle versioned rows
         Visit(modification,
-            [] (const NFuture::NRowModifications::TWriteRow&) { },
-            [] (const NFuture::NRowModifications::TDeleteRow&) { },
-            [] (const NFuture::NRowModifications::TWriteAndLockRow&) { },
-            [] (const NFuture::NRowModifications::TVersionedWriteRow&) {
+            [] (const NRowModifications::TWriteRow&) { },
+            [] (const NRowModifications::TDeleteRow&) { },
+            [] (const NRowModifications::TWriteAndLockRow&) { },
+            [] (const NRowModifications::TVersionedWriteRow&) {
                 YT_ABORT();
             });
     }
@@ -426,11 +426,11 @@ void TTransaction::FutureModifyRows(
     bool usedStrongLocks = false;
     bool usedWideLocks = false;
     for (const auto& modification : modifications) {
-        if (!std::holds_alternative<NFuture::NRowModifications::TWriteAndLockRow>(modification)) {
+        if (!std::holds_alternative<NRowModifications::TWriteAndLockRow>(modification)) {
             continue;
         }
 
-        auto mask = std::get<NFuture::NRowModifications::TWriteAndLockRow>(modification).Locks;
+        auto mask = std::get<NRowModifications::TWriteAndLockRow>(modification).Locks;
         usedWideLocks |= mask.GetSize() > TLegacyLockMask::MaxCount;
         if (usedWideLocks) {
             break;
@@ -477,22 +477,22 @@ void TTransaction::FutureModifyRows(
 
     for (const auto& modification : modifications) {
         Visit(modification,
-            [&req, &rows, &fillCorrespondingLock] (const NFuture::NRowModifications::TWriteRow& modification) {
+            [&req, &rows, &fillCorrespondingLock] (const NRowModifications::TWriteRow& modification) {
                 rows.emplace_back(modification.Row);
                 req->add_row_modification_types(NProto::ERowModificationType::RMT_WRITE);
                 fillCorrespondingLock(TLockMask{});
             },
-            [&req, &rows, &fillCorrespondingLock] (const NFuture::NRowModifications::TDeleteRow& modification) {
+            [&req, &rows, &fillCorrespondingLock] (const NRowModifications::TDeleteRow& modification) {
                 rows.emplace_back(modification.Key);
                 req->add_row_modification_types(NProto::ERowModificationType::RMT_DELETE);
                 fillCorrespondingLock(TLockMask{});
             },
-            [&req, &rows, &fillCorrespondingLock] (const NFuture::NRowModifications::TWriteAndLockRow& writeAndLockModification) {
+            [&req, &rows, &fillCorrespondingLock] (const NRowModifications::TWriteAndLockRow& writeAndLockModification) {
                 rows.emplace_back(writeAndLockModification.Row);
                 req->add_row_modification_types(NProto::ERowModificationType::RMT_MODIFY);
                 fillCorrespondingLock(writeAndLockModification.Locks);
             },
-            [] (const NFuture::NRowModifications::TVersionedWriteRow&) {
+            [] (const NRowModifications::TVersionedWriteRow&) {
                 // NB: Checked above.
                 YT_ABORT();
             });
diff --git a/yt/yt/client/api/rpc_proxy/transaction_impl.h b/yt/yt/client/api/rpc_proxy/transaction_impl.h
index 4134079de94..ea39092b39e 100644
--- a/yt/yt/client/api/rpc_proxy/transaction_impl.h
+++ b/yt/yt/client/api/rpc_proxy/transaction_impl.h
@@ -75,10 +75,10 @@ public:
     void SubscribeAborted(const TAbortedHandler& handler) override;
     void UnsubscribeAborted(const TAbortedHandler& handler) override;
 
-    void FutureModifyRows(
+    void ModifyRows(
         const NYPath::TYPath& path,
         NTableClient::TNameTablePtr nameTable,
-        TSharedRange<NApi::NFuture::TRowModification> modifications,
+        TSharedRange<NApi::TRowModification> modifications,
         const NApi::TModifyRowsOptions& options) override;
 
     using TQueueTransactionMixin::AdvanceQueueConsumer;
diff --git a/yt/yt/client/federated/client.cpp b/yt/yt/client/federated/client.cpp
index fb09ded2e77..897c9029937 100644
--- a/yt/yt/client/federated/client.cpp
+++ b/yt/yt/client/federated/client.cpp
@@ -66,10 +66,10 @@ public:
         const std::string& query,
         const TSelectRowsOptions& options = {}) override;
 
-    void FutureModifyRows(
+    void ModifyRows(
         const NYPath::TYPath& path,
         NTableClient::TNameTablePtr nameTable,
-        TSharedRange<NFuture::TRowModification> modifications,
+        TSharedRange<TRowModification> modifications,
         const TModifyRowsOptions& options) override;
 
     using TQueueTransactionMixin::AdvanceQueueConsumer;
@@ -584,13 +584,13 @@ TRANSACTION_METHOD_IMPL(NYson::TYsonString, GetNode, (const NYPath::TYPath&, con
 TRANSACTION_METHOD_IMPL(NYson::TYsonString, ListNode, (const NYPath::TYPath&, const TListNodeOptions&));
 TRANSACTION_METHOD_IMPL(bool, NodeExists, (const NYPath::TYPath&, const TNodeExistsOptions&));
 
-void TTransaction::FutureModifyRows(
+void TTransaction::ModifyRows(
     const NYPath::TYPath& path,
     NTableClient::TNameTablePtr nameTable,
-    TSharedRange<NFuture::TRowModification> modifications,
+    TSharedRange<TRowModification> modifications,
     const TModifyRowsOptions& options)
 {
-    Underlying_->FutureModifyRows(path, nameTable, modifications, options);
+    Underlying_->ModifyRows(path, nameTable, modifications, options);
 }
 
 TFuture<TTransactionFlushResult> TTransaction::Flush()
diff --git a/yt/yt/client/unittests/mock/transaction.h b/yt/yt/client/unittests/mock/transaction.h
index 3fda33423ff..f2d9861a5d9 100644
--- a/yt/yt/client/unittests/mock/transaction.h
+++ b/yt/yt/client/unittests/mock/transaction.h
@@ -240,12 +240,6 @@ public:
         i64 newOffset,
         const TAdvanceQueueConsumerOptions& options), (override));
 
-    MOCK_METHOD(void, FutureModifyRows, (
-        const NYPath::TYPath& path,
-        NTableClient::TNameTablePtr nameTable,
-        TSharedRange<NFuture::TRowModification> modifications,
-        const TModifyRowsOptions& options), (override));
-
     MOCK_METHOD(void, ModifyRows, (
         const NYPath::TYPath& path,
         NTableClient::TNameTablePtr nameTable,