From 3e756cc18da2c49b8757d65d9431e69b778d1e58 Mon Sep 17 00:00:00 2001 From: Aleksei Uzhegov Date: Tue, 30 Jan 2024 18:52:54 +0300 Subject: [PATCH 01/11] Initial implementation --- ydb/core/protos/feature_flags.proto | 1 + ydb/core/testlib/basics/feature_flags.h | 1 + .../tx/schemeshard/schemeshard__operation.cpp | 14 +- ...__operation_alter_external_data_source.cpp | 287 ++++++++++++ ...eshard__operation_alter_external_table.cpp | 421 ++++++++++++++++++ ..._operation_create_external_data_source.cpp | 52 ++- ...shard__operation_create_external_table.cpp | 51 ++- .../schemeshard/schemeshard__operation_part.h | 10 +- ydb/core/tx/schemeshard/schemeshard_impl.cpp | 3 +- ydb/core/tx/schemeshard/schemeshard_impl.h | 1 + ydb/core/tx/schemeshard/schemeshard_path.cpp | 31 ++ ydb/core/tx/schemeshard/schemeshard_path.h | 2 + .../tx/schemeshard/ut_helpers/test_env.cpp | 5 +- ydb/core/tx/schemeshard/ut_helpers/test_env.h | 3 +- ydb/core/tx/schemeshard/ya.make | 2 + 15 files changed, 867 insertions(+), 17 deletions(-) create mode 100644 ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp create mode 100644 ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp diff --git a/ydb/core/protos/feature_flags.proto b/ydb/core/protos/feature_flags.proto index 12871c77b59b..042c2e7b5cf3 100644 --- a/ydb/core/protos/feature_flags.proto +++ b/ydb/core/protos/feature_flags.proto @@ -128,4 +128,5 @@ message TFeatureFlags { optional bool EnableServerlessExclusiveDynamicNodes = 113 [default = false]; optional bool EnableAccessServiceBulkAuthorization = 114 [default = false]; optional bool EnableAddColumsWithDefaults = 115 [ default = false]; + optional bool EnableReplaceIfExists = 116 [ default = false]; } diff --git a/ydb/core/testlib/basics/feature_flags.h b/ydb/core/testlib/basics/feature_flags.h index 22086cb95595..73d0d43a7eb8 100644 --- a/ydb/core/testlib/basics/feature_flags.h +++ b/ydb/core/testlib/basics/feature_flags.h @@ -57,6 +57,7 @@ class TTestFeatureFlagsHolder { FEATURE_FLAG_SETTER(EnableServerlessExclusiveDynamicNodes) FEATURE_FLAG_SETTER(EnableAccessServiceBulkAuthorization) FEATURE_FLAG_SETTER(EnableAddColumsWithDefaults) + FEATURE_FLAG_SETTER(EnableReplaceIfExists) #undef FEATURE_FLAG_SETTER }; diff --git a/ydb/core/tx/schemeshard/schemeshard__operation.cpp b/ydb/core/tx/schemeshard/schemeshard__operation.cpp index 33a4df19795d..676771621098 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation.cpp @@ -1062,14 +1062,14 @@ ISubOperation::TPtr TOperation::RestorePart(TTxState::ETxType txType, TTxState:: case TTxState::ETxType::TxDropExternalTable: return CreateDropExternalTable(NextPartId(), txState); case TTxState::ETxType::TxAlterExternalTable: - Y_ABORT("TODO: implement"); + return CreateAlterExternalTable(NextPartId(), txState); case TTxState::ETxType::TxCreateExternalDataSource: return CreateNewExternalDataSource(NextPartId(), txState); case TTxState::ETxType::TxDropExternalDataSource: return CreateDropExternalDataSource(NextPartId(), txState); case TTxState::ETxType::TxAlterExternalDataSource: - Y_ABORT("TODO: implement"); - + return CreateAlterExternalDataSource(NextPartId(), txState); + // View case TTxState::ETxType::TxCreateView: return CreateNewView(NextPartId(), txState); @@ -1282,7 +1282,7 @@ ISubOperation::TPtr TOperation::ConstructPart(NKikimrSchemeOp::EOperationType op // ExternalTable case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalTable: - return CreateNewExternalTable(NextPartId(), tx); + Y_ABORT("operation is handled before"); case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalTable: return CreateDropExternalTable(NextPartId(), tx); case NKikimrSchemeOp::EOperationType::ESchemeOpAlterExternalTable: @@ -1290,7 +1290,7 @@ ISubOperation::TPtr TOperation::ConstructPart(NKikimrSchemeOp::EOperationType op // ExternalDataSource case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalDataSource: - return CreateNewExternalDataSource(NextPartId(), tx); + Y_ABORT("operation is handled before"); case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalDataSource: return CreateDropExternalDataSource(NextPartId(), tx); case NKikimrSchemeOp::EOperationType::ESchemeOpAlterExternalDataSource: @@ -1351,6 +1351,10 @@ TVector TOperation::ConstructParts(const TTxTransaction& tx return CreateConsistentMoveIndex(NextPartId(), tx, context); case NKikimrSchemeOp::EOperationType::ESchemeOpAlterExtSubDomain: return CreateCompatibleAlterExtSubDomain(NextPartId(), tx, context); + case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalDataSource: + return CreateNewExternalDataSource(NextPartId(), tx, context); + case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalTable: + return CreateNewExternalTable(NextPartId(), tx, context); default: return {ConstructPart(opType, tx)}; } diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp new file mode 100644 index 000000000000..49b87d8059f5 --- /dev/null +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp @@ -0,0 +1,287 @@ +#include "schemeshard__operation_common_external_data_source.h" +#include "schemeshard__operation_part.h" +#include "schemeshard__operation_common.h" +#include "schemeshard_impl.h" + +#include + +namespace { + +using namespace NKikimr; +using namespace NSchemeShard; + +class TPropose: public TSubOperationState { +private: + const TOperationId OperationId; + + TString DebugHint() const override { + return TStringBuilder() + << "TAlterExternalDataSource TPropose" + << ", operationId: " << OperationId; + } + +public: + explicit TPropose(TOperationId id) + : OperationId(std::move(id)) + { + } + + bool HandleReply(TEvPrivate::TEvOperationPlan::TPtr& ev, TOperationContext& context) override { + const TStepId step = TStepId(ev->Get()->StepId); + + LOG_I(DebugHint() << "HandleReply TEvOperationPlan" + << ": step# " << step); + + const TTxState* txState = context.SS->FindTx(OperationId); + Y_ABORT_UNLESS(txState); + Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalDataSource); + + const auto pathId = txState->TargetPathId; + const auto path = TPath::Init(pathId, context.SS); + const TPathElement::TPtr pathPtr = context.SS->PathsById.at(pathId); + + NIceDb::TNiceDb db(context.GetDB()); + + IncParentDirAlterVersionWithRepublish(OperationId, path, context); + + context.SS->ClearDescribePathCaches(pathPtr); + context.OnComplete.PublishToSchemeBoard(OperationId, pathId); + + context.SS->ChangeTxState(db, OperationId, TTxState::Done); + return true; + } + + bool ProgressState(TOperationContext& context) override { + LOG_I(DebugHint() << "ProgressState"); + + const TTxState* txState = context.SS->FindTx(OperationId); + Y_ABORT_UNLESS(txState); + Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalDataSource); + + context.OnComplete.ProposeToCoordinator(OperationId, txState->TargetPathId, TStepId(0)); + return false; + } +}; + +class TAlterExternalDataSource : public TSubOperation { + static TTxState::ETxState NextState() { return TTxState::Propose; } + + TTxState::ETxState NextState(TTxState::ETxState state) const override { + switch (state) { + case TTxState::Waiting: + case TTxState::Propose: + return TTxState::Done; + default: + return TTxState::Invalid; + } + } + + TSubOperationState::TPtr SelectStateFunc(TTxState::ETxState state) override { + switch (state) { + case TTxState::Waiting: + case TTxState::Propose: + return MakeHolder(OperationId); + case TTxState::Done: + return MakeHolder(OperationId); + default: + return nullptr; + } + } + + static bool IsDestinationPathValid(const THolder& result, + const TPath& dstPath, + const TString& acl) { + const auto checks = dstPath.Check(); + // ReSharper disable once CppExpressionWithoutSideEffects + checks.IsAtLocalSchemeShard() + .IsResolved() + .NotUnderDeleting() + .FailOnWrongType(TPathElement::EPathType::EPathTypeExternalDataSource) + .IsValidLeafName() + .DepthLimit() + .PathsLimit() + .DirChildrenLimit() + .IsValidACL(acl); + + if (!checks) { + result->SetError(checks.GetStatus(), checks.GetError()); + if (dstPath.IsResolved()) { + result->SetPathCreateTxId(static_cast(dstPath.Base()->CreateTxId)); + result->SetPathId(dstPath.Base()->PathId.LocalPathId); + } + } + + return static_cast(checks); + } + + bool IsApplyIfChecksPassed(const THolder& result, + const TOperationContext& context) const { + TString errorMessage; + if (!context.SS->CheckApplyIf(Transaction, errorMessage)) { + result->SetError(NKikimrScheme::StatusPreconditionFailed, errorMessage); + return false; + } + return true; + } + + static bool IsDescriptionValid( + const THolder& result, + const NKikimrSchemeOp::TExternalDataSourceDescription& desc, + const NExternalSource::IExternalSourceFactory::TPtr& factory) { + TString errorMessage; + if (!NExternalDataSource::Validate(desc, factory, errorMessage)) { + result->SetError(NKikimrScheme::StatusSchemeError, errorMessage); + return false; + } + return true; + } + + static void AddPathInSchemeShard( + const THolder& result, const TPath& dstPath) { + result->SetPathId(dstPath.Base()->PathId.LocalPathId); + } + + TPathElement::TPtr CreateExternalDataSourcePathElement(const TPath& dstPath) const { + TPathElement::TPtr externalDataSource = dstPath.Base(); + + externalDataSource->PathState = TPathElement::EPathState::EPathStateAlter; + externalDataSource->LastTxId = OperationId.GetTxId(); + + return externalDataSource; + } + + void CreateTransaction(const TOperationContext& context, + const TPathId& externalDataSourcePathId) const { + TTxState& txState = context.SS->CreateTx(OperationId, + TTxState::TxAlterExternalDataSource, + externalDataSourcePathId); + txState.Shards.clear(); + } + + void RegisterParentPathDependencies(const TOperationContext& context, + const TPath& parentPath) const { + if (parentPath.Base()->HasActiveChanges()) { + const TTxId parentTxId = parentPath.Base()->PlannedToCreate() + ? parentPath.Base()->CreateTxId + : parentPath.Base()->LastTxId; + context.OnComplete.Dependence(parentTxId, OperationId.GetTxId()); + } + } + + void AdvanceTransactionStateToPropose(const TOperationContext& context, + NIceDb::TNiceDb& db) const { + context.SS->ChangeTxState(db, OperationId, TTxState::Propose); + context.OnComplete.ActivateTx(OperationId); + } + + void PersistExternalDataSource( + const TOperationContext& context, + NIceDb::TNiceDb& db, + const TPathElement::TPtr& externalDataSourcePath, + const TExternalDataSourceInfo::TPtr& externalDataSourceInfo, + const TString& acl) const { + const auto& externalDataSourcePathId = externalDataSourcePath->PathId; + + context.SS->ExternalDataSources[externalDataSourcePathId] = externalDataSourceInfo; + context.SS->PersistPath(db, externalDataSourcePathId); + + if (!acl.empty()) { + externalDataSourcePath->ApplyACL(acl); + context.SS->PersistACL(db, externalDataSourcePath); + } + + context.SS->PersistExternalDataSource(db, + externalDataSourcePathId, + externalDataSourceInfo); + context.SS->PersistTxState(db, OperationId); + } + +public: + using TSubOperation::TSubOperation; + + THolder Propose(const TString& owner, + TOperationContext& context) override { + Y_UNUSED(owner); + const auto ssId = context.SS->SelfTabletId(); + const TString& parentPathStr = Transaction.GetWorkingDir(); + const auto& externalDataSourceDescription = + Transaction.GetCreateExternalDataSource(); + const TString& name = externalDataSourceDescription.GetName(); + + LOG_N("TAlterExternalDataSource Propose" + << ": opId# " << OperationId << ", path# " << parentPathStr << "/" << name); + + auto result = MakeHolder(NKikimrScheme::StatusAccepted, + static_cast(OperationId.GetTxId()), + static_cast(ssId)); + + const TPath parentPath = TPath::Resolve(parentPathStr, context.SS); + RETURN_RESULT_UNLESS(NExternalDataSource::IsParentPathValid(result, parentPath)); + + const TString acl = Transaction.GetModifyACL().GetDiffACL(); + const TPath dstPath = parentPath.Child(name); + + RETURN_RESULT_UNLESS(IsDestinationPathValid(result, dstPath, acl)); + RETURN_RESULT_UNLESS(IsApplyIfChecksPassed(result, context)); + RETURN_RESULT_UNLESS(IsDescriptionValid(result, + externalDataSourceDescription, + context.SS->ExternalSourceFactory)); + + const auto oldExternalDataSourceInfo = + context.SS->ExternalDataSources.Value(dstPath->PathId, nullptr); + Y_ABORT_UNLESS(oldExternalDataSourceInfo); + const TExternalDataSourceInfo::TPtr externalDataSourceInfo = + NExternalDataSource::CreateExternalDataSource(externalDataSourceDescription, + oldExternalDataSourceInfo->AlterVersion + 1); + Y_ABORT_UNLESS(externalDataSourceInfo); + + AddPathInSchemeShard(result, dstPath); + const TPathElement::TPtr externalDataSource = + CreateExternalDataSourcePathElement(dstPath); + CreateTransaction(context, externalDataSource->PathId); + + NIceDb::TNiceDb db(context.GetDB()); + + RegisterParentPathDependencies(context, parentPath); + + AdvanceTransactionStateToPropose(context, db); + + PersistExternalDataSource( + context, db, externalDataSource, externalDataSourceInfo, acl); + + IncParentDirAlterVersionWithRepublishSafeWithUndo(OperationId, + dstPath, + context.SS, + context.OnComplete); + + SetState(NextState()); + return result; + } + + void AbortPropose(TOperationContext& context) override { + LOG_N("TAlterExternalDataSource AbortPropose" + << ": opId# " << OperationId); + Y_ABORT("no AbortPropose for TAlterExternalDataSource"); + } + + void AbortUnsafe(TTxId forceDropTxId, TOperationContext& context) override { + LOG_N("TAlterExternalDataSource AbortUnsafe" + << ": opId# " << OperationId << ", txId# " << forceDropTxId); + context.OnComplete.DoneOperation(OperationId); + } +}; + +} // namespace + +namespace NKikimr::NSchemeShard { + +ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, const TTxTransaction& tx) { + return MakeSubOperation(id, tx); +} + +ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, TTxState::ETxState state) { + Y_ABORT_UNLESS(state != TTxState::Invalid); + return MakeSubOperation(id, state); +} + +} diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp new file mode 100644 index 000000000000..9498102bdcb5 --- /dev/null +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp @@ -0,0 +1,421 @@ +#include "schemeshard__operation_common_external_table.h" +#include "schemeshard__operation_part.h" +#include "schemeshard__operation_common.h" +#include "schemeshard_impl.h" + +#include + +namespace { + +using namespace NKikimr; +using namespace NSchemeShard; + +class TPropose: public TSubOperationState { +private: + const TOperationId OperationId; + bool IsSameDataSource = false; + TPathId OldSourcePathId = InvalidPathId; + + TString DebugHint() const override { + return TStringBuilder() + << "TAlterExternalTable TPropose" + << ", operationId: " << OperationId; + } + + void ClearDescribePathCaches(const TOperationContext& context, + const TPathElement::TPtr& pathPtr, + const TPathElement::TPtr& dataSourcePathPtr, + const TPathElement::TPtr& oldDataSourcePathPtr) const { + context.SS->ClearDescribePathCaches(pathPtr); + if (!IsSameDataSource) { + context.SS->ClearDescribePathCaches(dataSourcePathPtr); + context.SS->ClearDescribePathCaches(oldDataSourcePathPtr); + } + } + + void PublishToSchemeBoard(const TOperationContext& context, + const TPathId& pathId, + const TPathId& dataSourcePathId) const { + context.OnComplete.PublishToSchemeBoard(OperationId, pathId); + if (!IsSameDataSource) { + context.OnComplete.PublishToSchemeBoard(OperationId, dataSourcePathId); + context.OnComplete.PublishToSchemeBoard(OperationId, OldSourcePathId); + } + } + +public: + explicit TPropose(TOperationId id, + bool isSameDataSource, + const TPathId oldSourcePathId) + : OperationId(std::move(id)) + , IsSameDataSource(isSameDataSource) + , OldSourcePathId(oldSourcePathId) { } + + bool HandleReply(TEvPrivate::TEvOperationPlan::TPtr& ev, TOperationContext& context) override { + const TStepId step = TStepId(ev->Get()->StepId); + + LOG_I(DebugHint() << " HandleReply TEvOperationPlan" + << ": step# " << step); + + const TTxState* txState = context.SS->FindTx(OperationId); + Y_ABORT_UNLESS(txState); + Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalTable); + + const auto pathId = txState->TargetPathId; + const auto dataSourcePathId = txState->SourcePathId; + const auto path = TPath::Init(pathId, context.SS); + const TPathElement::TPtr pathPtr = context.SS->PathsById.at(pathId); + const TPathElement::TPtr dataSourcePathPtr = + context.SS->PathsById.at(dataSourcePathId); + TPathElement::TPtr oldDataSourcePathPtr; + if (!IsSameDataSource) { + oldDataSourcePathPtr = context.SS->PathsById.at(OldSourcePathId); + } + + NIceDb::TNiceDb db(context.GetDB()); + + IncParentDirAlterVersionWithRepublish(OperationId, path, context); + + ClearDescribePathCaches(context, pathPtr, dataSourcePathPtr, oldDataSourcePathPtr); + PublishToSchemeBoard(context, pathId, dataSourcePathId); + + context.SS->ChangeTxState(db, OperationId, TTxState::Done); + return true; + } + + bool ProgressState(TOperationContext& context) override { + LOG_I(DebugHint() << " ProgressState"); + + const TTxState* txState = context.SS->FindTx(OperationId); + Y_ABORT_UNLESS(txState); + Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalTable); + + context.OnComplete.ProposeToCoordinator(OperationId, txState->TargetPathId, TStepId(0)); + return false; + } +}; + + +class TAltertExternalTable: public TSubOperation { +private: + bool IsSameDataSource = true; + TPathId OldDataSourcePathId = InvalidPathId; + + static TTxState::ETxState NextState() { + return TTxState::Propose; + } + + TTxState::ETxState NextState(TTxState::ETxState state) const override { + switch (state) { + case TTxState::Waiting: + case TTxState::Propose: + return TTxState::Done; + default: + return TTxState::Invalid; + } + } + + TSubOperationState::TPtr SelectStateFunc(TTxState::ETxState state) override { + switch (state) { + case TTxState::Waiting: + case TTxState::Propose: + return MakeHolder(OperationId, IsSameDataSource, OldDataSourcePathId); + case TTxState::Done: + return MakeHolder(OperationId); + default: + return nullptr; + } + } + + static bool IsDestinationPathValid(const THolder& result, + const TPath& dstPath, + const TString& acl) { + const auto checks = dstPath.Check(); + checks.IsAtLocalSchemeShard() + .IsResolved() + .NotUnderDeleting() + .FailOnWrongType(TPathElement::EPathType::EPathTypeExternalTable) + .IsValidLeafName() + .DepthLimit() + .PathsLimit() + .DirChildrenLimit() + .IsValidACL(acl); + + if (!checks) { + result->SetError(checks.GetStatus(), checks.GetError()); + if (dstPath.IsResolved()) { + result->SetPathCreateTxId(static_cast(dstPath.Base()->CreateTxId)); + result->SetPathId(dstPath.Base()->PathId.LocalPathId); + } + } + + return static_cast(checks); + } + + static bool IsDataSourcePathValid(const THolder& result, const TPath& dataSourcePath) { + const auto checks = dataSourcePath.Check(); + checks + .NotUnderDomainUpgrade() + .IsAtLocalSchemeShard() + .IsResolved() + .NotDeleted() + .NotUnderDeleting() + .IsCommonSensePath() + .IsExternalDataSource() + .NotUnderOperation(); + + if (!checks) { + result->SetError(checks.GetStatus(), checks.GetError()); + } + + return static_cast(checks); + } + + bool IsApplyIfChecksPassed(const THolder& result, + const TOperationContext& context) const { + TString errorMessage; + if (!context.SS->CheckApplyIf(Transaction, errorMessage)) { + result->SetError(NKikimrScheme::StatusPreconditionFailed, errorMessage); + return false; + } + + return true; + } + + static bool IsDataSourceValid(const THolder& result, + const TExternalDataSourceInfo::TPtr& externalDataSource) { + if (!externalDataSource) { + result->SetError(NKikimrScheme::StatusSchemeError, "Data source doesn't exist"); + return false; + } + return true; + } + + static bool IsExternalTableDescriptionValid( + const THolder& result, + const TString& sourceType, + const NKikimrSchemeOp::TExternalTableDescription& desc) { + if (TString errorMessage; !NExternalTable::Validate(sourceType, desc, errorMessage)) { + result->SetError(NKikimrScheme::StatusSchemeError, errorMessage); + return false; + } + + return true; + } + + static void AddPathInSchemeShard(const THolder& result, + TPath& dstPath) { + result->SetPathId(dstPath.Base()->PathId.LocalPathId); + } + + TPathElement::TPtr CreateExternalTablePathElement(const TPath& dstPath) const { + TPathElement::TPtr externalTable = dstPath.Base(); + externalTable->PathState = TPathElement::EPathState::EPathStateAlter; + externalTable->LastTxId = OperationId.GetTxId(); + + return externalTable; + } + + void CreateTransaction(const TOperationContext& context, + const TPathId& externalTablePathId, + const TPathId& externalDataSourcePathId) const { + TTxState& txState = context.SS->CreateTx(OperationId, + TTxState::TxAlterExternalTable, + externalTablePathId, + externalDataSourcePathId); + txState.Shards.clear(); + } + + void RegisterParentPathDependencies(const TOperationContext& context, + const TPath& parentPath) const { + if (parentPath.Base()->HasActiveChanges()) { + const auto parentTxId = parentPath.Base()->PlannedToCreate() + ? parentPath.Base()->CreateTxId + : parentPath.Base()->LastTxId; + context.OnComplete.Dependence(parentTxId, OperationId.GetTxId()); + } + } + + void AdvanceTransactionStateToPropose(const TOperationContext& context, + NIceDb::TNiceDb& db) const { + context.SS->ChangeTxState(db, OperationId, TTxState::Propose); + context.OnComplete.ActivateTx(OperationId); + } + + static void LinkExternalDataSourceWithExternalTable( + const TExternalDataSourceInfo::TPtr& externalDataSource, + const TPathElement::TPtr& externalTable, + const TPath& dstPath, + const TExternalDataSourceInfo::TPtr& oldDataSource, + bool IsSameDataSource) { + + if (!IsSameDataSource) { + auto& reference = *externalDataSource->ExternalTableReferences.AddReferences(); + reference.SetPath(dstPath.PathString()); + PathIdFromPathId(externalTable->PathId, reference.MutablePathId()); + + EraseIf(*oldDataSource->ExternalTableReferences.MutableReferences(), + [pathId = externalTable->PathId]( + const NKikimrSchemeOp::TExternalTableReferences::TReference& reference) { + return PathIdFromPathId(reference.GetPathId()) == pathId; + }); + } + } + + void PersistExternalTable( + const TOperationContext& context, + NIceDb::TNiceDb& db, + const TPathElement::TPtr& externalTable, + const TExternalTableInfo::TPtr& externalTableInfo, + const TPathId& externalDataSourcePathId, + const TExternalDataSourceInfo::TPtr& externalDataSource, + const TPathId& oldExternalDataSourcePathId, + const TExternalDataSourceInfo::TPtr& oldExternalDataSource, + const TString& acl, + bool isSameDataSource) const { + context.SS->ExternalTables[externalTable->PathId] = externalTableInfo; + + context.SS->PersistPath(db, externalTable->PathId); + + if (!acl.empty()) { + externalTable->ApplyACL(acl); + context.SS->PersistACL(db, externalTable); + } + + if (!isSameDataSource) { + context.SS->PersistExternalDataSource(db, externalDataSourcePathId, externalDataSource); + context.SS->PersistExternalDataSource(db, oldExternalDataSourcePathId, oldExternalDataSource); + } + context.SS->PersistExternalTable(db, externalTable->PathId, externalTableInfo); + context.SS->PersistTxState(db, OperationId); + } + +public: + using TSubOperation::TSubOperation; + + THolder Propose(const TString& owner, TOperationContext& context) override { + Y_UNUSED(owner); + const auto ssId = context.SS->SelfTabletId(); + + const TString& parentPathStr = Transaction.GetWorkingDir(); + const auto& externalTableDescription = Transaction.GetCreateExternalTable(); + const TString& name = externalTableDescription.GetName(); + + LOG_N("TAlterExternalTable Propose" + << ": opId# " << OperationId + << ", path# " << parentPathStr << "/" << name << ", ReplaceIfExists:" << externalTableDescription.GetReplaceIfExists()); + + auto result = MakeHolder(NKikimrScheme::StatusAccepted, + static_cast(OperationId.GetTxId()), + static_cast(ssId)); + + const auto parentPath = TPath::Resolve(parentPathStr, context.SS); + RETURN_RESULT_UNLESS(NExternalTable::IsParentPathValid(result, parentPath)); + + const TString acl = Transaction.GetModifyACL().GetDiffACL(); + TPath dstPath = parentPath.Child(name); + RETURN_RESULT_UNLESS(IsDestinationPathValid(result, dstPath, acl)); + + const auto dataSourcePath = + TPath::Resolve(externalTableDescription.GetDataSourcePath(), context.SS); + RETURN_RESULT_UNLESS(IsDataSourcePathValid(result, dataSourcePath)); + + const auto externalDataSource = + context.SS->ExternalDataSources.Value(dataSourcePath->PathId, nullptr); + RETURN_RESULT_UNLESS(IsDataSourceValid(result, externalDataSource)); + + RETURN_RESULT_UNLESS(IsApplyIfChecksPassed(result, context)); + + RETURN_RESULT_UNLESS(IsExternalTableDescriptionValid(result, + externalDataSource->SourceType, + externalTableDescription)); + + /// Extract old data source + TExternalDataSourceInfo::TPtr oldDataSource; + { + const auto oldExternalTableRecord = context.SS->ExternalTables.Value(dstPath->PathId, nullptr); + Y_ABORT_UNLESS(oldExternalTableRecord); + const auto oldDataSourcePath = TPath::Resolve(oldExternalTableRecord->DataSourcePath, context.SS); + RETURN_RESULT_UNLESS(IsDataSourcePathValid(result, oldDataSourcePath)); + + OldDataSourcePathId = oldDataSourcePath->PathId; + IsSameDataSource = oldDataSourcePath.PathString() == dataSourcePath.PathString(); + if (!IsSameDataSource) { + oldDataSource = context.SS->ExternalDataSources.Value(oldDataSourcePath->PathId, nullptr); + Y_ABORT_UNLESS(oldDataSource); + } + } + /// Extract old data source end + + const auto oldExternalTableInfo = + context.SS->ExternalTables.Value(dstPath->PathId, nullptr); + Y_ABORT_UNLESS(oldExternalTableInfo); + auto [externalTableInfo, maybeError] = + NExternalTable::CreateExternalTable(externalDataSource->SourceType, + externalTableDescription, + context.SS->ExternalSourceFactory, + oldExternalTableInfo->AlterVersion + 1); + if (maybeError) { + result->SetError(NKikimrScheme::StatusSchemeError, *maybeError); + return result; + } + Y_ABORT_UNLESS(externalTableInfo); + + AddPathInSchemeShard(result, dstPath); + + const auto externalTable = CreateExternalTablePathElement(dstPath); + CreateTransaction(context, externalTable->PathId, dataSourcePath->PathId); + + NIceDb::TNiceDb db(context.GetDB()); + + RegisterParentPathDependencies(context, parentPath); + AdvanceTransactionStateToPropose(context, db); + + LinkExternalDataSourceWithExternalTable(externalDataSource, + externalTable, + dstPath, + oldDataSource, + IsSameDataSource); + + PersistExternalTable(context, db, externalTable, externalTableInfo, + dataSourcePath->PathId, externalDataSource, + OldDataSourcePathId, oldDataSource, acl, + IsSameDataSource); + + IncParentDirAlterVersionWithRepublishSafeWithUndo(OperationId, + dstPath, + context.SS, + context.OnComplete); + + SetState(NextState()); + return result; + } + + void AbortPropose(TOperationContext& context) override { + LOG_N("TAlterExternalTable AbortPropose" + << ": opId# " << OperationId); + Y_ABORT("no AbortPropose for TAlterExternalTable"); + } + + void AbortUnsafe(TTxId forceDropTxId, TOperationContext& context) override { + LOG_N("TAlterExternalTable AbortUnsafe" + << ": opId# " << OperationId + << ", txId# " << forceDropTxId); + context.OnComplete.DoneOperation(OperationId); + } +}; + +} + +namespace NKikimr::NSchemeShard { + +ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, const TTxTransaction& tx) { + return MakeSubOperation(std::move(id), tx); +} + +ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, TTxState::ETxState state) { + Y_ABORT_UNLESS(state != TTxState::Invalid); + return MakeSubOperation(std::move(id), state); +} + +} diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp index 85780c48889d..5851800b0e03 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp @@ -298,10 +298,56 @@ class TCreateExternalDataSource : public TSubOperation { namespace NKikimr::NSchemeShard { -ISubOperation::TPtr CreateNewExternalDataSource(TOperationId id, const TTxTransaction& tx) { - return MakeSubOperation(id, tx); -} +TVector CreateNewExternalDataSource(TOperationId id, + const TTxTransaction& tx, + TOperationContext& context) { + Y_ABORT_UNLESS(tx.GetOperationType() == NKikimrSchemeOp::ESchemeOpCreateExternalDataSource); + + LOG_I("CreateNewExternalDataSource, opId " << id << ", feature flag EnableOrReplace " + << context.SS->EnableReplaceIfExists << ", tx " + << tx.ShortDebugString()); + + auto errorResult = [&id](NKikimrScheme::EStatus status, const TStringBuf& msg) -> TVector { + return {CreateReject(id, status, TStringBuilder() << "Invalid TCreateExternalDataSource request: " << msg)}; + }; + + const auto& operation = tx.GetCreateExternalDataSource(); + const auto replaceIfExists = operation.GetReplaceIfExists(); + const TString& name = operation.GetName(); + + if (replaceIfExists && !context.SS->EnableReplaceIfExists) { + return errorResult(NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExists is off"); + } + + const TString& parentPathStr = tx.GetWorkingDir(); + const TPath parentPath = TPath::Resolve(parentPathStr, context.SS); + { + const auto checks = NExternalDataSource::IsParentPathValid(parentPath); + if (!checks) { + return errorResult(checks.GetStatus(), checks.GetError()); + } + } + + const TPath dstPath = parentPath.Child(name); + + const auto checks = dstPath.Check() + .IsAtLocalSchemeShard(); + if (!checks) { + return errorResult(checks.GetStatus(), checks.GetError()); + } + + if (replaceIfExists) { + const auto isAlreadyExists = checks + .IsResolved() + .NotUnderDeleting(); + if (isAlreadyExists) { + return {CreateAlterExternalDataSource(id, tx)}; + } + } + + return {MakeSubOperation(id, tx)}; +} ISubOperation::TPtr CreateNewExternalDataSource(TOperationId id, TTxState::ETxState state) { Y_ABORT_UNLESS(state != TTxState::Invalid); return MakeSubOperation(id, state); diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp index a82af9859b70..67000675e83d 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp @@ -192,7 +192,7 @@ class TCreateExternalTable: public TSubOperation { } static bool IsDataSourceValid(const THolder& result, - const TExternalDataSourceInfo::TPtr& externalDataSource) { + const TExternalDataSourceInfo::TPtr& externalDataSource) { if (!externalDataSource) { result->SetError(NKikimrScheme::StatusSchemeError, "Data source doesn't exist"); return false; @@ -393,8 +393,53 @@ class TCreateExternalTable: public TSubOperation { namespace NKikimr::NSchemeShard { -ISubOperation::TPtr CreateNewExternalTable(TOperationId id, const TTxTransaction& tx) { - return MakeSubOperation(id, tx); +TVector CreateNewExternalTable(TOperationId id, const TTxTransaction& tx, TOperationContext& context) { + Y_ABORT_UNLESS(tx.GetOperationType() == NKikimrSchemeOp::ESchemeOpCreateExternalTable); + + LOG_I("CreateNewExternalTable, opId " << id << ", feature flag EnableOrReplace " + << context.SS->EnableReplaceIfExists << ", tx " + << tx.ShortDebugString()); + + auto errorResult = [&id](NKikimrScheme::EStatus status, const TStringBuf& msg) -> TVector { + return {CreateReject(id, status, TStringBuilder() << "Invalid TCreateExternalTable request: " << msg)}; + }; + + const auto& operation = tx.GetCreateExternalTable(); + const auto replaceIfExists = operation.GetReplaceIfExists(); + const TString& name = operation.GetName(); + + if (replaceIfExists && !context.SS->EnableReplaceIfExists) { + return errorResult(NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExists is off"); + } + + const TString& parentPathStr = tx.GetWorkingDir(); + const TPath parentPath = TPath::Resolve(parentPathStr, context.SS); + + { + const auto checks = NExternalTable::IsParentPathValid(parentPath); + if (!checks) { + return errorResult(checks.GetStatus(), checks.GetError()); + } + } + + const TPath dstPath = parentPath.Child(name); + + const auto checks = dstPath.Check() + .IsAtLocalSchemeShard(); + if (!checks) { + return errorResult(checks.GetStatus(), checks.GetError()); + } + + if (replaceIfExists) { + const auto isAlreadyExists = checks + .IsResolved() + .NotUnderDeleting(); + if (isAlreadyExists) { + return {CreateAlterExternalTable(id, tx)}; + } + } + + return {MakeSubOperation(id, tx)}; } ISubOperation::TPtr CreateNewExternalTable(TOperationId id, TTxState::ETxState state) { diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_part.h b/ydb/core/tx/schemeshard/schemeshard__operation_part.h index 4c592f579028..0a9cf8b6b8b3 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_part.h +++ b/ydb/core/tx/schemeshard/schemeshard__operation_part.h @@ -369,16 +369,22 @@ ISubOperation::TPtr CreateUpdateMainTableOnIndexMove(TOperationId id, TTxState:: // External Table // Create -ISubOperation::TPtr CreateNewExternalTable(TOperationId id, const TTxTransaction& tx); +TVector CreateNewExternalTable(TOperationId id, const TTxTransaction& tx, TOperationContext& context); ISubOperation::TPtr CreateNewExternalTable(TOperationId id, TTxState::ETxState state); +// Alter +ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, const TTxTransaction& tx); +ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, TTxState::ETxState state); // Drop ISubOperation::TPtr CreateDropExternalTable(TOperationId id, const TTxTransaction& tx); ISubOperation::TPtr CreateDropExternalTable(TOperationId id, TTxState::ETxState state); // External Data Source // Create -ISubOperation::TPtr CreateNewExternalDataSource(TOperationId id, const TTxTransaction& tx); +TVector CreateNewExternalDataSource(TOperationId id, const TTxTransaction& tx, TOperationContext& context); ISubOperation::TPtr CreateNewExternalDataSource(TOperationId id, TTxState::ETxState state); +// Alter +ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, const TTxTransaction& tx); +ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, TTxState::ETxState state); // Drop ISubOperation::TPtr CreateDropExternalDataSource(TOperationId id, const TTxTransaction& tx); ISubOperation::TPtr CreateDropExternalDataSource(TOperationId id, TTxState::ETxState state); diff --git a/ydb/core/tx/schemeshard/schemeshard_impl.cpp b/ydb/core/tx/schemeshard/schemeshard_impl.cpp index 9060584c9dd4..7a479a11213d 100644 --- a/ydb/core/tx/schemeshard/schemeshard_impl.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_impl.cpp @@ -4297,6 +4297,7 @@ void TSchemeShard::OnActivateExecutor(const TActorContext &ctx) { EnableTablePgTypes = appData->FeatureFlags.GetEnableTablePgTypes(); EnableServerlessExclusiveDynamicNodes = appData->FeatureFlags.GetEnableServerlessExclusiveDynamicNodes(); EnableAddColumsWithDefaults = appData->FeatureFlags.GetEnableAddColumsWithDefaults(); + EnableReplaceIfExists = appData->FeatureFlags.GetEnableReplaceIfExists(); ConfigureCompactionQueues(appData->CompactionConfig, ctx); ConfigureStatsBatching(appData->SchemeShardConfig, ctx); @@ -5483,7 +5484,7 @@ void TSchemeShard::Handle(TEvHive::TEvUpdateDomainReply::TPtr &ev, const TActorC << ", at schemeshard: " << TabletID()); return; } - + Execute(CreateTxOperationReply(TOperationId(txId, partId), ev), ctx); } diff --git a/ydb/core/tx/schemeshard/schemeshard_impl.h b/ydb/core/tx/schemeshard/schemeshard_impl.h index 295f45ce5eeb..f0c80259a28c 100644 --- a/ydb/core/tx/schemeshard/schemeshard_impl.h +++ b/ydb/core/tx/schemeshard/schemeshard_impl.h @@ -272,6 +272,7 @@ class TSchemeShard bool EnableTablePgTypes = false; bool EnableServerlessExclusiveDynamicNodes = false; bool EnableAddColumsWithDefaults = false; + bool EnableReplaceIfExists = true; TShardDeleter ShardDeleter; diff --git a/ydb/core/tx/schemeshard/schemeshard_path.cpp b/ydb/core/tx/schemeshard/schemeshard_path.cpp index 78c87a86f837..04fccfece46d 100644 --- a/ydb/core/tx/schemeshard/schemeshard_path.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_path.cpp @@ -565,6 +565,37 @@ const TPath::TChecker& TPath::TChecker::IsTheSameDomain(const TPath& another, ES << ", another path: " << another.PathString()); } +const TPath::TChecker& TPath::TChecker::FailOnWrongType(const TSet& expectedTypes) const { + if (Failed) { + return *this; + } + + if (!Path.IsResolved()) { + return *this; + } + + if (Path.IsDeleted()) { + return *this; + } + + if (!expectedTypes.contains(Path.Base()->PathType)) { + return Fail(EStatus::StatusNameConflict, TStringBuilder() << "unexpected path type" + << " (" << BasicPathInfo(Path.Base()) << ")" + << ", expected types: " << JoinSeq(", ", expectedTypes)); + } + + if (!Path.Base()->IsCreateFinished()) { + return Fail(EStatus::StatusMultipleModifications, TStringBuilder() << "path exist but creating right now" + << " (" << BasicPathInfo(Path.Base()) << ")"); + } + + return *this; +} + +const TPath::TChecker& TPath::TChecker::FailOnWrongType(TPathElement::EPathType expectedType) const { + return FailOnWrongType(TSet{expectedType}); +} + const TPath::TChecker& TPath::TChecker::FailOnExist(const TSet& expectedTypes, bool acceptAlreadyExist) const { if (Failed) { return *this; diff --git a/ydb/core/tx/schemeshard/schemeshard_path.h b/ydb/core/tx/schemeshard/schemeshard_path.h index 93257d943f2c..2087a853f06e 100644 --- a/ydb/core/tx/schemeshard/schemeshard_path.h +++ b/ydb/core/tx/schemeshard/schemeshard_path.h @@ -75,6 +75,8 @@ class TPath { const TChecker& IsLikeDirectory(EStatus status = EStatus::StatusPathIsNotDirectory) const; const TChecker& IsDirectory(EStatus status = EStatus::StatusPathIsNotDirectory) const; const TChecker& IsTheSameDomain(const TPath& another, EStatus status = EStatus::StatusInvalidParameter) const; + const TChecker& FailOnWrongType(const TSet& expectedTypes) const; + const TChecker& FailOnWrongType(TPathElement::EPathType expectedType) const; const TChecker& FailOnExist(const TSet& expectedTypes, bool acceptAlreadyExist) const; const TChecker& FailOnExist(TPathElement::EPathType expectedType, bool acceptAlreadyExist) const; const TChecker& IsValidLeafName(EStatus status = EStatus::StatusSchemeError) const; diff --git a/ydb/core/tx/schemeshard/ut_helpers/test_env.cpp b/ydb/core/tx/schemeshard/ut_helpers/test_env.cpp index f8eb23547139..94498c38da1b 100644 --- a/ydb/core/tx/schemeshard/ut_helpers/test_env.cpp +++ b/ydb/core/tx/schemeshard/ut_helpers/test_env.cpp @@ -537,6 +537,7 @@ NSchemeShardUT_Private::TTestEnv::TTestEnv(TTestActorRuntime& runtime, const TTe app.SetEnableTablePgTypes(opts.EnableTablePgTypes_); app.SetEnableServerlessExclusiveDynamicNodes(opts.EnableServerlessExclusiveDynamicNodes_); app.SetEnableAddColumsWithDefaults(opts.EnableAddColumsWithDefaults_); + app.SetEnableReplaceIfExists(opts.EnableReplaceIfExists_); app.ColumnShardConfig.SetDisabledOnSchemeShard(false); @@ -826,8 +827,8 @@ std::function, EnableTablePgTypes, std::nullopt); OPTION(std::optional, EnableServerlessExclusiveDynamicNodes, std::nullopt); OPTION(std::optional, EnableAddColumsWithDefaults, std::nullopt); + OPTION(std::optional, EnableReplaceIfExists, std::nullopt); #undef OPTION }; @@ -116,7 +117,7 @@ namespace NSchemeShardUT_Private { void SimulateSleep(TTestActorRuntime& runtime, TDuration duration); - void TestServerlessComputeResourcesModeInHive(TTestActorRuntime& runtime, const TString& path, + void TestServerlessComputeResourcesModeInHive(TTestActorRuntime& runtime, const TString& path, NKikimrSubDomains::EServerlessComputeResourcesMode serverlessComputeResourcesMode, ui64 hive = TTestTxConfig::Hive); diff --git a/ydb/core/tx/schemeshard/ya.make b/ydb/core/tx/schemeshard/ya.make index 3d2a5004cc91..47d6597cbda4 100644 --- a/ydb/core/tx/schemeshard/ya.make +++ b/ydb/core/tx/schemeshard/ya.make @@ -80,6 +80,8 @@ SRCS( schemeshard__operation_memory_changes.cpp schemeshard__operation_db_changes.cpp schemeshard__operation_alter_bsv.cpp + schemeshard__operation_alter_external_data_source.cpp + schemeshard__operation_alter_external_table.cpp schemeshard__operation_alter_extsubdomain.cpp schemeshard__operation_alter_fs.cpp schemeshard__operation_alter_index.cpp From 84f575d53c663868c13a949af17f86ee22dd44bb Mon Sep 17 00:00:00 2001 From: Aleksei Uzhegov Date: Tue, 30 Jan 2024 19:04:40 +0300 Subject: [PATCH 02/11] Cleaned code #1 --- .../schemeshard__operation_alter_external_data_source.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp index 49b87d8059f5..9bca7579ba8d 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp @@ -92,7 +92,6 @@ class TAlterExternalDataSource : public TSubOperation { const TPath& dstPath, const TString& acl) { const auto checks = dstPath.Check(); - // ReSharper disable once CppExpressionWithoutSideEffects checks.IsAtLocalSchemeShard() .IsResolved() .NotUnderDeleting() From 73b8d3c4294cc5084db52a9eb4a16670dad24189 Mon Sep 17 00:00:00 2001 From: Aleksei Uzhegov Date: Wed, 31 Jan 2024 12:14:10 +0300 Subject: [PATCH 03/11] Tests #1 --- .../ut_external_data_source.cpp | 76 ++++++++++++++++ .../ut_external_table/ut_external_table.cpp | 86 +++++++++++++++++++ 2 files changed, 162 insertions(+) diff --git a/ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp b/ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp index c4c6d3dad58c..eb49133ff542 100644 --- a/ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp +++ b/ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp @@ -403,4 +403,80 @@ Y_UNIT_TEST_SUITE(TExternalDataSourceTest) { TestLs(runtime, "/MyRoot/ExternalDataSource", false, NLs::PathNotExist); } + + Y_UNIT_TEST(CreateExternalDataSourceWithOrReplace) { + TTestBasicRuntime runtime; + TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExists(true)); + ui64 txId = 100; + + TestCreateExternalDataSource(runtime, ++txId, "/MyRoot",R"( + Name: "MyExternalDataSource" + SourceType: "ObjectStorage" + Location: "https://s3.cloud.net/my_bucket" + Auth { + None { + } + } + ReplaceIfExists: true + )",{NKikimrScheme::StatusAccepted}); + + env.TestWaitNotification(runtime, txId); + + { + auto describeResult = DescribePath(runtime, "/MyRoot/MyExternalDataSource"); + TestDescribeResult(describeResult, {NLs::PathExist}); + UNIT_ASSERT(describeResult.GetPathDescription().HasExternalDataSourceDescription()); + const auto& externalDataSourceDescription = describeResult.GetPathDescription().GetExternalDataSourceDescription(); + UNIT_ASSERT_VALUES_EQUAL(externalDataSourceDescription.GetName(), "MyExternalDataSource"); + UNIT_ASSERT_VALUES_EQUAL(externalDataSourceDescription.GetSourceType(), "ObjectStorage"); + UNIT_ASSERT_VALUES_EQUAL(externalDataSourceDescription.GetVersion(), 1); + UNIT_ASSERT_VALUES_EQUAL(externalDataSourceDescription.GetLocation(), "https://s3.cloud.net/my_bucket"); + UNIT_ASSERT_EQUAL(externalDataSourceDescription.GetAuth().identity_case(), NKikimrSchemeOp::TAuth::kNone); + } + + TestCreateExternalDataSource(runtime, ++txId, "/MyRoot",R"( + Name: "MyExternalDataSource" + SourceType: "ObjectStorage" + Location: "https://s3.cloud.net/my_new_bucket" + Auth { + None { + } + } + ReplaceIfExists: true + )",{NKikimrScheme::StatusAccepted}); + env.TestWaitNotification(runtime, txId); + + { + auto describeResult = DescribePath(runtime, "/MyRoot/MyExternalDataSource"); + TestDescribeResult(describeResult, {NLs::PathExist}); + UNIT_ASSERT(describeResult.GetPathDescription().HasExternalDataSourceDescription()); + const auto& externalDataSourceDescription = describeResult.GetPathDescription().GetExternalDataSourceDescription(); + UNIT_ASSERT_VALUES_EQUAL(externalDataSourceDescription.GetName(), "MyExternalDataSource"); + UNIT_ASSERT_VALUES_EQUAL(externalDataSourceDescription.GetSourceType(), "ObjectStorage"); + UNIT_ASSERT_VALUES_EQUAL(externalDataSourceDescription.GetVersion(), 2); + UNIT_ASSERT_VALUES_EQUAL(externalDataSourceDescription.GetLocation(), "https://s3.cloud.net/my_new_bucket"); + UNIT_ASSERT_EQUAL(externalDataSourceDescription.GetAuth().identity_case(), NKikimrSchemeOp::TAuth::kNone); + } + } + + Y_UNIT_TEST(CreateExternalDataSourceWithOrReplaceShouldFailIfFeatureFlagIsNotSet) { + TTestBasicRuntime runtime; + TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExists(false)); + ui64 txId = 100; + + TestCreateExternalDataSource(runtime, ++txId, "/MyRoot",R"( + Name: "MyExternalDataSource" + SourceType: "ObjectStorage" + Location: "https://s3.cloud.net/my_bucket" + Auth { + None { + } + } + ReplaceIfExists: true + )",{{NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExists is off"}}); + + env.TestWaitNotification(runtime, txId); + + TestLs(runtime, "/MyRoot/MyExternalDataSource", false, NLs::PathNotExist); + } } diff --git a/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp b/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp index 7c8921a28139..63b4de83e840 100644 --- a/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp +++ b/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp @@ -312,4 +312,90 @@ Y_UNIT_TEST_SUITE(TExternalTableTest) { Columns { Name: "Value" Type: "Utf8"} )", {{NKikimrScheme::StatusPathDoesNotExist, "Check failed: path: '/MyRoot/ExternalDataSource1'"}}); } + + Y_UNIT_TEST(CreateExternalTableWithOrReplace) { + TTestBasicRuntime runtime; + TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExists(true)); + ui64 txId = 100; + + CreateExternalDataSource(runtime, env, ++txId); + TestCreateExternalTable(runtime, ++txId, "/MyRoot", R"( + Name: "ExternalTable" + SourceType: "General" + DataSourcePath: "/MyRoot/ExternalDataSource" + Location: "/" + Columns { Name: "key" Type: "Uint64" } + ReplaceIfExists: true + )", {NKikimrScheme::StatusAccepted}); + + env.TestWaitNotification(runtime, txId); + + { + auto describeResult = DescribePath(runtime, "/MyRoot/ExternalTable"); + TestDescribeResult(describeResult, {NLs::PathExist}); + UNIT_ASSERT(describeResult.GetPathDescription().HasExternalTableDescription()); + const auto& externalTableDescription = describeResult.GetPathDescription().GetExternalTableDescription(); + UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetName(), "ExternalTable"); + UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetDataSourcePath(), "/MyRoot/ExternalDataSource"); + UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetLocation(), "/"); + UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetSourceType(), "ObjectStorage"); + UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetVersion(), 1); + auto& columns = externalTableDescription.GetColumns(); + UNIT_ASSERT_VALUES_EQUAL(columns.size(), 1); + UNIT_ASSERT_VALUES_EQUAL(columns.Get(0).GetName(), "key"); + UNIT_ASSERT_VALUES_EQUAL(columns.Get(0).GetType(), "Uint64"); + UNIT_ASSERT_VALUES_EQUAL(columns.Get(0).GetNotNull(), false); + } + + TestCreateExternalTable(runtime, ++txId, "/MyRoot", R"( + Name: "ExternalTable" + SourceType: "General" + DataSourcePath: "/MyRoot/ExternalDataSource" + Location: "/new_location" + Columns { Name: "key" Type: "Uint64" } + Columns { Name: "value" Type: "Uint64" } + ReplaceIfExists: true + )", {NKikimrScheme::StatusAccepted}); + env.TestWaitNotification(runtime, txId); + + { + auto describeResult = DescribePath(runtime, "/MyRoot/ExternalTable"); + TestDescribeResult(describeResult, {NLs::PathExist}); + UNIT_ASSERT(describeResult.GetPathDescription().HasExternalTableDescription()); + const auto& externalTableDescription = describeResult.GetPathDescription().GetExternalTableDescription(); + UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetName(), "ExternalTable"); + UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetDataSourcePath(), "/MyRoot/ExternalDataSource"); + UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetLocation(), "/new_location"); + UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetSourceType(), "ObjectStorage"); + UNIT_ASSERT_VALUES_EQUAL(externalTableDescription.GetVersion(), 2); + auto& columns = externalTableDescription.GetColumns(); + UNIT_ASSERT_VALUES_EQUAL(columns.size(), 2); + UNIT_ASSERT_VALUES_EQUAL(columns.Get(0).GetName(), "key"); + UNIT_ASSERT_VALUES_EQUAL(columns.Get(0).GetType(), "Uint64"); + UNIT_ASSERT_VALUES_EQUAL(columns.Get(0).GetNotNull(), false); + UNIT_ASSERT_VALUES_EQUAL(columns.Get(1).GetName(), "value"); + UNIT_ASSERT_VALUES_EQUAL(columns.Get(1).GetType(), "Uint64"); + UNIT_ASSERT_VALUES_EQUAL(columns.Get(1).GetNotNull(), false); + } + } + + Y_UNIT_TEST(CreateExternalTableWithOrReplaceShouldFailIfFeatureFlagIsNotSet) { + TTestBasicRuntime runtime; + TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExists(false)); + ui64 txId = 100; + + CreateExternalDataSource(runtime, env, ++txId); + TestCreateExternalTable(runtime, ++txId, "/MyRoot", R"( + Name: "ExternalTable" + SourceType: "General" + DataSourcePath: "/MyRoot/ExternalDataSource" + Location: "/" + Columns { Name: "key" Type: "Uint64" } + ReplaceIfExists: true + )", {{NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExists is off"}}); + + env.TestWaitNotification(runtime, txId); + + TestLs(runtime, "/MyRoot/ExternalTable", false, NLs::PathNotExist); + } } From b2e29bf91ebcc69a0d3d52e6f18efe2efc466f2c Mon Sep 17 00:00:00 2001 From: Aleksei Uzhegov Date: Wed, 31 Jan 2024 14:07:57 +0300 Subject: [PATCH 04/11] Fixed PR comments #1 --- ydb/core/protos/counters_schemeshard.proto | 8 +++++- ydb/core/protos/feature_flags.proto | 2 +- ydb/core/protos/flat_scheme_op.proto | 3 +++ .../tx/schemeshard/schemeshard__operation.cpp | 12 +++++++-- ..._operation_create_external_data_source.cpp | 8 +++--- ...shard__operation_create_external_table.cpp | 8 +++--- .../schemeshard/schemeshard__operation_part.h | 12 ++++----- ...peration_replace_external_data_source.cpp} | 26 +++++++++---------- ...ard__operation_replace_external_table.cpp} | 26 +++++++++---------- .../schemeshard_audit_log_fragment.cpp | 6 ++++- ydb/core/tx/schemeshard/schemeshard_impl.cpp | 4 ++- ydb/core/tx/schemeshard/schemeshard_impl.h | 2 +- .../tx/schemeshard/schemeshard_tx_infly.h | 10 +++++++ .../ut_external_data_source.cpp | 6 ++--- .../ut_external_table/ut_external_table.cpp | 6 ++--- .../tx/schemeshard/ut_helpers/test_env.cpp | 2 +- ydb/core/tx/schemeshard/ut_helpers/test_env.h | 2 +- ydb/core/tx/schemeshard/ya.make | 4 +-- 18 files changed, 90 insertions(+), 57 deletions(-) rename ydb/core/tx/schemeshard/{schemeshard__operation_alter_external_data_source.cpp => schemeshard__operation_replace_external_data_source.cpp} (91%) rename ydb/core/tx/schemeshard/{schemeshard__operation_alter_external_table.cpp => schemeshard__operation_replace_external_table.cpp} (94%) diff --git a/ydb/core/protos/counters_schemeshard.proto b/ydb/core/protos/counters_schemeshard.proto index ca2a07a1d508..cfc2111cc4f1 100644 --- a/ydb/core/protos/counters_schemeshard.proto +++ b/ydb/core/protos/counters_schemeshard.proto @@ -194,7 +194,10 @@ enum ESimpleCounters { COUNTER_IN_FLIGHT_OPS_TxAlterView = 156 [(CounterOpts) = {Name: "InFlightOps/AlterView"}]; COUNTER_IN_FLIGHT_OPS_TxDropView = 157 [(CounterOpts) = {Name: "InFlightOps/DropView"}]; - COUNTER_GRAPHSHARD_COUNT = 158 [(CounterOpts) = {Name: "GraphShards"}]; + COUNTER_IN_FLIGHT_OPS_TxReplaceExternalTable = 158 [(CounterOpts) = {Name: "InFlightOps/ReplaceExternalTable"}]; + COUNTER_IN_FLIGHT_OPS_TxReplaceExternalDataSource = 159 [(CounterOpts) = {Name: "InFlightOps/ReplaceExternalDataSource"}]; + + COUNTER_GRAPHSHARD_COUNT = 160 [(CounterOpts) = {Name: "GraphShards"}]; } enum ECumulativeCounters { @@ -315,6 +318,9 @@ enum ECumulativeCounters { COUNTER_FINISHED_OPS_TxCreateView = 95 [(CounterOpts) = {Name: "FinishedOps/CreateView"}]; COUNTER_FINISHED_OPS_TxDropView = 96 [(CounterOpts) = {Name: "FinishedOps/DropView"}]; COUNTER_FINISHED_OPS_TxAlterView = 97 [(CounterOpts) = {Name: "FinishedOps/AlterView"}]; + + COUNTER_FINISHED_OPS_TxReplaceExternalTable = 98 [(CounterOpts) = {Name: "FinishedOps/ReplaceExternalTable"}]; + COUNTER_FINISHED_OPS_TxReplaceExternalDataSource = 99 [(CounterOpts) = {Name: "FinishedOps/ReplaceExternalDataSource"}]; } enum EPercentileCounters { diff --git a/ydb/core/protos/feature_flags.proto b/ydb/core/protos/feature_flags.proto index 042c2e7b5cf3..7876feeb65fe 100644 --- a/ydb/core/protos/feature_flags.proto +++ b/ydb/core/protos/feature_flags.proto @@ -128,5 +128,5 @@ message TFeatureFlags { optional bool EnableServerlessExclusiveDynamicNodes = 113 [default = false]; optional bool EnableAccessServiceBulkAuthorization = 114 [default = false]; optional bool EnableAddColumsWithDefaults = 115 [ default = false]; - optional bool EnableReplaceIfExists = 116 [ default = false]; + optional bool EnableReplaceIfExistsForExternalEntities = 116 [ default = false]; } diff --git a/ydb/core/protos/flat_scheme_op.proto b/ydb/core/protos/flat_scheme_op.proto index 361006fcc062..26dedd92ffc5 100644 --- a/ydb/core/protos/flat_scheme_op.proto +++ b/ydb/core/protos/flat_scheme_op.proto @@ -1431,6 +1431,9 @@ enum EOperationType { ESchemeOpCreateView = 93; ESchemeOpAlterView = 94; ESchemeOpDropView = 95; + + ESchemeOpReplaceExternalDataSource = 96; + ESchemeOpReplaceExternalTable = 97; } message TApplyIf { diff --git a/ydb/core/tx/schemeshard/schemeshard__operation.cpp b/ydb/core/tx/schemeshard/schemeshard__operation.cpp index 676771621098..49bda89c53ec 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation.cpp @@ -1061,14 +1061,18 @@ ISubOperation::TPtr TOperation::RestorePart(TTxState::ETxType txType, TTxState:: return CreateNewExternalTable(NextPartId(), txState); case TTxState::ETxType::TxDropExternalTable: return CreateDropExternalTable(NextPartId(), txState); + case TTxState::ETxType::TxReplaceExternalTable: + return CreateReplaceExternalTable(NextPartId(), txState); case TTxState::ETxType::TxAlterExternalTable: - return CreateAlterExternalTable(NextPartId(), txState); + Y_ABORT("TODO: implement"); case TTxState::ETxType::TxCreateExternalDataSource: return CreateNewExternalDataSource(NextPartId(), txState); case TTxState::ETxType::TxDropExternalDataSource: return CreateDropExternalDataSource(NextPartId(), txState); + case TTxState::ETxType::TxReplaceExternalDataSource: + return CreateReplaceExternalDataSource(NextPartId(), txState); case TTxState::ETxType::TxAlterExternalDataSource: - return CreateAlterExternalDataSource(NextPartId(), txState); + Y_ABORT("TODO: implement"); // View case TTxState::ETxType::TxCreateView: @@ -1281,6 +1285,8 @@ ISubOperation::TPtr TOperation::ConstructPart(NKikimrSchemeOp::EOperationType op return CreateDropBlobDepot(NextPartId(), tx); // ExternalTable + case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalTable: + Y_ABORT("this operation must be constructed by ESchemeOpCreateExternalTable"); case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalTable: Y_ABORT("operation is handled before"); case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalTable: @@ -1289,6 +1295,8 @@ ISubOperation::TPtr TOperation::ConstructPart(NKikimrSchemeOp::EOperationType op Y_ABORT("TODO: implement"); // ExternalDataSource + case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalDataSource: + Y_ABORT("this operation must be constructed by ESchemeOpCreateExternalTable"); case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalDataSource: Y_ABORT("operation is handled before"); case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalDataSource: diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp index 5851800b0e03..eb436ced7220 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp @@ -304,7 +304,7 @@ TVector CreateNewExternalDataSource(TOperationId id, Y_ABORT_UNLESS(tx.GetOperationType() == NKikimrSchemeOp::ESchemeOpCreateExternalDataSource); LOG_I("CreateNewExternalDataSource, opId " << id << ", feature flag EnableOrReplace " - << context.SS->EnableReplaceIfExists << ", tx " + << context.SS->EnableReplaceIfExistsForExternalEntities << ", tx " << tx.ShortDebugString()); auto errorResult = [&id](NKikimrScheme::EStatus status, const TStringBuf& msg) -> TVector { @@ -315,8 +315,8 @@ TVector CreateNewExternalDataSource(TOperationId id, const auto replaceIfExists = operation.GetReplaceIfExists(); const TString& name = operation.GetName(); - if (replaceIfExists && !context.SS->EnableReplaceIfExists) { - return errorResult(NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExists is off"); + if (replaceIfExists && !context.SS->EnableReplaceIfExistsForExternalEntities) { + return errorResult(NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExistsForExternalEntities is off"); } const TString& parentPathStr = tx.GetWorkingDir(); @@ -342,7 +342,7 @@ TVector CreateNewExternalDataSource(TOperationId id, .IsResolved() .NotUnderDeleting(); if (isAlreadyExists) { - return {CreateAlterExternalDataSource(id, tx)}; + return {CreateReplaceExternalDataSource(id, tx)}; } } diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp index 67000675e83d..b59c4319de3b 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp @@ -397,7 +397,7 @@ TVector CreateNewExternalTable(TOperationId id, const TTxTr Y_ABORT_UNLESS(tx.GetOperationType() == NKikimrSchemeOp::ESchemeOpCreateExternalTable); LOG_I("CreateNewExternalTable, opId " << id << ", feature flag EnableOrReplace " - << context.SS->EnableReplaceIfExists << ", tx " + << context.SS->EnableReplaceIfExistsForExternalEntities << ", tx " << tx.ShortDebugString()); auto errorResult = [&id](NKikimrScheme::EStatus status, const TStringBuf& msg) -> TVector { @@ -408,8 +408,8 @@ TVector CreateNewExternalTable(TOperationId id, const TTxTr const auto replaceIfExists = operation.GetReplaceIfExists(); const TString& name = operation.GetName(); - if (replaceIfExists && !context.SS->EnableReplaceIfExists) { - return errorResult(NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExists is off"); + if (replaceIfExists && !context.SS->EnableReplaceIfExistsForExternalEntities) { + return errorResult(NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExistsForExternalEntities is off"); } const TString& parentPathStr = tx.GetWorkingDir(); @@ -435,7 +435,7 @@ TVector CreateNewExternalTable(TOperationId id, const TTxTr .IsResolved() .NotUnderDeleting(); if (isAlreadyExists) { - return {CreateAlterExternalTable(id, tx)}; + return {CreateReplaceExternalTable(id, tx)}; } } diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_part.h b/ydb/core/tx/schemeshard/schemeshard__operation_part.h index 0a9cf8b6b8b3..c0367b99b699 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_part.h +++ b/ydb/core/tx/schemeshard/schemeshard__operation_part.h @@ -371,9 +371,9 @@ ISubOperation::TPtr CreateUpdateMainTableOnIndexMove(TOperationId id, TTxState:: // Create TVector CreateNewExternalTable(TOperationId id, const TTxTransaction& tx, TOperationContext& context); ISubOperation::TPtr CreateNewExternalTable(TOperationId id, TTxState::ETxState state); -// Alter -ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, const TTxTransaction& tx); -ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, TTxState::ETxState state); +// Replace +ISubOperation::TPtr CreateReplaceExternalTable(TOperationId id, const TTxTransaction& tx); +ISubOperation::TPtr CreateReplaceExternalTable(TOperationId id, TTxState::ETxState state); // Drop ISubOperation::TPtr CreateDropExternalTable(TOperationId id, const TTxTransaction& tx); ISubOperation::TPtr CreateDropExternalTable(TOperationId id, TTxState::ETxState state); @@ -382,9 +382,9 @@ ISubOperation::TPtr CreateDropExternalTable(TOperationId id, TTxState::ETxState // Create TVector CreateNewExternalDataSource(TOperationId id, const TTxTransaction& tx, TOperationContext& context); ISubOperation::TPtr CreateNewExternalDataSource(TOperationId id, TTxState::ETxState state); -// Alter -ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, const TTxTransaction& tx); -ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, TTxState::ETxState state); +// Replace +ISubOperation::TPtr CreateReplaceExternalDataSource(TOperationId id, const TTxTransaction& tx); +ISubOperation::TPtr CreateReplaceExternalDataSource(TOperationId id, TTxState::ETxState state); // Drop ISubOperation::TPtr CreateDropExternalDataSource(TOperationId id, const TTxTransaction& tx); ISubOperation::TPtr CreateDropExternalDataSource(TOperationId id, TTxState::ETxState state); diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_replace_external_data_source.cpp similarity index 91% rename from ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp rename to ydb/core/tx/schemeshard/schemeshard__operation_replace_external_data_source.cpp index 9bca7579ba8d..174dc1202794 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_replace_external_data_source.cpp @@ -16,7 +16,7 @@ class TPropose: public TSubOperationState { TString DebugHint() const override { return TStringBuilder() - << "TAlterExternalDataSource TPropose" + << "TReplaceExternalDataSource TPropose" << ", operationId: " << OperationId; } @@ -34,7 +34,7 @@ class TPropose: public TSubOperationState { const TTxState* txState = context.SS->FindTx(OperationId); Y_ABORT_UNLESS(txState); - Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalDataSource); + Y_ABORT_UNLESS(txState->TxType == TTxState::TxReplaceExternalDataSource); const auto pathId = txState->TargetPathId; const auto path = TPath::Init(pathId, context.SS); @@ -56,14 +56,14 @@ class TPropose: public TSubOperationState { const TTxState* txState = context.SS->FindTx(OperationId); Y_ABORT_UNLESS(txState); - Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalDataSource); + Y_ABORT_UNLESS(txState->TxType == TTxState::TxReplaceExternalDataSource); context.OnComplete.ProposeToCoordinator(OperationId, txState->TargetPathId, TStepId(0)); return false; } }; -class TAlterExternalDataSource : public TSubOperation { +class TReplaceExternalDataSource : public TSubOperation { static TTxState::ETxState NextState() { return TTxState::Propose; } TTxState::ETxState NextState(TTxState::ETxState state) const override { @@ -152,7 +152,7 @@ class TAlterExternalDataSource : public TSubOperation { void CreateTransaction(const TOperationContext& context, const TPathId& externalDataSourcePathId) const { TTxState& txState = context.SS->CreateTx(OperationId, - TTxState::TxAlterExternalDataSource, + TTxState::TxReplaceExternalDataSource, externalDataSourcePathId); txState.Shards.clear(); } @@ -207,7 +207,7 @@ class TAlterExternalDataSource : public TSubOperation { Transaction.GetCreateExternalDataSource(); const TString& name = externalDataSourceDescription.GetName(); - LOG_N("TAlterExternalDataSource Propose" + LOG_N("TReplaceExternalDataSource Propose" << ": opId# " << OperationId << ", path# " << parentPathStr << "/" << name); auto result = MakeHolder(NKikimrScheme::StatusAccepted, @@ -258,13 +258,13 @@ class TAlterExternalDataSource : public TSubOperation { } void AbortPropose(TOperationContext& context) override { - LOG_N("TAlterExternalDataSource AbortPropose" + LOG_N("TReplaceExternalDataSource AbortPropose" << ": opId# " << OperationId); - Y_ABORT("no AbortPropose for TAlterExternalDataSource"); + Y_ABORT("no AbortPropose for TReplaceExternalDataSource"); } void AbortUnsafe(TTxId forceDropTxId, TOperationContext& context) override { - LOG_N("TAlterExternalDataSource AbortUnsafe" + LOG_N("TReplaceExternalDataSource AbortUnsafe" << ": opId# " << OperationId << ", txId# " << forceDropTxId); context.OnComplete.DoneOperation(OperationId); } @@ -274,13 +274,13 @@ class TAlterExternalDataSource : public TSubOperation { namespace NKikimr::NSchemeShard { -ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, const TTxTransaction& tx) { - return MakeSubOperation(id, tx); +ISubOperation::TPtr CreateReplaceExternalDataSource(TOperationId id, const TTxTransaction& tx) { + return MakeSubOperation(id, tx); } -ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, TTxState::ETxState state) { +ISubOperation::TPtr CreateReplaceExternalDataSource(TOperationId id, TTxState::ETxState state) { Y_ABORT_UNLESS(state != TTxState::Invalid); - return MakeSubOperation(id, state); + return MakeSubOperation(id, state); } } diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_replace_external_table.cpp similarity index 94% rename from ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp rename to ydb/core/tx/schemeshard/schemeshard__operation_replace_external_table.cpp index 9498102bdcb5..7e93bd78e0b2 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_replace_external_table.cpp @@ -18,7 +18,7 @@ class TPropose: public TSubOperationState { TString DebugHint() const override { return TStringBuilder() - << "TAlterExternalTable TPropose" + << "TReplaceExternalTable TPropose" << ", operationId: " << OperationId; } @@ -59,7 +59,7 @@ class TPropose: public TSubOperationState { const TTxState* txState = context.SS->FindTx(OperationId); Y_ABORT_UNLESS(txState); - Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalTable); + Y_ABORT_UNLESS(txState->TxType == TTxState::TxReplaceExternalTable); const auto pathId = txState->TargetPathId; const auto dataSourcePathId = txState->SourcePathId; @@ -88,7 +88,7 @@ class TPropose: public TSubOperationState { const TTxState* txState = context.SS->FindTx(OperationId); Y_ABORT_UNLESS(txState); - Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalTable); + Y_ABORT_UNLESS(txState->TxType == TTxState::TxReplaceExternalTable); context.OnComplete.ProposeToCoordinator(OperationId, txState->TargetPathId, TStepId(0)); return false; @@ -96,7 +96,7 @@ class TPropose: public TSubOperationState { }; -class TAltertExternalTable: public TSubOperation { +class TReplacetExternalTable: public TSubOperation { private: bool IsSameDataSource = true; TPathId OldDataSourcePathId = InvalidPathId; @@ -220,7 +220,7 @@ class TAltertExternalTable: public TSubOperation { const TPathId& externalTablePathId, const TPathId& externalDataSourcePathId) const { TTxState& txState = context.SS->CreateTx(OperationId, - TTxState::TxAlterExternalTable, + TTxState::TxReplaceExternalTable, externalTablePathId, externalDataSourcePathId); txState.Shards.clear(); @@ -301,7 +301,7 @@ class TAltertExternalTable: public TSubOperation { const auto& externalTableDescription = Transaction.GetCreateExternalTable(); const TString& name = externalTableDescription.GetName(); - LOG_N("TAlterExternalTable Propose" + LOG_N("TReplaceExternalTable Propose" << ": opId# " << OperationId << ", path# " << parentPathStr << "/" << name << ", ReplaceIfExists:" << externalTableDescription.GetReplaceIfExists()); @@ -392,13 +392,13 @@ class TAltertExternalTable: public TSubOperation { } void AbortPropose(TOperationContext& context) override { - LOG_N("TAlterExternalTable AbortPropose" + LOG_N("TReplaceExternalTable AbortPropose" << ": opId# " << OperationId); - Y_ABORT("no AbortPropose for TAlterExternalTable"); + Y_ABORT("no AbortPropose for TReplaceExternalTable"); } void AbortUnsafe(TTxId forceDropTxId, TOperationContext& context) override { - LOG_N("TAlterExternalTable AbortUnsafe" + LOG_N("TReplaceExternalTable AbortUnsafe" << ": opId# " << OperationId << ", txId# " << forceDropTxId); context.OnComplete.DoneOperation(OperationId); @@ -409,13 +409,13 @@ class TAltertExternalTable: public TSubOperation { namespace NKikimr::NSchemeShard { -ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, const TTxTransaction& tx) { - return MakeSubOperation(std::move(id), tx); +ISubOperation::TPtr CreateReplaceExternalTable(TOperationId id, const TTxTransaction& tx) { + return MakeSubOperation(std::move(id), tx); } -ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, TTxState::ETxState state) { +ISubOperation::TPtr CreateReplaceExternalTable(TOperationId id, TTxState::ETxState state) { Y_ABORT_UNLESS(state != TTxState::Invalid); - return MakeSubOperation(std::move(id), state); + return MakeSubOperation(std::move(id), state); } } diff --git a/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp b/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp index 7a31db701843..f5963d5e433c 100644 --- a/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp @@ -208,12 +208,14 @@ TString DefineUserOperationName(const NKikimrSchemeOp::TModifyScheme& tx) { return "ALTER BLOB DEPOT"; case NKikimrSchemeOp::EOperationType::ESchemeOpDropBlobDepot: return "DROP BLOB DEPOT"; + case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalTable: case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalTable: return "CREATE EXTERNAL TABLE"; case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalTable: return "DROP EXTERNAL TABLE"; case NKikimrSchemeOp::EOperationType::ESchemeOpAlterExternalTable: return "ALTER EXTERNAL TABLE"; + case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalDataSource: case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalDataSource: return "CREATE EXTERNAL DATA SOURCE"; case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalDataSource: @@ -487,6 +489,7 @@ TVector ExtractChangingPaths(const NKikimrSchemeOp::TModifyScheme& tx) result.emplace_back(NKikimr::JoinPath({tx.GetMoveIndex().GetTablePath(), tx.GetMoveIndex().GetSrcPath()})); result.emplace_back(NKikimr::JoinPath({tx.GetMoveIndex().GetTablePath(), tx.GetMoveIndex().GetDstPath()})); break; + case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalTable: case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalTable: result.emplace_back(NKikimr::JoinPath({tx.GetWorkingDir(), tx.GetCreateExternalTable().GetName()})); break; @@ -496,6 +499,7 @@ TVector ExtractChangingPaths(const NKikimrSchemeOp::TModifyScheme& tx) case NKikimrSchemeOp::EOperationType::ESchemeOpAlterExternalTable: // TODO: unimplemented break; + case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalDataSource: case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalDataSource: result.emplace_back(NKikimr::JoinPath({tx.GetWorkingDir(), tx.GetCreateExternalDataSource().GetName()})); break; @@ -636,7 +640,7 @@ TAuditLogFragment MakeAuditLogFragment(const NKikimrSchemeOp::TModifyScheme& tx) auto [aclAdd, aclRemove] = ExtractACLChange(tx); auto [userAttrsAdd, userAttrsRemove] = ExtractUserAttrChange(tx); auto [loginUser, loginGroup, loginMember] = ExtractLoginChange(tx); - + return { .Operation = DefineUserOperationName(tx), .Paths = ExtractChangingPaths(tx), diff --git a/ydb/core/tx/schemeshard/schemeshard_impl.cpp b/ydb/core/tx/schemeshard/schemeshard_impl.cpp index 7a479a11213d..826d31c78dc6 100644 --- a/ydb/core/tx/schemeshard/schemeshard_impl.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_impl.cpp @@ -1455,6 +1455,8 @@ TPathElement::EPathState TSchemeShard::CalcPathState(TTxState::ETxType txType, T case TTxState::TxUpdateMainTableOnIndexMove: case TTxState::TxAlterExternalTable: case TTxState::TxAlterExternalDataSource: + case TTxState::TxReplaceExternalTable: + case TTxState::TxReplaceExternalDataSource: case TTxState::TxAlterView: return TPathElement::EPathState::EPathStateAlter; case TTxState::TxDropTable: @@ -4297,7 +4299,7 @@ void TSchemeShard::OnActivateExecutor(const TActorContext &ctx) { EnableTablePgTypes = appData->FeatureFlags.GetEnableTablePgTypes(); EnableServerlessExclusiveDynamicNodes = appData->FeatureFlags.GetEnableServerlessExclusiveDynamicNodes(); EnableAddColumsWithDefaults = appData->FeatureFlags.GetEnableAddColumsWithDefaults(); - EnableReplaceIfExists = appData->FeatureFlags.GetEnableReplaceIfExists(); + EnableReplaceIfExistsForExternalEntities = appData->FeatureFlags.GetEnableReplaceIfExistsForExternalEntities(); ConfigureCompactionQueues(appData->CompactionConfig, ctx); ConfigureStatsBatching(appData->SchemeShardConfig, ctx); diff --git a/ydb/core/tx/schemeshard/schemeshard_impl.h b/ydb/core/tx/schemeshard/schemeshard_impl.h index f0c80259a28c..435c74eff807 100644 --- a/ydb/core/tx/schemeshard/schemeshard_impl.h +++ b/ydb/core/tx/schemeshard/schemeshard_impl.h @@ -272,7 +272,7 @@ class TSchemeShard bool EnableTablePgTypes = false; bool EnableServerlessExclusiveDynamicNodes = false; bool EnableAddColumsWithDefaults = false; - bool EnableReplaceIfExists = true; + bool EnableReplaceIfExistsForExternalEntities = false; TShardDeleter ShardDeleter; diff --git a/ydb/core/tx/schemeshard/schemeshard_tx_infly.h b/ydb/core/tx/schemeshard/schemeshard_tx_infly.h index 849f91a9dee0..3ea7e75a3d68 100644 --- a/ydb/core/tx/schemeshard/schemeshard_tx_infly.h +++ b/ydb/core/tx/schemeshard/schemeshard_tx_infly.h @@ -129,6 +129,8 @@ struct TTxState { item(TxCreateView, 83) \ item(TxAlterView, 84) \ item(TxDropView, 85) \ + item(TxReplaceExternalDataSource, 86) \ + item(TxReplaceExternalTable, 87) \ // TX_STATE_TYPE_ENUM @@ -405,6 +407,8 @@ struct TTxState { case TxAlterBlobDepot: case TxAlterExternalTable: case TxAlterExternalDataSource: + case TxReplaceExternalDataSource: + case TxReplaceExternalTable: case TxAlterView: return false; case TxMoveTable: @@ -502,6 +506,8 @@ struct TTxState { case TxAlterBlobDepot: case TxAlterExternalTable: case TxAlterExternalDataSource: + case TxReplaceExternalDataSource: + case TxReplaceExternalTable: case TxAlterView: return false; case TxMoveTable: @@ -602,6 +608,8 @@ struct TTxState { case TxAlterBlobDepot: case TxAlterExternalTable: case TxAlterExternalDataSource: + case TxReplaceExternalDataSource: + case TxReplaceExternalTable: case TxAlterView: return false; case TxInvalid: @@ -698,8 +706,10 @@ struct TTxState { case NKikimrSchemeOp::ESchemeOpDeallocatePersQueueGroup: return TxInvalid; case NKikimrSchemeOp::ESchemeOpCreateExternalTable: return TxCreateExternalTable; case NKikimrSchemeOp::ESchemeOpAlterExternalTable: return TxAlterExternalTable; + case NKikimrSchemeOp::ESchemeOpReplaceExternalTable: return TxReplaceExternalTable; case NKikimrSchemeOp::ESchemeOpCreateExternalDataSource: return TxCreateExternalDataSource; case NKikimrSchemeOp::ESchemeOpAlterExternalDataSource: return TxAlterExternalDataSource; + case NKikimrSchemeOp::ESchemeOpReplaceExternalDataSource: return TxReplaceExternalDataSource; case NKikimrSchemeOp::ESchemeOpCreateView: return TxCreateView; case NKikimrSchemeOp::ESchemeOpAlterView: return TxAlterView; case NKikimrSchemeOp::ESchemeOpDropView: return TxDropView; diff --git a/ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp b/ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp index eb49133ff542..6f791e96d82b 100644 --- a/ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp +++ b/ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp @@ -406,7 +406,7 @@ Y_UNIT_TEST_SUITE(TExternalDataSourceTest) { Y_UNIT_TEST(CreateExternalDataSourceWithOrReplace) { TTestBasicRuntime runtime; - TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExists(true)); + TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExistsForExternalEntities(true)); ui64 txId = 100; TestCreateExternalDataSource(runtime, ++txId, "/MyRoot",R"( @@ -461,7 +461,7 @@ Y_UNIT_TEST_SUITE(TExternalDataSourceTest) { Y_UNIT_TEST(CreateExternalDataSourceWithOrReplaceShouldFailIfFeatureFlagIsNotSet) { TTestBasicRuntime runtime; - TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExists(false)); + TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExistsForExternalEntities(false)); ui64 txId = 100; TestCreateExternalDataSource(runtime, ++txId, "/MyRoot",R"( @@ -473,7 +473,7 @@ Y_UNIT_TEST_SUITE(TExternalDataSourceTest) { } } ReplaceIfExists: true - )",{{NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExists is off"}}); + )",{{NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExistsForExternalEntities is off"}}); env.TestWaitNotification(runtime, txId); diff --git a/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp b/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp index 63b4de83e840..90d5f2237c98 100644 --- a/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp +++ b/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp @@ -315,7 +315,7 @@ Y_UNIT_TEST_SUITE(TExternalTableTest) { Y_UNIT_TEST(CreateExternalTableWithOrReplace) { TTestBasicRuntime runtime; - TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExists(true)); + TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExistsForExternalEntities(true)); ui64 txId = 100; CreateExternalDataSource(runtime, env, ++txId); @@ -381,7 +381,7 @@ Y_UNIT_TEST_SUITE(TExternalTableTest) { Y_UNIT_TEST(CreateExternalTableWithOrReplaceShouldFailIfFeatureFlagIsNotSet) { TTestBasicRuntime runtime; - TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExists(false)); + TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExistsForExternalEntities(false)); ui64 txId = 100; CreateExternalDataSource(runtime, env, ++txId); @@ -392,7 +392,7 @@ Y_UNIT_TEST_SUITE(TExternalTableTest) { Location: "/" Columns { Name: "key" Type: "Uint64" } ReplaceIfExists: true - )", {{NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExists is off"}}); + )", {{NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExistsForExternalEntities is off"}}); env.TestWaitNotification(runtime, txId); diff --git a/ydb/core/tx/schemeshard/ut_helpers/test_env.cpp b/ydb/core/tx/schemeshard/ut_helpers/test_env.cpp index 94498c38da1b..1247239013b4 100644 --- a/ydb/core/tx/schemeshard/ut_helpers/test_env.cpp +++ b/ydb/core/tx/schemeshard/ut_helpers/test_env.cpp @@ -537,7 +537,7 @@ NSchemeShardUT_Private::TTestEnv::TTestEnv(TTestActorRuntime& runtime, const TTe app.SetEnableTablePgTypes(opts.EnableTablePgTypes_); app.SetEnableServerlessExclusiveDynamicNodes(opts.EnableServerlessExclusiveDynamicNodes_); app.SetEnableAddColumsWithDefaults(opts.EnableAddColumsWithDefaults_); - app.SetEnableReplaceIfExists(opts.EnableReplaceIfExists_); + app.SetEnableReplaceIfExistsForExternalEntities(opts.EnableReplaceIfExistsForExternalEntities_); app.ColumnShardConfig.SetDisabledOnSchemeShard(false); diff --git a/ydb/core/tx/schemeshard/ut_helpers/test_env.h b/ydb/core/tx/schemeshard/ut_helpers/test_env.h index 52e4ffce501a..9c300ee098be 100644 --- a/ydb/core/tx/schemeshard/ut_helpers/test_env.h +++ b/ydb/core/tx/schemeshard/ut_helpers/test_env.h @@ -59,7 +59,7 @@ namespace NSchemeShardUT_Private { OPTION(std::optional, EnableTablePgTypes, std::nullopt); OPTION(std::optional, EnableServerlessExclusiveDynamicNodes, std::nullopt); OPTION(std::optional, EnableAddColumsWithDefaults, std::nullopt); - OPTION(std::optional, EnableReplaceIfExists, std::nullopt); + OPTION(std::optional, EnableReplaceIfExistsForExternalEntities, std::nullopt); #undef OPTION }; diff --git a/ydb/core/tx/schemeshard/ya.make b/ydb/core/tx/schemeshard/ya.make index 47d6597cbda4..095ada7cbf60 100644 --- a/ydb/core/tx/schemeshard/ya.make +++ b/ydb/core/tx/schemeshard/ya.make @@ -80,8 +80,6 @@ SRCS( schemeshard__operation_memory_changes.cpp schemeshard__operation_db_changes.cpp schemeshard__operation_alter_bsv.cpp - schemeshard__operation_alter_external_data_source.cpp - schemeshard__operation_alter_external_table.cpp schemeshard__operation_alter_extsubdomain.cpp schemeshard__operation_alter_fs.cpp schemeshard__operation_alter_index.cpp @@ -144,6 +142,8 @@ SRCS( schemeshard__operation_move_table_index.cpp schemeshard__operation_part.cpp schemeshard__operation_part.h + schemeshard__operation_replace_external_data_source.cpp + schemeshard__operation_replace_external_table.cpp schemeshard__operation_rmdir.cpp schemeshard__operation_split_merge.cpp schemeshard__operation_just_reject.cpp From eff009dc8882a34c09ae770190347c38d2a5e817 Mon Sep 17 00:00:00 2001 From: Aleksei Uzhegov Date: Wed, 31 Jan 2024 14:12:03 +0300 Subject: [PATCH 05/11] Fixed PR comments #2 --- ydb/core/testlib/basics/feature_flags.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/core/testlib/basics/feature_flags.h b/ydb/core/testlib/basics/feature_flags.h index 73d0d43a7eb8..5e82db9e8f28 100644 --- a/ydb/core/testlib/basics/feature_flags.h +++ b/ydb/core/testlib/basics/feature_flags.h @@ -57,7 +57,7 @@ class TTestFeatureFlagsHolder { FEATURE_FLAG_SETTER(EnableServerlessExclusiveDynamicNodes) FEATURE_FLAG_SETTER(EnableAccessServiceBulkAuthorization) FEATURE_FLAG_SETTER(EnableAddColumsWithDefaults) - FEATURE_FLAG_SETTER(EnableReplaceIfExists) + FEATURE_FLAG_SETTER(EnableReplaceIfExistsForExternalEntities) #undef FEATURE_FLAG_SETTER }; From 0d715a851b4f41b3fd7251a3bcb7ec1920ec3d5f Mon Sep 17 00:00:00 2001 From: Alexey Uzhegov Date: Wed, 31 Jan 2024 14:44:23 +0300 Subject: [PATCH 06/11] Update ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp Co-authored-by: Vasily Gerasimov --- ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp b/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp index 90d5f2237c98..9b3babbc5b69 100644 --- a/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp +++ b/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp @@ -313,7 +313,7 @@ Y_UNIT_TEST_SUITE(TExternalTableTest) { )", {{NKikimrScheme::StatusPathDoesNotExist, "Check failed: path: '/MyRoot/ExternalDataSource1'"}}); } - Y_UNIT_TEST(CreateExternalTableWithOrReplace) { + Y_UNIT_TEST(ReplaceExternalTableIfNotExists) { TTestBasicRuntime runtime; TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExistsForExternalEntities(true)); ui64 txId = 100; From 37e2203e1f4aab0c9bc8c44a22b07f43e67412c2 Mon Sep 17 00:00:00 2001 From: Alexey Uzhegov Date: Wed, 31 Jan 2024 14:44:48 +0300 Subject: [PATCH 07/11] Update ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp Co-authored-by: Vasily Gerasimov --- ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp b/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp index 9b3babbc5b69..bcb9809c5531 100644 --- a/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp +++ b/ydb/core/tx/schemeshard/ut_external_table/ut_external_table.cpp @@ -379,7 +379,7 @@ Y_UNIT_TEST_SUITE(TExternalTableTest) { } } - Y_UNIT_TEST(CreateExternalTableWithOrReplaceShouldFailIfFeatureFlagIsNotSet) { + Y_UNIT_TEST(ReplaceExternalTableIfNotExistsShouldFailIfFeatureFlagIsNotSet) { TTestBasicRuntime runtime; TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExistsForExternalEntities(false)); ui64 txId = 100; From 249a4cc36f3070209baab1c30a516b595eef45d8 Mon Sep 17 00:00:00 2001 From: Aleksei Uzhegov Date: Wed, 31 Jan 2024 14:44:59 +0300 Subject: [PATCH 08/11] Fixed PR comments #3 --- ..._operation_create_external_data_source.cpp | 20 +++++++---------- ...shard__operation_create_external_table.cpp | 22 +++++++------------ ydb/core/tx/schemeshard/schemeshard_path.cpp | 4 ++-- 3 files changed, 18 insertions(+), 28 deletions(-) diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp index eb436ced7220..03e9dccf0392 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp @@ -303,7 +303,7 @@ TVector CreateNewExternalDataSource(TOperationId id, TOperationContext& context) { Y_ABORT_UNLESS(tx.GetOperationType() == NKikimrSchemeOp::ESchemeOpCreateExternalDataSource); - LOG_I("CreateNewExternalDataSource, opId " << id << ", feature flag EnableOrReplace " + LOG_I("CreateNewExternalDataSource, opId " << id << ", feature flag EnableReplaceIfExistsForExternalEntities " << context.SS->EnableReplaceIfExistsForExternalEntities << ", tx " << tx.ShortDebugString()); @@ -311,9 +311,9 @@ TVector CreateNewExternalDataSource(TOperationId id, return {CreateReject(id, status, TStringBuilder() << "Invalid TCreateExternalDataSource request: " << msg)}; }; - const auto& operation = tx.GetCreateExternalDataSource(); + const auto &operation = tx.GetCreateExternalDataSource(); const auto replaceIfExists = operation.GetReplaceIfExists(); - const TString& name = operation.GetName(); + const TString &name = operation.GetName(); if (replaceIfExists && !context.SS->EnableReplaceIfExistsForExternalEntities) { return errorResult(NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExistsForExternalEntities is off"); @@ -329,18 +329,14 @@ TVector CreateNewExternalDataSource(TOperationId id, } } - const TPath dstPath = parentPath.Child(name); - const auto checks = dstPath.Check() - .IsAtLocalSchemeShard(); - if (!checks) { - return errorResult(checks.GetStatus(), checks.GetError()); - } if (replaceIfExists) { - const auto isAlreadyExists = checks - .IsResolved() - .NotUnderDeleting(); + const TPath dstPath = parentPath.Child(name); + const auto isAlreadyExists = + dstPath.Check() + .IsResolved() + .NotUnderDeleting(); if (isAlreadyExists) { return {CreateReplaceExternalDataSource(id, tx)}; } diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp index b59c4319de3b..e39a1ee7525f 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp @@ -396,7 +396,7 @@ namespace NKikimr::NSchemeShard { TVector CreateNewExternalTable(TOperationId id, const TTxTransaction& tx, TOperationContext& context) { Y_ABORT_UNLESS(tx.GetOperationType() == NKikimrSchemeOp::ESchemeOpCreateExternalTable); - LOG_I("CreateNewExternalTable, opId " << id << ", feature flag EnableOrReplace " + LOG_I("CreateNewExternalTable, opId " << id << ", feature flag EnableReplaceIfExistsForExternalEntities " << context.SS->EnableReplaceIfExistsForExternalEntities << ", tx " << tx.ShortDebugString()); @@ -404,9 +404,9 @@ TVector CreateNewExternalTable(TOperationId id, const TTxTr return {CreateReject(id, status, TStringBuilder() << "Invalid TCreateExternalTable request: " << msg)}; }; - const auto& operation = tx.GetCreateExternalTable(); + const auto &operation = tx.GetCreateExternalTable(); const auto replaceIfExists = operation.GetReplaceIfExists(); - const TString& name = operation.GetName(); + const TString &name = operation.GetName(); if (replaceIfExists && !context.SS->EnableReplaceIfExistsForExternalEntities) { return errorResult(NKikimrScheme::StatusPreconditionFailed, "Unsupported: feature flag EnableReplaceIfExistsForExternalEntities is off"); @@ -422,18 +422,12 @@ TVector CreateNewExternalTable(TOperationId id, const TTxTr } } - const TPath dstPath = parentPath.Child(name); - - const auto checks = dstPath.Check() - .IsAtLocalSchemeShard(); - if (!checks) { - return errorResult(checks.GetStatus(), checks.GetError()); - } - if (replaceIfExists) { - const auto isAlreadyExists = checks - .IsResolved() - .NotUnderDeleting(); + const TPath dstPath = parentPath.Child(name); + const auto isAlreadyExists = + dstPath.Check() + .IsResolved() + .NotUnderDeleting(); if (isAlreadyExists) { return {CreateReplaceExternalTable(id, tx)}; } diff --git a/ydb/core/tx/schemeshard/schemeshard_path.cpp b/ydb/core/tx/schemeshard/schemeshard_path.cpp index 04fccfece46d..cf5674c4ad1e 100644 --- a/ydb/core/tx/schemeshard/schemeshard_path.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_path.cpp @@ -585,7 +585,7 @@ const TPath::TChecker& TPath::TChecker::FailOnWrongType(const TSetIsCreateFinished()) { - return Fail(EStatus::StatusMultipleModifications, TStringBuilder() << "path exist but creating right now" + return Fail(EStatus::StatusMultipleModifications, TStringBuilder() << "path exists but creating right now" << " (" << BasicPathInfo(Path.Base()) << ")"); } @@ -616,7 +616,7 @@ const TPath::TChecker& TPath::TChecker::FailOnExist(const TSetIsCreateFinished()) { - return Fail(EStatus::StatusMultipleModifications, TStringBuilder() << "path exist but creating right now" + return Fail(EStatus::StatusMultipleModifications, TStringBuilder() << "path exists but creating right now" << " (" << BasicPathInfo(Path.Base()) << ")"); } From fc108f7c484082cccd8cc6101424a78c38b9aa74 Mon Sep 17 00:00:00 2001 From: Aleksei Uzhegov Date: Wed, 31 Jan 2024 15:45:07 +0300 Subject: [PATCH 09/11] Fixed PR comments #4 --- ydb/core/protos/counters_schemeshard.proto | 8 +----- ydb/core/protos/flat_scheme_op.proto | 3 --- .../tx/schemeshard/schemeshard__operation.cpp | 12 ++------- ..._operation_alter_external_data_source.cpp} | 26 +++++++++---------- ...shard__operation_alter_external_table.cpp} | 26 +++++++++---------- ..._operation_create_external_data_source.cpp | 4 +-- ...shard__operation_create_external_table.cpp | 2 +- .../schemeshard/schemeshard__operation_part.h | 12 ++++----- .../schemeshard_audit_log_fragment.cpp | 4 --- ydb/core/tx/schemeshard/schemeshard_impl.cpp | 2 -- .../tx/schemeshard/schemeshard_tx_infly.h | 10 ------- .../ut_external_data_source.cpp | 4 +-- ydb/core/tx/schemeshard/ya.make | 4 +-- 13 files changed, 41 insertions(+), 76 deletions(-) rename ydb/core/tx/schemeshard/{schemeshard__operation_replace_external_data_source.cpp => schemeshard__operation_alter_external_data_source.cpp} (91%) rename ydb/core/tx/schemeshard/{schemeshard__operation_replace_external_table.cpp => schemeshard__operation_alter_external_table.cpp} (94%) diff --git a/ydb/core/protos/counters_schemeshard.proto b/ydb/core/protos/counters_schemeshard.proto index cfc2111cc4f1..ca2a07a1d508 100644 --- a/ydb/core/protos/counters_schemeshard.proto +++ b/ydb/core/protos/counters_schemeshard.proto @@ -194,10 +194,7 @@ enum ESimpleCounters { COUNTER_IN_FLIGHT_OPS_TxAlterView = 156 [(CounterOpts) = {Name: "InFlightOps/AlterView"}]; COUNTER_IN_FLIGHT_OPS_TxDropView = 157 [(CounterOpts) = {Name: "InFlightOps/DropView"}]; - COUNTER_IN_FLIGHT_OPS_TxReplaceExternalTable = 158 [(CounterOpts) = {Name: "InFlightOps/ReplaceExternalTable"}]; - COUNTER_IN_FLIGHT_OPS_TxReplaceExternalDataSource = 159 [(CounterOpts) = {Name: "InFlightOps/ReplaceExternalDataSource"}]; - - COUNTER_GRAPHSHARD_COUNT = 160 [(CounterOpts) = {Name: "GraphShards"}]; + COUNTER_GRAPHSHARD_COUNT = 158 [(CounterOpts) = {Name: "GraphShards"}]; } enum ECumulativeCounters { @@ -318,9 +315,6 @@ enum ECumulativeCounters { COUNTER_FINISHED_OPS_TxCreateView = 95 [(CounterOpts) = {Name: "FinishedOps/CreateView"}]; COUNTER_FINISHED_OPS_TxDropView = 96 [(CounterOpts) = {Name: "FinishedOps/DropView"}]; COUNTER_FINISHED_OPS_TxAlterView = 97 [(CounterOpts) = {Name: "FinishedOps/AlterView"}]; - - COUNTER_FINISHED_OPS_TxReplaceExternalTable = 98 [(CounterOpts) = {Name: "FinishedOps/ReplaceExternalTable"}]; - COUNTER_FINISHED_OPS_TxReplaceExternalDataSource = 99 [(CounterOpts) = {Name: "FinishedOps/ReplaceExternalDataSource"}]; } enum EPercentileCounters { diff --git a/ydb/core/protos/flat_scheme_op.proto b/ydb/core/protos/flat_scheme_op.proto index 26dedd92ffc5..361006fcc062 100644 --- a/ydb/core/protos/flat_scheme_op.proto +++ b/ydb/core/protos/flat_scheme_op.proto @@ -1431,9 +1431,6 @@ enum EOperationType { ESchemeOpCreateView = 93; ESchemeOpAlterView = 94; ESchemeOpDropView = 95; - - ESchemeOpReplaceExternalDataSource = 96; - ESchemeOpReplaceExternalTable = 97; } message TApplyIf { diff --git a/ydb/core/tx/schemeshard/schemeshard__operation.cpp b/ydb/core/tx/schemeshard/schemeshard__operation.cpp index 49bda89c53ec..676771621098 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation.cpp @@ -1061,18 +1061,14 @@ ISubOperation::TPtr TOperation::RestorePart(TTxState::ETxType txType, TTxState:: return CreateNewExternalTable(NextPartId(), txState); case TTxState::ETxType::TxDropExternalTable: return CreateDropExternalTable(NextPartId(), txState); - case TTxState::ETxType::TxReplaceExternalTable: - return CreateReplaceExternalTable(NextPartId(), txState); case TTxState::ETxType::TxAlterExternalTable: - Y_ABORT("TODO: implement"); + return CreateAlterExternalTable(NextPartId(), txState); case TTxState::ETxType::TxCreateExternalDataSource: return CreateNewExternalDataSource(NextPartId(), txState); case TTxState::ETxType::TxDropExternalDataSource: return CreateDropExternalDataSource(NextPartId(), txState); - case TTxState::ETxType::TxReplaceExternalDataSource: - return CreateReplaceExternalDataSource(NextPartId(), txState); case TTxState::ETxType::TxAlterExternalDataSource: - Y_ABORT("TODO: implement"); + return CreateAlterExternalDataSource(NextPartId(), txState); // View case TTxState::ETxType::TxCreateView: @@ -1285,8 +1281,6 @@ ISubOperation::TPtr TOperation::ConstructPart(NKikimrSchemeOp::EOperationType op return CreateDropBlobDepot(NextPartId(), tx); // ExternalTable - case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalTable: - Y_ABORT("this operation must be constructed by ESchemeOpCreateExternalTable"); case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalTable: Y_ABORT("operation is handled before"); case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalTable: @@ -1295,8 +1289,6 @@ ISubOperation::TPtr TOperation::ConstructPart(NKikimrSchemeOp::EOperationType op Y_ABORT("TODO: implement"); // ExternalDataSource - case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalDataSource: - Y_ABORT("this operation must be constructed by ESchemeOpCreateExternalTable"); case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalDataSource: Y_ABORT("operation is handled before"); case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalDataSource: diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_replace_external_data_source.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp similarity index 91% rename from ydb/core/tx/schemeshard/schemeshard__operation_replace_external_data_source.cpp rename to ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp index 174dc1202794..9bca7579ba8d 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_replace_external_data_source.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp @@ -16,7 +16,7 @@ class TPropose: public TSubOperationState { TString DebugHint() const override { return TStringBuilder() - << "TReplaceExternalDataSource TPropose" + << "TAlterExternalDataSource TPropose" << ", operationId: " << OperationId; } @@ -34,7 +34,7 @@ class TPropose: public TSubOperationState { const TTxState* txState = context.SS->FindTx(OperationId); Y_ABORT_UNLESS(txState); - Y_ABORT_UNLESS(txState->TxType == TTxState::TxReplaceExternalDataSource); + Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalDataSource); const auto pathId = txState->TargetPathId; const auto path = TPath::Init(pathId, context.SS); @@ -56,14 +56,14 @@ class TPropose: public TSubOperationState { const TTxState* txState = context.SS->FindTx(OperationId); Y_ABORT_UNLESS(txState); - Y_ABORT_UNLESS(txState->TxType == TTxState::TxReplaceExternalDataSource); + Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalDataSource); context.OnComplete.ProposeToCoordinator(OperationId, txState->TargetPathId, TStepId(0)); return false; } }; -class TReplaceExternalDataSource : public TSubOperation { +class TAlterExternalDataSource : public TSubOperation { static TTxState::ETxState NextState() { return TTxState::Propose; } TTxState::ETxState NextState(TTxState::ETxState state) const override { @@ -152,7 +152,7 @@ class TReplaceExternalDataSource : public TSubOperation { void CreateTransaction(const TOperationContext& context, const TPathId& externalDataSourcePathId) const { TTxState& txState = context.SS->CreateTx(OperationId, - TTxState::TxReplaceExternalDataSource, + TTxState::TxAlterExternalDataSource, externalDataSourcePathId); txState.Shards.clear(); } @@ -207,7 +207,7 @@ class TReplaceExternalDataSource : public TSubOperation { Transaction.GetCreateExternalDataSource(); const TString& name = externalDataSourceDescription.GetName(); - LOG_N("TReplaceExternalDataSource Propose" + LOG_N("TAlterExternalDataSource Propose" << ": opId# " << OperationId << ", path# " << parentPathStr << "/" << name); auto result = MakeHolder(NKikimrScheme::StatusAccepted, @@ -258,13 +258,13 @@ class TReplaceExternalDataSource : public TSubOperation { } void AbortPropose(TOperationContext& context) override { - LOG_N("TReplaceExternalDataSource AbortPropose" + LOG_N("TAlterExternalDataSource AbortPropose" << ": opId# " << OperationId); - Y_ABORT("no AbortPropose for TReplaceExternalDataSource"); + Y_ABORT("no AbortPropose for TAlterExternalDataSource"); } void AbortUnsafe(TTxId forceDropTxId, TOperationContext& context) override { - LOG_N("TReplaceExternalDataSource AbortUnsafe" + LOG_N("TAlterExternalDataSource AbortUnsafe" << ": opId# " << OperationId << ", txId# " << forceDropTxId); context.OnComplete.DoneOperation(OperationId); } @@ -274,13 +274,13 @@ class TReplaceExternalDataSource : public TSubOperation { namespace NKikimr::NSchemeShard { -ISubOperation::TPtr CreateReplaceExternalDataSource(TOperationId id, const TTxTransaction& tx) { - return MakeSubOperation(id, tx); +ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, const TTxTransaction& tx) { + return MakeSubOperation(id, tx); } -ISubOperation::TPtr CreateReplaceExternalDataSource(TOperationId id, TTxState::ETxState state) { +ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, TTxState::ETxState state) { Y_ABORT_UNLESS(state != TTxState::Invalid); - return MakeSubOperation(id, state); + return MakeSubOperation(id, state); } } diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_replace_external_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp similarity index 94% rename from ydb/core/tx/schemeshard/schemeshard__operation_replace_external_table.cpp rename to ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp index 7e93bd78e0b2..a1b29be8e504 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_replace_external_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp @@ -18,7 +18,7 @@ class TPropose: public TSubOperationState { TString DebugHint() const override { return TStringBuilder() - << "TReplaceExternalTable TPropose" + << "TAlterExternalTable TPropose" << ", operationId: " << OperationId; } @@ -59,7 +59,7 @@ class TPropose: public TSubOperationState { const TTxState* txState = context.SS->FindTx(OperationId); Y_ABORT_UNLESS(txState); - Y_ABORT_UNLESS(txState->TxType == TTxState::TxReplaceExternalTable); + Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalTable); const auto pathId = txState->TargetPathId; const auto dataSourcePathId = txState->SourcePathId; @@ -88,7 +88,7 @@ class TPropose: public TSubOperationState { const TTxState* txState = context.SS->FindTx(OperationId); Y_ABORT_UNLESS(txState); - Y_ABORT_UNLESS(txState->TxType == TTxState::TxReplaceExternalTable); + Y_ABORT_UNLESS(txState->TxType == TTxState::TxAlterExternalTable); context.OnComplete.ProposeToCoordinator(OperationId, txState->TargetPathId, TStepId(0)); return false; @@ -96,7 +96,7 @@ class TPropose: public TSubOperationState { }; -class TReplacetExternalTable: public TSubOperation { +class TAlterExternalTable: public TSubOperation { private: bool IsSameDataSource = true; TPathId OldDataSourcePathId = InvalidPathId; @@ -220,7 +220,7 @@ class TReplacetExternalTable: public TSubOperation { const TPathId& externalTablePathId, const TPathId& externalDataSourcePathId) const { TTxState& txState = context.SS->CreateTx(OperationId, - TTxState::TxReplaceExternalTable, + TTxState::TxAlterExternalTable, externalTablePathId, externalDataSourcePathId); txState.Shards.clear(); @@ -301,7 +301,7 @@ class TReplacetExternalTable: public TSubOperation { const auto& externalTableDescription = Transaction.GetCreateExternalTable(); const TString& name = externalTableDescription.GetName(); - LOG_N("TReplaceExternalTable Propose" + LOG_N("TAlterExternalTable Propose" << ": opId# " << OperationId << ", path# " << parentPathStr << "/" << name << ", ReplaceIfExists:" << externalTableDescription.GetReplaceIfExists()); @@ -392,13 +392,13 @@ class TReplacetExternalTable: public TSubOperation { } void AbortPropose(TOperationContext& context) override { - LOG_N("TReplaceExternalTable AbortPropose" + LOG_N("TAlterExternalTable AbortPropose" << ": opId# " << OperationId); - Y_ABORT("no AbortPropose for TReplaceExternalTable"); + Y_ABORT("no AbortPropose for TAlterExternalTable"); } void AbortUnsafe(TTxId forceDropTxId, TOperationContext& context) override { - LOG_N("TReplaceExternalTable AbortUnsafe" + LOG_N("TAlterExternalTable AbortUnsafe" << ": opId# " << OperationId << ", txId# " << forceDropTxId); context.OnComplete.DoneOperation(OperationId); @@ -409,13 +409,13 @@ class TReplacetExternalTable: public TSubOperation { namespace NKikimr::NSchemeShard { -ISubOperation::TPtr CreateReplaceExternalTable(TOperationId id, const TTxTransaction& tx) { - return MakeSubOperation(std::move(id), tx); +ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, const TTxTransaction& tx) { + return MakeSubOperation(std::move(id), tx); } -ISubOperation::TPtr CreateReplaceExternalTable(TOperationId id, TTxState::ETxState state) { +ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, TTxState::ETxState state) { Y_ABORT_UNLESS(state != TTxState::Invalid); - return MakeSubOperation(std::move(id), state); + return MakeSubOperation(std::move(id), state); } } diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp index 03e9dccf0392..bebd2423904d 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_data_source.cpp @@ -329,8 +329,6 @@ TVector CreateNewExternalDataSource(TOperationId id, } } - - if (replaceIfExists) { const TPath dstPath = parentPath.Child(name); const auto isAlreadyExists = @@ -338,7 +336,7 @@ TVector CreateNewExternalDataSource(TOperationId id, .IsResolved() .NotUnderDeleting(); if (isAlreadyExists) { - return {CreateReplaceExternalDataSource(id, tx)}; + return {CreateAlterExternalDataSource(id, tx)}; } } diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp index e39a1ee7525f..702e8c699582 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_create_external_table.cpp @@ -429,7 +429,7 @@ TVector CreateNewExternalTable(TOperationId id, const TTxTr .IsResolved() .NotUnderDeleting(); if (isAlreadyExists) { - return {CreateReplaceExternalTable(id, tx)}; + return {CreateAlterExternalTable(id, tx)}; } } diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_part.h b/ydb/core/tx/schemeshard/schemeshard__operation_part.h index c0367b99b699..0a9cf8b6b8b3 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_part.h +++ b/ydb/core/tx/schemeshard/schemeshard__operation_part.h @@ -371,9 +371,9 @@ ISubOperation::TPtr CreateUpdateMainTableOnIndexMove(TOperationId id, TTxState:: // Create TVector CreateNewExternalTable(TOperationId id, const TTxTransaction& tx, TOperationContext& context); ISubOperation::TPtr CreateNewExternalTable(TOperationId id, TTxState::ETxState state); -// Replace -ISubOperation::TPtr CreateReplaceExternalTable(TOperationId id, const TTxTransaction& tx); -ISubOperation::TPtr CreateReplaceExternalTable(TOperationId id, TTxState::ETxState state); +// Alter +ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, const TTxTransaction& tx); +ISubOperation::TPtr CreateAlterExternalTable(TOperationId id, TTxState::ETxState state); // Drop ISubOperation::TPtr CreateDropExternalTable(TOperationId id, const TTxTransaction& tx); ISubOperation::TPtr CreateDropExternalTable(TOperationId id, TTxState::ETxState state); @@ -382,9 +382,9 @@ ISubOperation::TPtr CreateDropExternalTable(TOperationId id, TTxState::ETxState // Create TVector CreateNewExternalDataSource(TOperationId id, const TTxTransaction& tx, TOperationContext& context); ISubOperation::TPtr CreateNewExternalDataSource(TOperationId id, TTxState::ETxState state); -// Replace -ISubOperation::TPtr CreateReplaceExternalDataSource(TOperationId id, const TTxTransaction& tx); -ISubOperation::TPtr CreateReplaceExternalDataSource(TOperationId id, TTxState::ETxState state); +// Alter +ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, const TTxTransaction& tx); +ISubOperation::TPtr CreateAlterExternalDataSource(TOperationId id, TTxState::ETxState state); // Drop ISubOperation::TPtr CreateDropExternalDataSource(TOperationId id, const TTxTransaction& tx); ISubOperation::TPtr CreateDropExternalDataSource(TOperationId id, TTxState::ETxState state); diff --git a/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp b/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp index f5963d5e433c..12c04ccfe53f 100644 --- a/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp @@ -208,14 +208,12 @@ TString DefineUserOperationName(const NKikimrSchemeOp::TModifyScheme& tx) { return "ALTER BLOB DEPOT"; case NKikimrSchemeOp::EOperationType::ESchemeOpDropBlobDepot: return "DROP BLOB DEPOT"; - case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalTable: case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalTable: return "CREATE EXTERNAL TABLE"; case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalTable: return "DROP EXTERNAL TABLE"; case NKikimrSchemeOp::EOperationType::ESchemeOpAlterExternalTable: return "ALTER EXTERNAL TABLE"; - case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalDataSource: case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalDataSource: return "CREATE EXTERNAL DATA SOURCE"; case NKikimrSchemeOp::EOperationType::ESchemeOpDropExternalDataSource: @@ -489,7 +487,6 @@ TVector ExtractChangingPaths(const NKikimrSchemeOp::TModifyScheme& tx) result.emplace_back(NKikimr::JoinPath({tx.GetMoveIndex().GetTablePath(), tx.GetMoveIndex().GetSrcPath()})); result.emplace_back(NKikimr::JoinPath({tx.GetMoveIndex().GetTablePath(), tx.GetMoveIndex().GetDstPath()})); break; - case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalTable: case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalTable: result.emplace_back(NKikimr::JoinPath({tx.GetWorkingDir(), tx.GetCreateExternalTable().GetName()})); break; @@ -499,7 +496,6 @@ TVector ExtractChangingPaths(const NKikimrSchemeOp::TModifyScheme& tx) case NKikimrSchemeOp::EOperationType::ESchemeOpAlterExternalTable: // TODO: unimplemented break; - case NKikimrSchemeOp::EOperationType::ESchemeOpReplaceExternalDataSource: case NKikimrSchemeOp::EOperationType::ESchemeOpCreateExternalDataSource: result.emplace_back(NKikimr::JoinPath({tx.GetWorkingDir(), tx.GetCreateExternalDataSource().GetName()})); break; diff --git a/ydb/core/tx/schemeshard/schemeshard_impl.cpp b/ydb/core/tx/schemeshard/schemeshard_impl.cpp index 826d31c78dc6..511109a404eb 100644 --- a/ydb/core/tx/schemeshard/schemeshard_impl.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_impl.cpp @@ -1455,8 +1455,6 @@ TPathElement::EPathState TSchemeShard::CalcPathState(TTxState::ETxType txType, T case TTxState::TxUpdateMainTableOnIndexMove: case TTxState::TxAlterExternalTable: case TTxState::TxAlterExternalDataSource: - case TTxState::TxReplaceExternalTable: - case TTxState::TxReplaceExternalDataSource: case TTxState::TxAlterView: return TPathElement::EPathState::EPathStateAlter; case TTxState::TxDropTable: diff --git a/ydb/core/tx/schemeshard/schemeshard_tx_infly.h b/ydb/core/tx/schemeshard/schemeshard_tx_infly.h index 3ea7e75a3d68..849f91a9dee0 100644 --- a/ydb/core/tx/schemeshard/schemeshard_tx_infly.h +++ b/ydb/core/tx/schemeshard/schemeshard_tx_infly.h @@ -129,8 +129,6 @@ struct TTxState { item(TxCreateView, 83) \ item(TxAlterView, 84) \ item(TxDropView, 85) \ - item(TxReplaceExternalDataSource, 86) \ - item(TxReplaceExternalTable, 87) \ // TX_STATE_TYPE_ENUM @@ -407,8 +405,6 @@ struct TTxState { case TxAlterBlobDepot: case TxAlterExternalTable: case TxAlterExternalDataSource: - case TxReplaceExternalDataSource: - case TxReplaceExternalTable: case TxAlterView: return false; case TxMoveTable: @@ -506,8 +502,6 @@ struct TTxState { case TxAlterBlobDepot: case TxAlterExternalTable: case TxAlterExternalDataSource: - case TxReplaceExternalDataSource: - case TxReplaceExternalTable: case TxAlterView: return false; case TxMoveTable: @@ -608,8 +602,6 @@ struct TTxState { case TxAlterBlobDepot: case TxAlterExternalTable: case TxAlterExternalDataSource: - case TxReplaceExternalDataSource: - case TxReplaceExternalTable: case TxAlterView: return false; case TxInvalid: @@ -706,10 +698,8 @@ struct TTxState { case NKikimrSchemeOp::ESchemeOpDeallocatePersQueueGroup: return TxInvalid; case NKikimrSchemeOp::ESchemeOpCreateExternalTable: return TxCreateExternalTable; case NKikimrSchemeOp::ESchemeOpAlterExternalTable: return TxAlterExternalTable; - case NKikimrSchemeOp::ESchemeOpReplaceExternalTable: return TxReplaceExternalTable; case NKikimrSchemeOp::ESchemeOpCreateExternalDataSource: return TxCreateExternalDataSource; case NKikimrSchemeOp::ESchemeOpAlterExternalDataSource: return TxAlterExternalDataSource; - case NKikimrSchemeOp::ESchemeOpReplaceExternalDataSource: return TxReplaceExternalDataSource; case NKikimrSchemeOp::ESchemeOpCreateView: return TxCreateView; case NKikimrSchemeOp::ESchemeOpAlterView: return TxAlterView; case NKikimrSchemeOp::ESchemeOpDropView: return TxDropView; diff --git a/ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp b/ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp index 6f791e96d82b..35dc0610e21e 100644 --- a/ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp +++ b/ydb/core/tx/schemeshard/ut_external_data_source/ut_external_data_source.cpp @@ -404,7 +404,7 @@ Y_UNIT_TEST_SUITE(TExternalDataSourceTest) { TestLs(runtime, "/MyRoot/ExternalDataSource", false, NLs::PathNotExist); } - Y_UNIT_TEST(CreateExternalDataSourceWithOrReplace) { + Y_UNIT_TEST(ReplaceExternalDataSourceIfNotExists) { TTestBasicRuntime runtime; TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExistsForExternalEntities(true)); ui64 txId = 100; @@ -459,7 +459,7 @@ Y_UNIT_TEST_SUITE(TExternalDataSourceTest) { } } - Y_UNIT_TEST(CreateExternalDataSourceWithOrReplaceShouldFailIfFeatureFlagIsNotSet) { + Y_UNIT_TEST(ReplaceExternalDataSourceIfNotExistsShouldFailIfFeatureFlagIsNotSet) { TTestBasicRuntime runtime; TTestEnv env(runtime, TTestEnvOptions().EnableReplaceIfExistsForExternalEntities(false)); ui64 txId = 100; diff --git a/ydb/core/tx/schemeshard/ya.make b/ydb/core/tx/schemeshard/ya.make index 095ada7cbf60..47d6597cbda4 100644 --- a/ydb/core/tx/schemeshard/ya.make +++ b/ydb/core/tx/schemeshard/ya.make @@ -80,6 +80,8 @@ SRCS( schemeshard__operation_memory_changes.cpp schemeshard__operation_db_changes.cpp schemeshard__operation_alter_bsv.cpp + schemeshard__operation_alter_external_data_source.cpp + schemeshard__operation_alter_external_table.cpp schemeshard__operation_alter_extsubdomain.cpp schemeshard__operation_alter_fs.cpp schemeshard__operation_alter_index.cpp @@ -142,8 +144,6 @@ SRCS( schemeshard__operation_move_table_index.cpp schemeshard__operation_part.cpp schemeshard__operation_part.h - schemeshard__operation_replace_external_data_source.cpp - schemeshard__operation_replace_external_table.cpp schemeshard__operation_rmdir.cpp schemeshard__operation_split_merge.cpp schemeshard__operation_just_reject.cpp From 02ad968b3986596785c95950980a14ffaa43786b Mon Sep 17 00:00:00 2001 From: Aleksei Uzhegov Date: Wed, 31 Jan 2024 15:53:29 +0300 Subject: [PATCH 10/11] Fixed PR comments #5 --- .../schemeshard__operation_alter_external_data_source.cpp | 4 ++-- .../schemeshard__operation_alter_external_table.cpp | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp index 9bca7579ba8d..b7c8cb88e8d4 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_data_source.cpp @@ -140,7 +140,7 @@ class TAlterExternalDataSource : public TSubOperation { result->SetPathId(dstPath.Base()->PathId.LocalPathId); } - TPathElement::TPtr CreateExternalDataSourcePathElement(const TPath& dstPath) const { + TPathElement::TPtr ReplaceExternalDataSourcePathElement(const TPath& dstPath) const { TPathElement::TPtr externalDataSource = dstPath.Base(); externalDataSource->PathState = TPathElement::EPathState::EPathStateAlter; @@ -236,7 +236,7 @@ class TAlterExternalDataSource : public TSubOperation { AddPathInSchemeShard(result, dstPath); const TPathElement::TPtr externalDataSource = - CreateExternalDataSourcePathElement(dstPath); + ReplaceExternalDataSourcePathElement(dstPath); CreateTransaction(context, externalDataSource->PathId); NIceDb::TNiceDb db(context.GetDB()); diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp index a1b29be8e504..5b52536ed00a 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_external_table.cpp @@ -208,7 +208,7 @@ class TAlterExternalTable: public TSubOperation { result->SetPathId(dstPath.Base()->PathId.LocalPathId); } - TPathElement::TPtr CreateExternalTablePathElement(const TPath& dstPath) const { + TPathElement::TPtr ReplaceExternalTablePathElement(const TPath& dstPath) const { TPathElement::TPtr externalTable = dstPath.Base(); externalTable->PathState = TPathElement::EPathState::EPathStateAlter; externalTable->LastTxId = OperationId.GetTxId(); @@ -247,9 +247,9 @@ class TAlterExternalTable: public TSubOperation { const TPathElement::TPtr& externalTable, const TPath& dstPath, const TExternalDataSourceInfo::TPtr& oldDataSource, - bool IsSameDataSource) { + bool isSameDataSource) { - if (!IsSameDataSource) { + if (!isSameDataSource) { auto& reference = *externalDataSource->ExternalTableReferences.AddReferences(); reference.SetPath(dstPath.PathString()); PathIdFromPathId(externalTable->PathId, reference.MutablePathId()); @@ -363,7 +363,7 @@ class TAlterExternalTable: public TSubOperation { AddPathInSchemeShard(result, dstPath); - const auto externalTable = CreateExternalTablePathElement(dstPath); + const auto externalTable = ReplaceExternalTablePathElement(dstPath); CreateTransaction(context, externalTable->PathId, dataSourcePath->PathId); NIceDb::TNiceDb db(context.GetDB()); From b3251c7f7f787c164ed14d73e6047ca914bd107b Mon Sep 17 00:00:00 2001 From: Aleksei Uzhegov Date: Wed, 31 Jan 2024 16:22:19 +0300 Subject: [PATCH 11/11] Fixed PR comments #6 --- ydb/core/tx/schemeshard/schemeshard_impl.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/ydb/core/tx/schemeshard/schemeshard_impl.cpp b/ydb/core/tx/schemeshard/schemeshard_impl.cpp index 2809bc71b481..c55bca5942fe 100644 --- a/ydb/core/tx/schemeshard/schemeshard_impl.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_impl.cpp @@ -6847,6 +6847,7 @@ void TSchemeShard::ApplyConsoleConfigs(const NKikimrConfig::TFeatureFlags& featu EnableServerlessExclusiveDynamicNodes = featureFlags.GetEnableServerlessExclusiveDynamicNodes(); EnableAddColumsWithDefaults = featureFlags.GetEnableAddColumsWithDefaults(); EnableTempTables = featureFlags.GetEnableTempTables(); + EnableReplaceIfExistsForExternalEntities = featureFlags.GetEnableReplaceIfExistsForExternalEntities(); } void TSchemeShard::ConfigureStatsBatching(const NKikimrConfig::TSchemeShardConfig& config, const TActorContext& ctx) {