diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_move_index.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_move_index.cpp index f1ec56ee19c6..6d2b311adca6 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_move_index.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_move_index.cpp @@ -583,6 +583,7 @@ TVector 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; diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_move_sequence.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_move_sequence.cpp index 28678f81455c..bd487cd72ad9 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_move_sequence.cpp +++ b/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; diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_move_tables.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_move_tables.cpp index 48e609945baf..12ba21b3a29b 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_move_tables.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_move_tables.cpp @@ -92,23 +92,28 @@ TVector 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& result, const TPath& srcTable, + const TString& dstPath, const THashSet& 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; } } diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_part.h b/ydb/core/tx/schemeshard/schemeshard__operation_part.h index b0bcb176383e..909c91b95671 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_part.h +++ b/ydb/core/tx/schemeshard/schemeshard__operation_part.h @@ -634,6 +634,8 @@ ISubOperation::TPtr CreateAlterLogin(TOperationId id, TTxState::ETxState state); TVector CreateConsistentMoveTable(TOperationId id, const TTxTransaction& tx, TOperationContext& context); TVector CreateConsistentMoveIndex(TOperationId id, const TTxTransaction& tx, TOperationContext& context); +void AddMoveSequences(TOperationId nextId, TVector& result, + const TPath& srcTable, const TString& dstPath, const THashSet& sequences); ISubOperation::TPtr CreateMoveTable(TOperationId id, const TTxTransaction& tx); ISubOperation::TPtr CreateMoveTable(TOperationId id, TTxState::ETxState state); diff --git a/ydb/core/tx/schemeshard/ut_move/ut_move.cpp b/ydb/core/tx/schemeshard/ut_move/ut_move.cpp index 83aa5d0db7d7..ff0ff1203c1d 100644 --- a/ydb/core/tx/schemeshard/ut_move/ut_move.cpp +++ b/ydb/core/tx/schemeshard/ut_move/ut_move.cpp @@ -1059,7 +1059,7 @@ 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); @@ -1067,11 +1067,20 @@ Y_UNIT_TEST_SUITE(TSchemeShardMoveTest) { 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); }