Fix moving sequences during the new prefixed vector index move/replace

ydb/core/tx/schemeshard/schemeshard__operation_move_index.cpp

@@ -583,6 +583,7 @@ TVector<ISubOperation::TPtr> CreateConsistentMoveIndex(TOperationId nextId, cons
         TPath dstImplTable = dstIndexPath.Child(srcImplTableName);
 
         result.push_back(CreateMoveTable(NextPartId(nextId, result), MoveTableTask(srcImplTable, dstImplTable)));
+        AddMoveSequences(nextId, result, srcImplTable, dstImplTable.PathString(), GetLocalSequences(context, srcImplTable));
     }
 
     return result;

ydb/core/tx/schemeshard/schemeshard__operation_move_sequence.cpp

@@ -813,18 +813,28 @@ class TMoveSequence: public TSubOperation {
                 .IsResolved()
                 .NotDeleted()
                 .NotUnderDeleting()
-                .IsCommonSensePath()
                 .NotAsyncReplicaTable();
 
             if (checks) {
-                if (srcParentPath->IsTable()) {
+                if (srcParentPath.Parent()->IsTableIndex()) {
+                    // Only __ydb_id sequence can be created in the prefixed index
+                    if (srcPath.LeafName() != NTableIndex::NKMeans::IdColumnSequence) {
+                        result->SetError(NKikimrScheme::EStatus::StatusNameConflict, "sequences are not allowed in indexes");
+                        return result;
+                    }
+                    if (srcParentPath.IsUnderOperation()) {
+                        checks.IsUnderTheSameOperation(OperationId.GetTxId()); // allowed only as part of consistent operations
+                    }
+                } else if (srcParentPath->IsTable()) {
                     // allow immediately inside a normal table
+                    checks.IsCommonSensePath();
                     if (srcParentPath.IsUnderOperation()) {
                         checks.IsUnderTheSameOperation(OperationId.GetTxId()); // allowed only as part of consistent operations
                     }
                 } else {
                     // otherwise don't allow unexpected object types
-                    checks.IsLikeDirectory();
+                    checks.IsCommonSensePath()
+                          .IsLikeDirectory();
                 }
             }
 
@@ -835,28 +845,6 @@ class TMoveSequence: public TSubOperation {
         }
 
         TPath dstPath = TPath::Resolve(dstPathStr, context.SS);
-        TPath dstParentPath = dstPath.Parent();
-
-        {
-            TPath::TChecker checks = dstParentPath.Check();
-            checks
-                .NotUnderDomainUpgrade()
-                .IsAtLocalSchemeShard()
-                .IsResolved();
-
-            if (dstParentPath.IsUnderOperation()) {
-                checks
-                    .IsUnderTheSameOperation(OperationId.GetTxId());
-            } else {
-                checks
-                    .NotUnderOperation();
-            }
-
-            if (!checks) {
-                result->SetError(checks.GetStatus(), checks.GetError());
-                return result;
-            }
-        }
 
         const TString acl = Transaction.GetModifyACL().GetDiffACL();
 
@@ -893,7 +881,6 @@ class TMoveSequence: public TSubOperation {
                     .DepthLimit()
                     .IsValidLeafName(context.UserToken.Get())
                     .IsTheSameDomain(srcPath)
-                    .DirChildrenLimit()
                     .IsValidACL(acl);
             }
 
@@ -903,6 +890,50 @@ class TMoveSequence: public TSubOperation {
             }
         }
 
+        // Parent is probably already modified and inactive, because it's either a regular table or an index
+        // implementation table which is in the process of moving. "Inactive" paths are only used in move
+        // operations and mean that the path isn't yet inserted into the child node map of its parent itself.
+        // Thus, dstParentPath checks are performed on the 'new' (inactive) version.
+
+        // Most checks on dstPath are performed on the 'old' path, but DirChildrenLimit requires dstParentPath
+        // to be resolved, so we perform it on the 'new' version of the path.
+
+        dstPath = TPath::ResolveWithInactive(OperationId, dstPathStr, context.SS);
+        TPath dstParentPath = dstPath.Parent();
+
+        {
+            TPath::TChecker checks = dstPath.Check();
+
+            checks
+                .DirChildrenLimit();
+
+            if (!checks) {
+                result->SetError(checks.GetStatus(), checks.GetError());
+                return result;
+            }
+        }
+
+        {
+            TPath::TChecker checks = dstParentPath.Check();
+
+            checks
+                .NotUnderDomainUpgrade()
+                .IsAtLocalSchemeShard()
+                .IsResolved();
+            if (dstParentPath.IsUnderOperation()) {
+                checks
+                    .IsUnderTheSameOperation(OperationId.GetTxId());
+            } else {
+                checks
+                    .NotUnderOperation();
+            }
+
+            if (!checks) {
+                result->SetError(checks.GetStatus(), checks.GetError());
+                return result;
+            }
+        }
+
         if (!context.SS->CheckApplyIf(Transaction, errStr)) {
             result->SetError(NKikimrScheme::StatusPreconditionFailed, errStr);
             return result;

ydb/core/tx/schemeshard/schemeshard__operation_move_tables.cpp

@@ -92,23 +92,28 @@ TVector<ISubOperation::TPtr> CreateConsistentMoveTable(TOperationId nextId, cons
             TPath dstImplTable = dstIndexPath.Child(implTableName);
 
             result.push_back(CreateMoveTable(NextPartId(nextId, result), MoveTableTask(srcImplTable, dstImplTable)));
+            AddMoveSequences(nextId, result, srcImplTable, dstImplTable.PathString(), GetLocalSequences(context, srcImplTable));
         }
     }
 
+    AddMoveSequences(nextId, result, srcPath, dstPath.PathString(), sequences);
+
+    return result;
+}
+
+void AddMoveSequences(TOperationId nextId, TVector<ISubOperation::TPtr>& result, const TPath& srcTable,
+    const TString& dstPath, const THashSet<TString>& sequences)
+{
     for (const auto& sequence : sequences) {
-        auto scheme = TransactionTemplate(
-            dstPath.PathString(),
-            NKikimrSchemeOp::EOperationType::ESchemeOpMoveSequence);
+        auto scheme = TransactionTemplate(dstPath, NKikimrSchemeOp::EOperationType::ESchemeOpMoveSequence);
         scheme.SetFailOnExist(true);
 
         auto* moveSequence = scheme.MutableMoveSequence();
-        moveSequence->SetSrcPath(srcPath.PathString() + "/" + sequence);
-        moveSequence->SetDstPath(dstPath.PathString() + "/" + sequence);
+        moveSequence->SetSrcPath(srcTable.PathString() + "/" + sequence);
+        moveSequence->SetDstPath(dstPath + "/" + sequence);
 
         result.push_back(CreateMoveSequence(NextPartId(nextId, result), scheme));
     }
-
-    return result;
 }
 
 }

ydb/core/tx/schemeshard/schemeshard__operation_part.h

@@ -634,6 +634,8 @@ ISubOperation::TPtr CreateAlterLogin(TOperationId id, TTxState::ETxState state);
 
 TVector<ISubOperation::TPtr> CreateConsistentMoveTable(TOperationId id, const TTxTransaction& tx, TOperationContext& context);
 TVector<ISubOperation::TPtr> CreateConsistentMoveIndex(TOperationId id, const TTxTransaction& tx, TOperationContext& context);
+void AddMoveSequences(TOperationId nextId, TVector<ISubOperation::TPtr>& result,
+    const TPath& srcTable, const TString& dstPath, const THashSet<TString>& sequences);
 
 ISubOperation::TPtr CreateMoveTable(TOperationId id, const TTxTransaction& tx);
 ISubOperation::TPtr CreateMoveTable(TOperationId id, TTxState::ETxState state);

ydb/core/tx/schemeshard/ut_move/ut_move.cpp

@@ -1059,19 +1059,28 @@ Y_UNIT_TEST_SUITE(TSchemeShardMoveTest) {
 
         // Replace again - it previously crashed here when Dec/IncAliveChildren were incorrect
 
-        TestBuildVectorIndex(runtime, ++txId, TTestTxConfig::SchemeShard, "/MyRoot", "/MyRoot/Table", "index2", {"embedding"});
+        TestBuildVectorIndex(runtime, ++txId, TTestTxConfig::SchemeShard, "/MyRoot", "/MyRoot/Table", "index2", {"prefix", "embedding"});
         env.TestWaitNotification(runtime, txId);
 
         TestMoveIndex(runtime, ++txId, "/MyRoot/Table", "index2", "index1", true);
         env.TestWaitNotification(runtime, txId);
 
         TestDescribeResult(DescribePath(runtime, "/MyRoot/Table/index2"), {NLs::PathNotExist});
         TestDescribeResult(DescribePath(runtime, "/MyRoot/Table"), {NLs::PathExist, NLs::IndexesCount(1)});
-        TestDescribeResult(DescribePrivatePath(runtime, "/MyRoot/Table/index1/indexImplPrefixTable"), {NLs::PathNotExist});
+        TestDescribeResult(DescribePrivatePath(runtime, "/MyRoot/Table/index1/indexImplPrefixTable"),
+            { NLs::PathExist, NLs::CheckColumns(PrefixTable, {"prefix", IdColumn}, {}, {"prefix", IdColumn}, true) });
         TestDescribeResult(DescribePrivatePath(runtime, "/MyRoot/Table/index1/indexImplLevelTable"),
             { NLs::PathExist, NLs::CheckColumns(LevelTable, {ParentColumn, IdColumn, CentroidColumn}, {}, {ParentColumn, IdColumn}, true) });
         TestDescribeResult(DescribePrivatePath(runtime, "/MyRoot/Table/index1/indexImplPostingTable"),
             { NLs::PathExist, NLs::CheckColumns(PostingTable, {ParentColumn, "key"}, {}, {ParentColumn, "key"}, true) });
+
+        // Drop - it also crashed here when the sequence wasn't moved correctly
+
+        TestDropTableIndex(runtime, ++txId, "/MyRoot", R"(
+            TableName: "Table"
+            IndexName: "index1"
+        )");
+        env.TestWaitNotification(runtime, txId);
     }