diff --git a/ydb/core/kqp/executer_actor/kqp_scheme_executer.cpp b/ydb/core/kqp/executer_actor/kqp_scheme_executer.cpp index 13f4c3ba9388..953a306bd376 100644 --- a/ydb/core/kqp/executer_actor/kqp_scheme_executer.cpp +++ b/ydb/core/kqp/executer_actor/kqp_scheme_executer.cpp @@ -139,17 +139,49 @@ class TKqpSchemeExecuter : public TActorBootstrapped { ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme); break; } + case NKqpProto::TKqpSchemeOperation::kAlterUser: { auto modifyScheme = schemeOp.GetAlterUser(); ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme); break; } + case NKqpProto::TKqpSchemeOperation::kDropUser: { auto modifyScheme = schemeOp.GetDropUser(); ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme); break; } + case NKqpProto::TKqpSchemeOperation::kCreateGroup: { + auto modifyScheme = schemeOp.GetCreateGroup(); + ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme); + break; + } + + case NKqpProto::TKqpSchemeOperation::kAddGroupMembership: { + auto modifyScheme = schemeOp.GetAddGroupMembership(); + ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme); + break; + } + + case NKqpProto::TKqpSchemeOperation::kRemoveGroupMembership: { + auto modifyScheme = schemeOp.GetRemoveGroupMembership(); + ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme); + break; + } + + case NKqpProto::TKqpSchemeOperation::kRenameGroup: { + auto modifyScheme = schemeOp.GetRenameGroup(); + ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme); + break; + } + + case NKqpProto::TKqpSchemeOperation::kDropGroup: { + auto modifyScheme = schemeOp.GetDropGroup(); + ev->Record.MutableTransaction()->MutableModifyScheme()->CopyFrom(modifyScheme); + break; + } + default: InternalError(TStringBuilder() << "Unexpected scheme operation: " << (ui32) schemeOp.GetOperationCase()); diff --git a/ydb/core/kqp/gateway/actors/scheme.h b/ydb/core/kqp/gateway/actors/scheme.h index d4473472d9ce..406e458e23c0 100644 --- a/ydb/core/kqp/gateway/actors/scheme.h +++ b/ydb/core/kqp/gateway/actors/scheme.h @@ -86,10 +86,17 @@ class TSchemeOpRequestHandler: public TRequestHandlerBase< { LOG_DEBUG_S(ctx, NKikimrServices::KQP_GATEWAY, "Successful completion of scheme request" << ", TxId: " << response.GetTxId()); - - TResult result; - result.SetSuccess(); - Promise.SetValue(std::move(result)); + + if (!response.GetIssues().empty()) { + NYql::TIssues issues; + NYql::IssuesFromMessage(response.GetIssues(), issues); + Promise.SetValue(NYql::NCommon::ResultFromIssues(NYql::TIssuesIds::SUCCESS, "", issues)); + } else { + TResult result; + result.SetSuccess(); + Promise.SetValue(std::move(result)); + } + this->Die(ctx); return; } diff --git a/ydb/core/kqp/gateway/kqp_ic_gateway.cpp b/ydb/core/kqp/gateway/kqp_ic_gateway.cpp index f8ae594b03fc..015eb888bb3d 100644 --- a/ydb/core/kqp/gateway/kqp_ic_gateway.cpp +++ b/ydb/core/kqp/gateway/kqp_ic_gateway.cpp @@ -1772,6 +1772,47 @@ class TKikimrIcGateway : public IKqpGateway { } } + TFuture RenameGroup(const TString& cluster, NYql::TRenameGroupSettings& settings) override { + using TRequest = TEvTxUserProxy::TEvProposeTransaction; + + try { + if (!CheckCluster(cluster)) { + return InvalidCluster(cluster); + } + + TString database; + if (!GetDatabaseForLoginOperation(database)) { + return MakeFuture(ResultFromError("Couldn't get domain name")); + } + + TPromise renameGroupPromise = NewPromise(); + + auto ev = MakeHolder(); + ev->Record.SetDatabaseName(database); + if (UserToken) { + ev->Record.SetUserToken(UserToken->GetSerializedToken()); + } + auto& schemeTx = *ev->Record.MutableTransaction()->MutableModifyScheme(); + schemeTx.SetWorkingDir(database); + schemeTx.SetOperationType(NKikimrSchemeOp::ESchemeOpAlterLogin); + auto& renameGroup = *schemeTx.MutableAlterLogin()->MutableRenameGroup(); + + renameGroup.SetGroup(settings.GroupName); + renameGroup.SetNewName(settings.NewName); + + SendSchemeRequest(ev.Release()).Apply( + [renameGroupPromise](const TFuture& future) mutable { + renameGroupPromise.SetValue(future.GetValue()); + } + ); + + return renameGroupPromise.GetFuture(); + } + catch (yexception& e) { + return MakeFuture(ResultFromException(e)); + } + } + TFuture DropGroup(const TString& cluster, const NYql::TDropGroupSettings& settings) override { using TRequest = TEvTxUserProxy::TEvProposeTransaction; @@ -1798,17 +1839,11 @@ class TKikimrIcGateway : public IKqpGateway { auto& dropGroup = *schemeTx.MutableAlterLogin()->MutableRemoveGroup(); dropGroup.SetGroup(settings.GroupName); + dropGroup.SetMissingOk(settings.Force); SendSchemeRequest(ev.Release()).Apply( - [dropGroupPromise, &settings](const TFuture& future) mutable { - const auto& realResult = future.GetValue(); - if (!realResult.Success() && realResult.Status() == TIssuesIds::DEFAULT_ERROR && settings.Force) { - IKqpGateway::TGenericResult fakeResult; - fakeResult.SetSuccess(); - dropGroupPromise.SetValue(std::move(fakeResult)); - } else { - dropGroupPromise.SetValue(realResult); - } + [dropGroupPromise](const TFuture& future) mutable { + dropGroupPromise.SetValue(future.GetValue()); } ); diff --git a/ydb/core/kqp/host/kqp_gateway_proxy.cpp b/ydb/core/kqp/host/kqp_gateway_proxy.cpp index bf97d2e3389f..0f6d15d6a149 100644 --- a/ydb/core/kqp/host/kqp_gateway_proxy.cpp +++ b/ydb/core/kqp/host/kqp_gateway_proxy.cpp @@ -908,15 +908,133 @@ class TKqpGatewayProxy : public IKikimrGateway { } TFuture CreateGroup(const TString& cluster, const TCreateGroupSettings& settings) override { - FORWARD_ENSURE_NO_PREPARE(CreateGroup, cluster, settings); + CHECK_PREPARED_DDL(CreateGroup); + + auto createGroupPromise = NewPromise(); + + TString database; + if (!SetDatabaseForLoginOperation(database, GetDomainLoginOnly(), GetDomainName(), GetDatabase())) { + return MakeFuture(ResultFromError("Couldn't get domain name")); + } + + NKikimrSchemeOp::TModifyScheme schemeTx; + schemeTx.SetWorkingDir(database); + schemeTx.SetOperationType(NKikimrSchemeOp::ESchemeOpAlterLogin); + auto& createGroup = *schemeTx.MutableAlterLogin()->MutableCreateGroup(); + createGroup.SetGroup(settings.GroupName); + + if (IsPrepare()) { + auto& phyQuery = *SessionCtx->Query().PreparingQuery->MutablePhysicalQuery(); + auto& phyTx = *phyQuery.AddTransactions(); + phyTx.SetType(NKqpProto::TKqpPhyTx::TYPE_SCHEME); + + phyTx.MutableSchemeOperation()->MutableCreateGroup()->Swap(&schemeTx); + + if (settings.Roles.size()) { + AddUsersToGroup(database, settings.GroupName, settings.Roles, NYql::TAlterGroupSettings::EAction::AddRoles); + } + + TGenericResult result; + result.SetSuccess(); + createGroupPromise.SetValue(result); + } else { + return Gateway->CreateGroup(cluster, settings); + } + + return createGroupPromise.GetFuture(); } TFuture AlterGroup(const TString& cluster, TAlterGroupSettings& settings) override { - FORWARD_ENSURE_NO_PREPARE(AlterGroup, cluster, settings); + CHECK_PREPARED_DDL(UpdateGroup); + + auto alterGroupPromise = NewPromise(); + + TString database; + if (!SetDatabaseForLoginOperation(database, GetDomainLoginOnly(), GetDomainName(), GetDatabase())) { + return MakeFuture(ResultFromError("Couldn't get domain name")); + } + + if (!settings.Roles.size()) { + return MakeFuture(ResultFromError("No roles given for AlterGroup request")); + } + + if (IsPrepare()) { + AddUsersToGroup(database, settings.GroupName, settings.Roles, settings.Action); + + TGenericResult result; + result.SetSuccess(); + alterGroupPromise.SetValue(result); + } else { + return Gateway->AlterGroup(cluster, settings); + } + + return alterGroupPromise.GetFuture(); + } + + TFuture RenameGroup(const TString& cluster, TRenameGroupSettings& settings) override { + CHECK_PREPARED_DDL(RenameGroup); + + auto renameGroupPromise = NewPromise(); + + TString database; + if (!SetDatabaseForLoginOperation(database, GetDomainLoginOnly(), GetDomainName(), GetDatabase())) { + return MakeFuture(ResultFromError("Couldn't get domain name")); + } + + NKikimrSchemeOp::TModifyScheme schemeTx; + schemeTx.SetWorkingDir(database); + schemeTx.SetOperationType(NKikimrSchemeOp::ESchemeOpAlterLogin); + auto& renameGroup = *schemeTx.MutableAlterLogin()->MutableRenameGroup(); + renameGroup.SetGroup(settings.GroupName); + renameGroup.SetNewName(settings.NewName); + + if (IsPrepare()) { + auto& phyQuery = *SessionCtx->Query().PreparingQuery->MutablePhysicalQuery(); + auto& phyTx = *phyQuery.AddTransactions(); + phyTx.SetType(NKqpProto::TKqpPhyTx::TYPE_SCHEME); + + phyTx.MutableSchemeOperation()->MutableRenameGroup()->Swap(&schemeTx); + TGenericResult result; + result.SetSuccess(); + renameGroupPromise.SetValue(result); + } else { + return Gateway->RenameGroup(cluster, settings); + } + + return renameGroupPromise.GetFuture(); } TFuture DropGroup(const TString& cluster, const TDropGroupSettings& settings) override { - FORWARD_ENSURE_NO_PREPARE(DropGroup, cluster, settings); + CHECK_PREPARED_DDL(DropGroup); + + auto dropGroupPromise = NewPromise(); + + TString database; + if (!SetDatabaseForLoginOperation(database, GetDomainLoginOnly(), GetDomainName(), GetDatabase())) { + return MakeFuture(ResultFromError("Couldn't get domain name")); + } + + NKikimrSchemeOp::TModifyScheme schemeTx; + schemeTx.SetWorkingDir(database); + schemeTx.SetOperationType(NKikimrSchemeOp::ESchemeOpAlterLogin); + auto& dropGroup = *schemeTx.MutableAlterLogin()->MutableRemoveGroup(); + dropGroup.SetGroup(settings.GroupName); + dropGroup.SetMissingOk(settings.Force); + + if (IsPrepare()) { + auto& phyQuery = *SessionCtx->Query().PreparingQuery->MutablePhysicalQuery(); + auto& phyTx = *phyQuery.AddTransactions(); + phyTx.SetType(NKqpProto::TKqpPhyTx::TYPE_SCHEME); + + phyTx.MutableSchemeOperation()->MutableAlterUser()->Swap(&schemeTx); + TGenericResult result; + result.SetSuccess(); + dropGroupPromise.SetValue(result); + } else { + return Gateway->DropGroup(cluster, settings); + } + + return dropGroupPromise.GetFuture(); } TFuture CreateColumnTable(TKikimrTableMetadataPtr metadata, bool createDir) override { @@ -1000,6 +1118,35 @@ class TKqpGatewayProxy : public IKikimrGateway { return Gateway->GetDomainName(); } + void AddUsersToGroup(const TString& database, const TString& group, const std::vector& roles, const NYql::TAlterGroupSettings::EAction& action) { + for (const auto& role : roles) { + NKikimrSchemeOp::TModifyScheme schemeTx; + schemeTx.SetWorkingDir(database); + schemeTx.SetOperationType(NKikimrSchemeOp::ESchemeOpAlterLogin); + + auto& phyQuery = *SessionCtx->Query().PreparingQuery->MutablePhysicalQuery(); + auto& phyTx = *phyQuery.AddTransactions(); + phyTx.SetType(NKqpProto::TKqpPhyTx::TYPE_SCHEME); + + switch (action) { + case NYql::TAlterGroupSettings::EAction::AddRoles: { + auto& alterGroup = *schemeTx.MutableAlterLogin()->MutableAddGroupMembership(); + alterGroup.SetGroup(group); + alterGroup.SetMember(role); + phyTx.MutableSchemeOperation()->MutableAddGroupMembership()->Swap(&schemeTx); + break; + } + case NYql::TAlterGroupSettings::EAction::RemoveRoles: { + auto& alterGroup = *schemeTx.MutableAlterLogin()->MutableRemoveGroupMembership(); + alterGroup.SetGroup(group); + alterGroup.SetMember(role); + phyTx.MutableSchemeOperation()->MutableRemoveGroupMembership()->Swap(&schemeTx); + break; + } + } + } + } + private: TIntrusivePtr Gateway; TIntrusivePtr SessionCtx; diff --git a/ydb/core/kqp/provider/yql_kikimr_datasink.cpp b/ydb/core/kqp/provider/yql_kikimr_datasink.cpp index dad609528476..58ab815886be 100644 --- a/ydb/core/kqp/provider/yql_kikimr_datasink.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_datasink.cpp @@ -157,6 +157,12 @@ class TKiSinkIntentDeterminationTransformer: public TKiSinkVisitorTransformer { return TStatus::Error; } + TStatus HandleRenameGroup(TKiRenameGroup node, TExprContext& ctx) override { + ctx.AddError(TIssue(ctx.GetPosition(node.Pos()), TStringBuilder() + << "RenameGroup is not yet implemented for intent determination transformer")); + return TStatus::Error; + } + TStatus HandleDropGroup(TKiDropGroup node, TExprContext& ctx) override { ctx.AddError(TIssue(ctx.GetPosition(node.Pos()), TStringBuilder() << "DropGroup is not yet implemented for intent determination transformer")); @@ -466,6 +472,7 @@ class TKikimrDataSink : public TDataProviderBase || node.IsCallable(TKiDropUser::CallableName()) || node.IsCallable(TKiCreateGroup::CallableName()) || node.IsCallable(TKiAlterGroup::CallableName()) + || node.IsCallable(TKiRenameGroup::CallableName()) || node.IsCallable(TKiDropGroup::CallableName()) || node.IsCallable(TKiUpsertObject::CallableName()) || node.IsCallable(TKiCreateObject::CallableName()) @@ -916,6 +923,7 @@ class TKikimrDataSink : public TDataProviderBase .World(node->Child(0)) .DataSink(node->Child(1)) .GroupName().Build(key.GetRoleName()) + .Roles(settings.Roles.Cast()) .Done() .Ptr(); } else if (mode == "addUsersToGroup" || mode == "dropUsersFromGroup") { @@ -927,6 +935,14 @@ class TKikimrDataSink : public TDataProviderBase .Roles(settings.Roles.Cast()) .Done() .Ptr(); + } else if (mode == "renameGroup") { + return Build(ctx, node->Pos()) + .World(node->Child(0)) + .DataSink(node->Child(1)) + .GroupName().Build(key.GetRoleName()) + .NewName(settings.NewName.Cast()) + .Done() + .Ptr(); } else if (mode == "dropGroup") { return Build(ctx, node->Pos()) .World(node->Child(0)) @@ -1103,6 +1119,10 @@ IGraphTransformer::TStatus TKiSinkVisitorTransformer::DoTransform(TExprNode::TPt return HandleAlterGroup(node.Cast(), ctx); } + if (auto node = TMaybeNode(input)) { + return HandleRenameGroup(node.Cast(), ctx); + } + if (auto node = TMaybeNode(input)) { return HandleDropGroup(node.Cast(), ctx); } diff --git a/ydb/core/kqp/provider/yql_kikimr_exec.cpp b/ydb/core/kqp/provider/yql_kikimr_exec.cpp index 8272c83f0a87..8b8224529760 100644 --- a/ydb/core/kqp/provider/yql_kikimr_exec.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_exec.cpp @@ -149,6 +149,10 @@ namespace { TCreateGroupSettings ParseCreateGroupSettings(TKiCreateGroup createGroup) { TCreateGroupSettings createGroupSettings; createGroupSettings.GroupName = TString(createGroup.GroupName()); + + for (auto role : createGroup.Roles()) { + createGroupSettings.Roles.push_back(role.Cast().StringValue()); + } return createGroupSettings; } @@ -169,6 +173,13 @@ namespace { return alterGroupSettings; } + TRenameGroupSettings ParseRenameGroupSettings(TKiRenameGroup renameGroup) { + TRenameGroupSettings renameGroupSettings; + renameGroupSettings.GroupName = TString(renameGroup.GroupName()); + renameGroupSettings.NewName = TString(renameGroup.NewName()); + return renameGroupSettings; + } + TDropGroupSettings ParseDropGroupSettings(TKiDropGroup dropGroup) { TDropGroupSettings dropGroupSettings; dropGroupSettings.GroupName = TString(dropGroup.GroupName()); @@ -1768,10 +1779,6 @@ class TKiSinkCallableExecutionTransformer : public TAsyncCallbackTransformer(input)) { - if (!EnsureNotPrepare("CREATE GROUP", input->Pos(), SessionCtx->Query(), ctx)) { - return SyncError(); - } - auto requireStatus = RequireChild(*input, 0); if (requireStatus.Level != TStatus::Ok) { return SyncStatus(requireStatus); @@ -1780,8 +1787,7 @@ class TKiSinkCallableExecutionTransformer : public TAsyncCallbackTransformerQuery().PrepareOnly; - auto future = prepareOnly ? CreateDummySuccess() : Gateway->CreateGroup(cluster, createGroupSettings); + auto future = Gateway->CreateGroup(cluster, createGroupSettings); return WrapFuture(future, [](const IKikimrGateway::TGenericResult& res, const TExprNode::TPtr& input, TExprContext& ctx) { @@ -1792,10 +1798,6 @@ class TKiSinkCallableExecutionTransformer : public TAsyncCallbackTransformer(input)) { - if (!EnsureNotPrepare("ALTER GROUP", input->Pos(), SessionCtx->Query(), ctx)) { - return SyncError(); - } - auto requireStatus = RequireChild(*input, 0); if (requireStatus.Level != TStatus::Ok) { return SyncStatus(requireStatus); @@ -1804,8 +1806,7 @@ class TKiSinkCallableExecutionTransformer : public TAsyncCallbackTransformerQuery().PrepareOnly; - auto future = prepareOnly ? CreateDummySuccess() : Gateway->AlterGroup(cluster, alterGroupSettings); + auto future = Gateway->AlterGroup(cluster, alterGroupSettings); return WrapFuture(future, [](const IKikimrGateway::TGenericResult& res, const TExprNode::TPtr& input, TExprContext& ctx) { @@ -1815,11 +1816,26 @@ class TKiSinkCallableExecutionTransformer : public TAsyncCallbackTransformer(input)) { - if (!EnsureNotPrepare("DROP GROUP", input->Pos(), SessionCtx->Query(), ctx)) { - return SyncError(); + if (auto maybeRenameGroup = TMaybeNode(input)) { + auto requireStatus = RequireChild(*input, 0); + if (requireStatus.Level != TStatus::Ok) { + return SyncStatus(requireStatus); } + auto cluster = TString(maybeRenameGroup.Cast().DataSink().Cluster()); + TRenameGroupSettings renameGroupSettings = ParseRenameGroupSettings(maybeRenameGroup.Cast()); + + auto future = Gateway->RenameGroup(cluster, renameGroupSettings); + + return WrapFuture(future, + [](const IKikimrGateway::TGenericResult& res, const TExprNode::TPtr& input, TExprContext& ctx) { + Y_UNUSED(res); + auto resultNode = ctx.NewWorld(input->Pos()); + return resultNode; + }, "Executing RENAME GROUP"); + } + + if (auto maybeDropGroup = TMaybeNode(input)) { auto requireStatus = RequireChild(*input, 0); if (requireStatus.Level != TStatus::Ok) { return SyncStatus(requireStatus); @@ -1828,8 +1844,7 @@ class TKiSinkCallableExecutionTransformer : public TAsyncCallbackTransformerQuery().PrepareOnly; - auto future = prepareOnly ? CreateDummySuccess() : Gateway->DropGroup(cluster, dropGroupSettings); + auto future = Gateway->DropGroup(cluster, dropGroupSettings); return WrapFuture(future, [](const IKikimrGateway::TGenericResult& res, const TExprNode::TPtr& input, TExprContext& ctx) { diff --git a/ydb/core/kqp/provider/yql_kikimr_expr_nodes.json b/ydb/core/kqp/provider/yql_kikimr_expr_nodes.json index ac76d07bb667..506bc99fe8ac 100644 --- a/ydb/core/kqp/provider/yql_kikimr_expr_nodes.json +++ b/ydb/core/kqp/provider/yql_kikimr_expr_nodes.json @@ -279,7 +279,8 @@ "Children": [ {"Index": 0, "Name": "World", "Type": "TExprBase"}, {"Index": 1, "Name": "DataSink", "Type": "TKiDataSink"}, - {"Index": 2, "Name": "GroupName", "Type": "TCoAtom"} + {"Index": 2, "Name": "GroupName", "Type": "TCoAtom"}, + {"Index": 3, "Name": "Roles", "Type": "TCoAtomList"} ] }, { @@ -294,6 +295,17 @@ {"Index": 4, "Name": "Roles", "Type": "TCoAtomList"} ] }, + { + "Name": "TKiRenameGroup", + "Base": "TCallable", + "Match": {"Type": "Callable", "Name": "KiRenameGroup!"}, + "Children": [ + {"Index": 0, "Name": "World", "Type": "TExprBase"}, + {"Index": 1, "Name": "DataSink", "Type": "TKiDataSink"}, + {"Index": 2, "Name": "GroupName", "Type": "TCoAtom"}, + {"Index": 3, "Name": "NewName", "Type": "TCoAtom"} + ] + }, { "Name": "TKiDropGroup", "Base": "TCallable", diff --git a/ydb/core/kqp/provider/yql_kikimr_gateway.h b/ydb/core/kqp/provider/yql_kikimr_gateway.h index 2af573bd82c5..63c67494eff4 100644 --- a/ydb/core/kqp/provider/yql_kikimr_gateway.h +++ b/ydb/core/kqp/provider/yql_kikimr_gateway.h @@ -587,6 +587,7 @@ struct TDropUserSettings { struct TCreateGroupSettings { TString GroupName; + std::vector Roles; }; struct TAlterGroupSettings { @@ -600,6 +601,11 @@ struct TAlterGroupSettings { std::vector Roles; }; +struct TRenameGroupSettings { + TString GroupName; + TString NewName; +}; + struct TDropGroupSettings { TString GroupName; bool Force = false; @@ -817,6 +823,8 @@ class IKikimrGateway : public TThrRefBase { virtual NThreading::TFuture AlterGroup(const TString& cluster, TAlterGroupSettings& settings) = 0; + virtual NThreading::TFuture RenameGroup(const TString& cluster, TRenameGroupSettings& settings) = 0; + virtual NThreading::TFuture DropGroup(const TString& cluster, const TDropGroupSettings& settings) = 0; virtual NThreading::TFuture CreateColumnTable(TKikimrTableMetadataPtr metadata, bool createDir) = 0; diff --git a/ydb/core/kqp/provider/yql_kikimr_opt_build.cpp b/ydb/core/kqp/provider/yql_kikimr_opt_build.cpp index ed03c030374d..3809402d7bc2 100644 --- a/ydb/core/kqp/provider/yql_kikimr_opt_build.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_opt_build.cpp @@ -563,6 +563,18 @@ bool ExploreTx(TExprBase node, TExprContext& ctx, const TKiDataSink& dataSink, T return result; } + if (auto maybeRenameGroup = node.Maybe()) { + auto renameGroup = maybeRenameGroup.Cast(); + if (!checkDataSink(renameGroup.DataSink())) { + return false; + } + + txRes.Ops.insert(node.Raw()); + auto result = ExploreTx(renameGroup.World(), ctx, dataSink, txRes, tablesData, types); + txRes.AddTableOperation(BuildYdbOpNode(cluster, TYdbOperation::RenameGroup, renameGroup.Pos(), ctx)); + return result; + } + if (auto maybeDropGroup = node.Maybe()) { auto dropGroup = maybeDropGroup.Cast(); if (!checkDataSink(dropGroup.DataSink())) { diff --git a/ydb/core/kqp/provider/yql_kikimr_provider.cpp b/ydb/core/kqp/provider/yql_kikimr_provider.cpp index 3e8b740cbb4b..f3291813cb8f 100644 --- a/ydb/core/kqp/provider/yql_kikimr_provider.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_provider.cpp @@ -57,6 +57,7 @@ struct TKikimrData { DataSinkNames.insert(TKiDropObject::CallableName()); DataSinkNames.insert(TKiCreateGroup::CallableName()); DataSinkNames.insert(TKiAlterGroup::CallableName()); + DataSinkNames.insert(TKiRenameGroup::CallableName()); DataSinkNames.insert(TKiDropGroup::CallableName()); DataSinkNames.insert(TKiDataQueryBlock::CallableName()); DataSinkNames.insert(TKiDataQueryBlocks::CallableName()); @@ -105,6 +106,7 @@ struct TKikimrData { TYdbOperation::CreateGroup | TYdbOperation::AlterGroup | TYdbOperation::DropGroup | + TYdbOperation::RenameGroup | TYdbOperation::ModifyPermission; SystemColumns = { diff --git a/ydb/core/kqp/provider/yql_kikimr_provider.h b/ydb/core/kqp/provider/yql_kikimr_provider.h index 1addc4d48643..549a34235785 100644 --- a/ydb/core/kqp/provider/yql_kikimr_provider.h +++ b/ydb/core/kqp/provider/yql_kikimr_provider.h @@ -235,7 +235,8 @@ enum class TYdbOperation : ui32 { CreateTopic = 1 << 19, AlterTopic = 1 << 20, DropTopic = 1 << 21, - ModifyPermission = 1 << 22 + ModifyPermission = 1 << 22, + RenameGroup = 1 << 23 }; Y_DECLARE_FLAGS(TYdbOperations, TYdbOperation); diff --git a/ydb/core/kqp/provider/yql_kikimr_provider_impl.h b/ydb/core/kqp/provider/yql_kikimr_provider_impl.h index ed74f09d1779..34619138c6f3 100644 --- a/ydb/core/kqp/provider/yql_kikimr_provider_impl.h +++ b/ydb/core/kqp/provider/yql_kikimr_provider_impl.h @@ -53,6 +53,7 @@ class TKiSinkVisitorTransformer : public TSyncTransformerBase { virtual TStatus HandleDropObject(NNodes::TKiDropObject node, TExprContext& ctx) = 0; virtual TStatus HandleCreateGroup(NNodes::TKiCreateGroup node, TExprContext& ctx) = 0; virtual TStatus HandleAlterGroup(NNodes::TKiAlterGroup node, TExprContext& ctx) = 0; + virtual TStatus HandleRenameGroup(NNodes::TKiRenameGroup node, TExprContext& ctx) = 0; virtual TStatus HandleDropGroup(NNodes::TKiDropGroup node, TExprContext& ctx) = 0; virtual TStatus HandleWrite(NNodes::TExprBase node, TExprContext& ctx) = 0; virtual TStatus HandleCommit(NNodes::TCoCommit node, TExprContext& ctx) = 0; diff --git a/ydb/core/kqp/provider/yql_kikimr_type_ann.cpp b/ydb/core/kqp/provider/yql_kikimr_type_ann.cpp index adac64948509..6ac78264aa5b 100644 --- a/ydb/core/kqp/provider/yql_kikimr_type_ann.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_type_ann.cpp @@ -1612,6 +1612,12 @@ virtual TStatus HandleCreateTable(TKiCreateTable create, TExprContext& ctx) over return TStatus::Ok; } + virtual TStatus HandleRenameGroup(TKiRenameGroup node, TExprContext& ctx) override { + Y_UNUSED(ctx); + node.Ptr()->SetTypeAnn(node.World().Ref().GetTypeAnn()); + return TStatus::Ok; + } + virtual TStatus HandleDropGroup(TKiDropGroup node, TExprContext& ctx) override { for (const auto& setting : node.Settings()) { auto name = setting.Name().Value(); diff --git a/ydb/core/kqp/session_actor/kqp_query_state.h b/ydb/core/kqp/session_actor/kqp_query_state.h index 60970b25880f..209224b19755 100644 --- a/ydb/core/kqp/session_actor/kqp_query_state.h +++ b/ydb/core/kqp/session_actor/kqp_query_state.h @@ -119,6 +119,8 @@ class TKqpQueryState : public TNonCopyable { TString ReplayMessage; + NYql::TIssues Issues; + NKikimrKqp::EQueryAction GetAction() const { return RequestEv->GetAction(); } diff --git a/ydb/core/kqp/session_actor/kqp_session_actor.cpp b/ydb/core/kqp/session_actor/kqp_session_actor.cpp index 8e770e7c5961..741b3099fe9b 100644 --- a/ydb/core/kqp/session_actor/kqp_session_actor.cpp +++ b/ydb/core/kqp/session_actor/kqp_session_actor.cpp @@ -1286,6 +1286,10 @@ class TKqpSessionActor : public TActorBootstrapped { exec->Swap(executerResults.MutableStats()); } + if (!response->GetIssues().empty()){ + NYql::IssuesFromMessage(response->GetIssues(), QueryState->Issues); + } + ExecuteOrDefer(); } @@ -1457,6 +1461,8 @@ class TKqpSessionActor : public TActorBootstrapped { AddQueryIssues(*response, QueryState->CompileResult->Issues); } + AddQueryIssues(*response, QueryState->Issues); + FillStats(record); if (QueryState->TxCtx) { diff --git a/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp b/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp index 766ac20bee59..fbd6c5fa5907 100644 --- a/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp +++ b/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp @@ -3195,7 +3195,6 @@ Y_UNIT_TEST_SUITE(KqpScheme) { Y_UNIT_TEST(CreateAndDropGroup) { TKikimrRunner kikimr; auto db = kikimr.GetTableClient(); - /* TODO: Fix flaky test in KIKIMR-18780 { // Drop non-existing group force auto query = TStringBuilder() << R"( @@ -3205,7 +3204,7 @@ Y_UNIT_TEST_SUITE(KqpScheme) { auto session = db.CreateSession().GetValueSync().GetSession(); auto result = session.ExecuteSchemeQuery(query).GetValueSync(); UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); - }*/ + } { auto query = TStringBuilder() << R"( --!syntax_v1 @@ -3234,7 +3233,6 @@ Y_UNIT_TEST_SUITE(KqpScheme) { auto result = session.ExecuteSchemeQuery(query).GetValueSync(); UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); } - /* TODO: Fix flaky test in KIKIMR-18780 { // Drop existing group force auto query = TStringBuilder() << R"( @@ -3244,15 +3242,22 @@ Y_UNIT_TEST_SUITE(KqpScheme) { auto session = db.CreateSession().GetValueSync().GetSession(); auto result = session.ExecuteSchemeQuery(query).GetValueSync(); UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); - }*/ + } { // Drop existing group - auto query = TStringBuilder() << R"( + auto query1 = TStringBuilder() << R"( --!syntax_v1 - DROP GROUP group1; + CREATE GROUP group1; )"; auto session = db.CreateSession().GetValueSync().GetSession(); - auto result = session.ExecuteSchemeQuery(query).GetValueSync(); + auto result = session.ExecuteSchemeQuery(query1).GetValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + auto query2 = TStringBuilder() << R"( + --!syntax_v1 + DROP GROUP group1; + )"; + result = session.ExecuteSchemeQuery(query2).GetValueSync(); UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); } } diff --git a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp index aafd22651bb7..666b376f6216 100644 --- a/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp +++ b/ydb/core/kqp/ut/service/kqp_qs_queries_ut.cpp @@ -584,8 +584,20 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { SELECT * FROM TestDdl; )", TTxControl::BeginTx().CommitTx()).ExtractValueSync(); UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, result.GetIssues().ToString()); + } - result = db.ExecuteQuery(R"( + Y_UNIT_TEST(DdlUser) { + NKikimrConfig::TAppConfig appConfig; + appConfig.MutableTableServiceConfig()->SetEnablePreparedDdl(true); + auto setting = NKikimrKqp::TKqpSetting(); + auto serverSettings = TKikimrSettings() + .SetAppConfig(appConfig) + .SetKqpSettings({setting}); + + TKikimrRunner kikimr(serverSettings); + auto db = kikimr.GetQueryClient(); + + auto result = db.ExecuteQuery(R"( CREATE USER user1 PASSWORD 'password1'; )", TTxControl::NoTx()).ExtractValueSync(); UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); @@ -605,6 +617,11 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { )", TTxControl::NoTx()).ExtractValueSync(); UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + result = db.ExecuteQuery(R"( + ALTER USER user1 WITH ENCRYPTED PASSWORD 'password3'; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::PRECONDITION_FAILED, result.GetIssues().ToString()); + result = db.ExecuteQuery(R"( DROP USER user1; )", TTxControl::NoTx()).ExtractValueSync(); @@ -616,6 +633,200 @@ Y_UNIT_TEST_SUITE(KqpQueryService) { UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); } + Y_UNIT_TEST(DdlGroup) { + NKikimrConfig::TAppConfig appConfig; + appConfig.MutableTableServiceConfig()->SetEnablePreparedDdl(true); + auto setting = NKikimrKqp::TKqpSetting(); + auto serverSettings = TKikimrSettings() + .SetAppConfig(appConfig) + .SetKqpSettings({setting}); + + TKikimrRunner kikimr(serverSettings); + auto db = kikimr.GetQueryClient(); + + // Check create and drop group + auto result = db.ExecuteQuery(R"( + CREATE GROUP group1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + CREATE GROUP group1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::PRECONDITION_FAILED, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + DROP GROUP group1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + CREATE GROUP group1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + DROP GROUP group2; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::PRECONDITION_FAILED, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + DROP GROUP IF EXISTS group2; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + // Check rename group + result = db.ExecuteQuery(R"( + ALTER GROUP group1 RENAME TO group2; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + CREATE GROUP group1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + CREATE GROUP group2; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::PRECONDITION_FAILED, result.GetIssues().ToString()); + + // Check add and drop users + result = db.ExecuteQuery(R"( + CREATE USER user1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + CREATE USER user2; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + ALTER GROUP group1 ADD USER user1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + ALTER GROUP group1 ADD USER user1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetIssues().Size() == 1); + UNIT_ASSERT(result.GetIssues().ToOneLineString() == "{
: Info: Success, code: 4 subissue: {
: Info: Role \"user1\" is already a member of role \"group1\", code: 2 } }"); + + result = db.ExecuteQuery(R"( + ALTER GROUP group1 ADD USER user3; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::PRECONDITION_FAILED, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + ALTER GROUP group1 DROP USER user1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + ALTER GROUP group1 DROP USER user1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetIssues().Size() == 1); + UNIT_ASSERT(result.GetIssues().ToOneLineString() == "{
: Warning: Success, code: 4 subissue: {
: Warning: Role \"user1\" is not a member of role \"group1\", code: 3 } }"); + + result = db.ExecuteQuery(R"( + ALTER GROUP group1 DROP USER user3; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetIssues().Size() == 1); + UNIT_ASSERT(result.GetIssues().ToOneLineString() == "{
: Warning: Success, code: 4 subissue: {
: Warning: Role \"user3\" is not a member of role \"group1\", code: 3 } }"); + + result = db.ExecuteQuery(R"( + ALTER GROUP group1 ADD USER user1, user3, user2; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::PRECONDITION_FAILED, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + ALTER GROUP group1 ADD USER user1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetIssues().Size() == 1); + UNIT_ASSERT(result.GetIssues().ToOneLineString() == "{
: Info: Success, code: 4 subissue: {
: Info: Role \"user1\" is already a member of role \"group1\", code: 2 } }"); + + result = db.ExecuteQuery(R"( + ALTER GROUP group1 ADD USER user2; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetIssues().Size() == 0); + + result = db.ExecuteQuery(R"( + ALTER GROUP group1 DROP USER user1, user3, user2; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetIssues().Size() == 1); + UNIT_ASSERT(result.GetIssues().ToOneLineString() == "{
: Warning: Success, code: 4 subissue: {
: Warning: Role \"user3\" is not a member of role \"group1\", code: 3 } }"); + + result = db.ExecuteQuery(R"( + ALTER GROUP group1 DROP USER user1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetIssues().Size() == 1); + UNIT_ASSERT(result.GetIssues().ToOneLineString() == "{
: Warning: Success, code: 4 subissue: {
: Warning: Role \"user1\" is not a member of role \"group1\", code: 3 } }"); + + result = db.ExecuteQuery(R"( + ALTER GROUP group1 DROP USER user2; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetIssues().Size() == 1); + UNIT_ASSERT(result.GetIssues().ToOneLineString() == "{
: Warning: Success, code: 4 subissue: {
: Warning: Role \"user2\" is not a member of role \"group1\", code: 3 } }"); + + //Check create with users + result = db.ExecuteQuery(R"( + CREATE GROUP group3 WITH USER user1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + CREATE GROUP group3; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::PRECONDITION_FAILED, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + ALTER GROUP group3 ADD USER user1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetIssues().Size() == 1); + UNIT_ASSERT(result.GetIssues().ToOneLineString() == "{
: Info: Success, code: 4 subissue: {
: Info: Role \"user1\" is already a member of role \"group3\", code: 2 } }"); + + result = db.ExecuteQuery(R"( + ALTER GROUP group3 ADD USER user2; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + CREATE GROUP group4 WITH USER user1, user3, user2; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::PRECONDITION_FAILED, result.GetIssues().ToString()); + + result = db.ExecuteQuery(R"( + CREATE GROUP group4; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::PRECONDITION_FAILED, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetIssues().Size() == 1); + UNIT_ASSERT(result.GetIssues().ToOneLineString() == "{
: Error: Group already exists, code: 2029 }"); + + result = db.ExecuteQuery(R"( + ALTER GROUP group4 ADD USER user1; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetIssues().Size() == 1); + UNIT_ASSERT(result.GetIssues().ToOneLineString() == "{
: Info: Success, code: 4 subissue: {
: Info: Role \"user1\" is already a member of role \"group4\", code: 2 } }"); + + result = db.ExecuteQuery(R"( + ALTER GROUP group4 ADD USER user2; + )", TTxControl::NoTx()).ExtractValueSync(); + UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SUCCESS, result.GetIssues().ToString()); + UNIT_ASSERT(result.GetIssues().Size() == 0); + + } + Y_UNIT_TEST(DdlCache) { NKikimrConfig::TAppConfig appConfig; appConfig.MutableTableServiceConfig()->SetEnablePreparedDdl(true); diff --git a/ydb/core/protos/flat_scheme_op.proto b/ydb/core/protos/flat_scheme_op.proto index 9e211a93912e..792c4ac4c215 100644 --- a/ydb/core/protos/flat_scheme_op.proto +++ b/ydb/core/protos/flat_scheme_op.proto @@ -707,8 +707,14 @@ message TLoginRemoveGroupMembership { optional string Member = 2; } +message TLoginRenameGroup { + optional string Group = 1; + optional string NewName = 2; +} + message TLoginRemoveGroup { optional string Group = 1; + optional bool MissingOk = 2; } message TAlterLogin { @@ -720,6 +726,7 @@ message TAlterLogin { TLoginAddGroupMembership AddGroupMembership = 5; TLoginRemoveGroupMembership RemoveGroupMembership = 6; TLoginRemoveGroup RemoveGroup = 7; + TLoginRenameGroup RenameGroup = 8; } } diff --git a/ydb/core/protos/flat_tx_scheme.proto b/ydb/core/protos/flat_tx_scheme.proto index 286e29b84b90..2a7488687896 100644 --- a/ydb/core/protos/flat_tx_scheme.proto +++ b/ydb/core/protos/flat_tx_scheme.proto @@ -6,6 +6,7 @@ import "ydb/core/protos/subdomains.proto"; import "ydb/core/protos/bind_channel_storage_pool.proto"; import "ydb/core/protos/flat_scheme_op.proto"; import "ydb/public/api/protos/ydb_cms.proto"; +import "ydb/public/api/protos/ydb_issue_message.proto"; package NKikimrScheme; option java_package = "ru.yandex.kikimr.proto"; @@ -97,6 +98,7 @@ message TEvModifySchemeTransactionResult { optional uint64 PathId = 5; optional uint64 PathCreateTxId = 6; optional uint64 PathDropTxId = 7; + repeated Ydb.Issue.IssueMessage Issues = 8; } message TEvDescribeSchemeResult { diff --git a/ydb/core/protos/kqp_physical.proto b/ydb/core/protos/kqp_physical.proto index 749bc7f06c4a..42b8708a3f3f 100644 --- a/ydb/core/protos/kqp_physical.proto +++ b/ydb/core/protos/kqp_physical.proto @@ -389,6 +389,11 @@ message TKqpSchemeOperation { NKikimrSchemeOp.TModifyScheme CreateUser = 5; NKikimrSchemeOp.TModifyScheme AlterUser = 6; NKikimrSchemeOp.TModifyScheme DropUser = 7; + NKikimrSchemeOp.TModifyScheme CreateGroup = 8; + NKikimrSchemeOp.TModifyScheme AddGroupMembership = 9; + NKikimrSchemeOp.TModifyScheme RemoveGroupMembership = 10; + NKikimrSchemeOp.TModifyScheme DropGroup = 11; + NKikimrSchemeOp.TModifyScheme RenameGroup = 12; } } diff --git a/ydb/core/tx/schemeshard/schemeshard.h b/ydb/core/tx/schemeshard/schemeshard.h index 4bb36375ef04..be4b061765c1 100644 --- a/ydb/core/tx/schemeshard/schemeshard.h +++ b/ydb/core/tx/schemeshard/schemeshard.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -226,6 +227,16 @@ struct TEvSchemeShard { Record.SetReason(errStr); } + void AddWarning(const TString& text) { + auto issue = MakeIssue(NKikimrIssues::TIssuesIds::WARNING, text); + NYql::IssueToMessage(issue, Record.AddIssues()); + } + + void AddNotice(const TString& text) { + auto issue = MakeIssue(NKikimrIssues::TIssuesIds::INFO, text); + NYql::IssueToMessage(issue, Record.AddIssues()); + } + void SetPathCreateTxId(ui64 txId) { Record.SetPathCreateTxId(txId); } void SetPathDropTxId(ui64 txId) { Record.SetPathDropTxId(txId); } void SetPathId(ui64 pathId) { Record.SetPathId(pathId); } diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp index f40820a46b51..a4a5fd6d2533 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp @@ -97,6 +97,9 @@ class TAlterLogin: public TSubOperationBase { } else { db.Table().Key(addGroupMembership.GetGroup(), addGroupMembership.GetMember()).Update(); result->SetStatus(NKikimrScheme::StatusSuccess); + if (response.Notice) { + result->AddNotice(response.Notice); + } } break; } @@ -111,13 +114,39 @@ class TAlterLogin: public TSubOperationBase { } else { db.Table().Key(removeGroupMembership.GetGroup(), removeGroupMembership.GetMember()).Delete(); result->SetStatus(NKikimrScheme::StatusSuccess); + if (response.Warning) { + result->AddWarning(response.Warning); + } + } + break; + } + case NKikimrSchemeOp::TAlterLogin::kRenameGroup: { + const auto& renameGroup = alterLogin.GetRenameGroup(); + const TString& group = renameGroup.GetGroup(); + const TString& newName = renameGroup.GetNewName(); + auto response = context.SS->LoginProvider.RenameGroup({ + .Group = group, + .NewName = newName + }); + if (response.Error) { + result->SetStatus(NKikimrScheme::StatusPreconditionFailed, response.Error); + } else { + db.Table().Key(group).Delete(); + for (const TString& parent : response.TouchedGroups) { + db.Table().Key(parent, group).Delete(); + db.Table().Key(parent, newName).Update(); + } + result->SetStatus(NKikimrScheme::StatusSuccess); } break; } case NKikimrSchemeOp::TAlterLogin::kRemoveGroup: { const auto& removeGroup = alterLogin.GetRemoveGroup(); const TString& group = removeGroup.GetGroup(); - auto response = context.SS->LoginProvider.RemoveGroup({.Group = group}); + auto response = context.SS->LoginProvider.RemoveGroup({ + .Group = group, + .MissingOk = removeGroup.GetMissingOk() + }); if (response.Error) { result->SetStatus(NKikimrScheme::StatusPreconditionFailed, response.Error); } else { diff --git a/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp b/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp index 4cef83929258..7a31db701843 100644 --- a/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp @@ -38,6 +38,8 @@ TString DefineUserOperationName(const NKikimrSchemeOp::TModifyScheme& tx) { return "ADD GROUP MEMBERSHIP"; case NKikimrSchemeOp::TAlterLogin::kRemoveGroupMembership: return "REMOVE GROUP MEMBERSHIP"; + case NKikimrSchemeOp::TAlterLogin::kRenameGroup: + return "RENAME GROUP"; case NKikimrSchemeOp::TAlterLogin::kRemoveGroup: return "REMOVE GROUP"; default: @@ -612,6 +614,9 @@ TChangeLogin ExtractLoginChange(const NKikimrSchemeOp::TModifyScheme& tx) { result.LoginGroup = tx.GetAlterLogin().GetRemoveGroupMembership().GetGroup(); result.LoginMember = tx.GetAlterLogin().GetRemoveGroupMembership().GetMember(); break; + case NKikimrSchemeOp::TAlterLogin::kRenameGroup: + result.LoginGroup = tx.GetAlterLogin().GetRenameGroup().GetGroup(); + break; case NKikimrSchemeOp::TAlterLogin::kRemoveGroup: result.LoginGroup = tx.GetAlterLogin().GetRemoveGroup().GetGroup(); break; diff --git a/ydb/core/tx/tx_proxy/schemereq.cpp b/ydb/core/tx/tx_proxy/schemereq.cpp index 6c4cde6d86e2..b3fafc13a948 100644 --- a/ydb/core/tx/tx_proxy/schemereq.cpp +++ b/ydb/core/tx/tx_proxy/schemereq.cpp @@ -431,6 +431,11 @@ struct TBaseSchemeReq: public TActorBootstrapped { if (shardResult->HasPathDropTxId()) { result->Record.SetPathDropTxId(shardResult->GetPathDropTxId()); } + + for (const auto& issue : shardResult->GetIssues()) { + auto newIssue = result->Record.AddIssues(); + newIssue->CopyFrom(issue); + } } else { switch (status) { case TEvTxUserProxy::TEvProposeTransactionStatus::EStatus::ResolveError: diff --git a/ydb/library/login/login.cpp b/ydb/library/login/login.cpp index 45bd462bf765..9e4bcd646156 100644 --- a/ydb/library/login/login.cpp +++ b/ydb/library/login/login.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -160,7 +161,12 @@ TLoginProvider::TBasicResponse TLoginProvider::AddGroupMembership(const TAddGrou } TSidRecord& group = itGroupModify->second; - group.Members.insert(request.Member); + + if (group.Members.count(request.Member)) { + response.Notice = TStringBuilder() << "Role \"" << request.Member << "\" is already a member of role \"" << group.Name << "\""; + } else { + group.Members.insert(request.Member); + } ChildToParentIndex[request.Member].insert(request.Group); @@ -177,19 +183,77 @@ TLoginProvider::TBasicResponse TLoginProvider::RemoveGroupMembership(const TRemo } TSidRecord& group = itGroupModify->second; - group.Members.erase(request.Member); + + if (!group.Members.count(request.Member)) { + response.Warning = TStringBuilder() << "Role \"" << request.Member << "\" is not a member of role \"" << group.Name << "\""; + } else { + group.Members.erase(request.Member); + } ChildToParentIndex[request.Member].erase(request.Group); return response; } +TLoginProvider::TRenameGroupResponse TLoginProvider::RenameGroup(const TRenameGroupRequest& request) { + TRenameGroupResponse response; + + if (request.Options.CheckName && !CheckAllowedName(request.NewName)) { + response.Error = "Name is not allowed"; + return response; + } + + auto itGroupModify = Sids.find(request.Group); + if (itGroupModify == Sids.end() || itGroupModify->second.Type != ESidType::GROUP) { + response.Error = "Group not found"; + return response; + } + + auto itGroupCreate = Sids.emplace(request.NewName, TSidRecord{.Type = ESidType::GROUP}); + if (!itGroupCreate.second) { + if (itGroupCreate.first->second.Type == ESidType::GROUP) { + response.Error = "Group already exists"; + } else { + response.Error = "Account already exists"; + } + return response; + } + + TSidRecord& group = itGroupCreate.first->second; + group.Name = request.NewName; + + auto itChildToParentIndex = ChildToParentIndex.find(request.Group); + if (itChildToParentIndex != ChildToParentIndex.end()) { + ChildToParentIndex[request.NewName] = itChildToParentIndex->second; + for (const TString& parent : ChildToParentIndex[request.NewName]) { + auto itGroup = Sids.find(parent); + if (itGroup != Sids.end()) { + response.TouchedGroups.emplace_back(itGroup->first); + itGroup->second.Members.erase(request.Group); + itGroup->second.Members.insert(request.NewName); + } + } + ChildToParentIndex.erase(itChildToParentIndex); + } + + for (const TString& member : itGroupModify->second.Members) { + ChildToParentIndex[member].erase(request.Group); + ChildToParentIndex[member].insert(request.NewName); + } + + Sids.erase(itGroupModify); + + return response; +} + TLoginProvider::TRemoveGroupResponse TLoginProvider::RemoveGroup(const TRemoveGroupRequest& request) { TRemoveGroupResponse response; auto itGroupModify = Sids.find(request.Group); if (itGroupModify == Sids.end() || itGroupModify->second.Type != ESidType::GROUP) { - response.Error = "Group not found"; + if (!request.MissingOk) { + response.Error = "Group not found"; + } return response; } diff --git a/ydb/library/login/login.h b/ydb/library/login/login.h index 758520dc75e8..7092e7ed2b45 100644 --- a/ydb/library/login/login.h +++ b/ydb/library/login/login.h @@ -30,6 +30,8 @@ class TLoginProvider { struct TBasicResponse { TString Error; + TString Warning; + TString Notice; }; struct TLoginUserRequest : TBasicRequest { @@ -99,8 +101,23 @@ class TLoginProvider { TString Member; }; + struct TRenameGroupRequest : TBasicRequest { + struct TOptions { + bool CheckName = true; + }; + + TString Group; + TString NewName; + TOptions Options; + }; + + struct TRenameGroupResponse : TBasicResponse { + std::vector TouchedGroups; + }; + struct TRemoveGroupRequest : TBasicRequest { TString Group; + bool MissingOk; }; struct TRemoveGroupResponse : TBasicResponse { @@ -152,6 +169,7 @@ class TLoginProvider { TBasicResponse CreateGroup(const TCreateGroupRequest& request); TBasicResponse AddGroupMembership(const TAddGroupMembershipRequest& request); TBasicResponse RemoveGroupMembership(const TRemoveGroupMembershipRequest& request); + TRenameGroupResponse RenameGroup(const TRenameGroupRequest& request); TRemoveGroupResponse RemoveGroup(const TRemoveGroupRequest& request); TLoginProvider(); diff --git a/ydb/library/login/login_ut.cpp b/ydb/library/login/login_ut.cpp index 4ace83b1ebe5..7e36047092fe 100644 --- a/ydb/library/login/login_ut.cpp +++ b/ydb/library/login/login_ut.cpp @@ -152,14 +152,60 @@ Y_UNIT_TEST_SUITE(Login) { UNIT_ASSERT(Count(groups, "group5") == 1); } { - auto response1 = provider.RemoveGroupMembership({.Group = "group2", .Member = "group4"}); + auto response1 = provider.RenameGroup({.Group = "group3", .NewName = "group33"}); UNIT_ASSERT(!response1.Error); + + auto sids = provider.Sids; + UNIT_ASSERT(sids.size() == 6); + UNIT_ASSERT(sids.count("user1") == 1); + UNIT_ASSERT(sids.count("group1") == 1); + UNIT_ASSERT(sids.count("group2") == 1); + UNIT_ASSERT(sids.count("group33") == 1); + UNIT_ASSERT(sids.count("group4") == 1); + UNIT_ASSERT(sids.count("group5") == 1); + + auto groups = provider.GetGroupsMembership("user1"); + UNIT_ASSERT(groups.size() == 5); + UNIT_ASSERT(Count(groups, "group1") == 1); + UNIT_ASSERT(Count(groups, "group2") == 1); + UNIT_ASSERT(Count(groups, "group33") == 1); + UNIT_ASSERT(Count(groups, "group4") == 1); + UNIT_ASSERT(Count(groups, "group5") == 1); + + groups = provider.GetGroupsMembership("group33"); + UNIT_ASSERT(groups.size() == 1); + UNIT_ASSERT(Count(groups, "group1") == 1); + + groups = provider.GetGroupsMembership("group5"); + UNIT_ASSERT(groups.size() == 2); + UNIT_ASSERT(Count(groups, "group33") == 1); + UNIT_ASSERT(Count(groups, "group1") == 1); + + groups = provider.GetGroupsMembership("group4"); + UNIT_ASSERT(groups.size() == 2); + UNIT_ASSERT(Count(groups, "group2") == 1); + UNIT_ASSERT(Count(groups, "group1") == 1); + } + { + auto response1 = provider.AddGroupMembership({.Group = "group2", .Member = {"group4"}}); + UNIT_ASSERT(!response1.Error); + UNIT_ASSERT(response1.Notice == "Role \"group4\" is already a member of role \"group2\""); + } + { + auto response1 = provider.RemoveGroupMembership({.Group = "group2", .Member = {"group4"}}); + UNIT_ASSERT(!response1.Error); + UNIT_ASSERT(!response1.Warning); + } + { + auto response1 = provider.RemoveGroupMembership({.Group = "group2", .Member = {"group4"}}); + UNIT_ASSERT(!response1.Error); + UNIT_ASSERT(response1.Warning == "Role \"group4\" is not a member of role \"group2\""); } { auto groups = provider.GetGroupsMembership("user1"); UNIT_ASSERT(groups.size() == 4); UNIT_ASSERT(Count(groups, "group1") == 1); - UNIT_ASSERT(Count(groups, "group3") == 1); + UNIT_ASSERT(Count(groups, "group33") == 1); UNIT_ASSERT(Count(groups, "group4") == 1); UNIT_ASSERT(Count(groups, "group5") == 1); } diff --git a/ydb/library/yql/providers/common/provider/yql_provider.cpp b/ydb/library/yql/providers/common/provider/yql_provider.cpp index 9b240dac4351..fc62e922c874 100644 --- a/ydb/library/yql/providers/common/provider/yql_provider.cpp +++ b/ydb/library/yql/providers/common/provider/yql_provider.cpp @@ -477,7 +477,8 @@ TWriteTopicSettings ParseWriteTopicSettings(TExprList node, TExprContext& ctx) { TWriteRoleSettings ParseWriteRoleSettings(TExprList node, TExprContext& ctx) { TMaybeNode mode; - TMaybeNode roles; + TVector roles; + TMaybeNode newName; TVector other; for (auto child : node) { if (auto maybeTuple = child.Maybe()) { @@ -489,19 +490,29 @@ TWriteRoleSettings ParseWriteRoleSettings(TExprList node, TExprContext& ctx) { mode = tuple.Value().Cast(); } else if (name == "roles") { YQL_ENSURE(tuple.Value().Maybe()); - roles = tuple.Value().Cast(); + for (const auto& item : tuple.Value().Cast()) { + roles.push_back(item); + } + } else if (name == "newName") { + YQL_ENSURE(tuple.Value().Maybe()); + newName = tuple.Value().Cast(); } else { other.push_back(tuple); } } } + const auto& builtRoles = Build(ctx, node.Pos()) + .Add(roles) + .Done(); + const auto& otherSettings = Build(ctx, node.Pos()) .Add(other) .Done(); TWriteRoleSettings ret(otherSettings); - ret.Roles = roles; + ret.Roles = builtRoles;; + ret.NewName = newName; ret.Mode = mode; return ret; diff --git a/ydb/library/yql/providers/common/provider/yql_provider.h b/ydb/library/yql/providers/common/provider/yql_provider.h index 9b86ff3f2952..a6ba6d285d79 100644 --- a/ydb/library/yql/providers/common/provider/yql_provider.h +++ b/ydb/library/yql/providers/common/provider/yql_provider.h @@ -74,6 +74,7 @@ struct TWriteTopicSettings { struct TWriteRoleSettings { NNodes::TMaybeNode Mode; NNodes::TMaybeNode Roles; + NNodes::TMaybeNode NewName; NNodes::TCoNameValueTupleList Other; TWriteRoleSettings(const NNodes::TCoNameValueTupleList& other) diff --git a/ydb/library/yql/sql/v1/SQLv1.g.in b/ydb/library/yql/sql/v1/SQLv1.g.in index fe76d2246ad8..3d7c0c7d6d5d 100644 --- a/ydb/library/yql/sql/v1/SQLv1.g.in +++ b/ydb/library/yql/sql/v1/SQLv1.g.in @@ -722,7 +722,7 @@ drop_table_stmt: DROP (TABLE | TABLESTORE | EXTERNAL TABLE) (IF EXISTS)? simple_ create_user_stmt: CREATE USER role_name create_user_option?; alter_user_stmt: ALTER USER role_name (WITH? create_user_option | RENAME TO role_name); -create_group_stmt: CREATE GROUP role_name; +create_group_stmt: CREATE GROUP role_name (WITH USER role_name (COMMA role_name)* COMMA?)?; alter_group_stmt: ALTER GROUP role_name ((ADD|DROP) USER role_name (COMMA role_name)* COMMA? | RENAME TO role_name); drop_role_stmt: DROP (USER|GROUP) (IF EXISTS)? role_name (COMMA role_name)* COMMA?; diff --git a/ydb/library/yql/sql/v1/format/sql_format_ut.cpp b/ydb/library/yql/sql/v1/format/sql_format_ut.cpp index 62e3d1648208..5e8c62832725 100644 --- a/ydb/library/yql/sql/v1/format/sql_format_ut.cpp +++ b/ydb/library/yql/sql/v1/format/sql_format_ut.cpp @@ -133,6 +133,8 @@ Y_UNIT_TEST_SUITE(CheckSqlFormatter) { Y_UNIT_TEST(CreateGroup) { TCases cases = { {"use plato;create group user;","USE plato;\n\nCREATE GROUP user;\n"}, + {"use plato;create group user with user user;","USE plato;\n\nCREATE GROUP user WITH USER user;\n"}, + {"use plato;create group user with user user, user,;","USE plato;\n\nCREATE GROUP user WITH USER user, user,;\n"}, }; TSetup setup; diff --git a/ydb/library/yql/sql/v1/node.h b/ydb/library/yql/sql/v1/node.h index 865ecd231d8e..efaf2f2687e1 100644 --- a/ydb/library/yql/sql/v1/node.h +++ b/ydb/library/yql/sql/v1/node.h @@ -1078,6 +1078,7 @@ namespace NSQLTranslationV1 { struct TRoleParameters { TMaybe Password; bool IsPasswordEncrypted = false; + TVector Roles; }; struct TTopicConsumerSettings { @@ -1218,7 +1219,7 @@ namespace NSQLTranslationV1 { // Implemented in query.cpp TNodePtr BuildCreateUser(TPosition pos, const TString& service, const TDeferredAtom& cluster, const TDeferredAtom& name, const TMaybe& params, TScopedStatePtr scoped); - TNodePtr BuildCreateGroup(TPosition pos, const TString& service, const TDeferredAtom& cluster, const TDeferredAtom& name, TScopedStatePtr scoped); + TNodePtr BuildCreateGroup(TPosition pos, const TString& service, const TDeferredAtom& cluster, const TDeferredAtom& name, const TMaybe& params, TScopedStatePtr scoped); TNodePtr BuildAlterUser(TPosition pos, const TString& service, const TDeferredAtom& cluster, const TDeferredAtom& name, const TRoleParameters& params, TScopedStatePtr scoped); TNodePtr BuildRenameUser(TPosition pos, const TString& service, const TDeferredAtom& cluster, const TDeferredAtom& name, const TDeferredAtom& newName, TScopedStatePtr scoped); TNodePtr BuildAlterGroup(TPosition pos, const TString& service, const TDeferredAtom& cluster, const TDeferredAtom& name, const TVector& toChange, bool isDrop, diff --git a/ydb/library/yql/sql/v1/query.cpp b/ydb/library/yql/sql/v1/query.cpp index 0b8cf58ab33d..5f50eb6e01f6 100644 --- a/ydb/library/yql/sql/v1/query.cpp +++ b/ydb/library/yql/sql/v1/query.cpp @@ -1770,6 +1770,17 @@ class TCreateRole final: public TAstListNode { return false; } + TVector roles; + if (Params && !Params->Roles.empty()) { + for (auto& item : Params->Roles) { + roles.push_back(item.Build()); + if (!roles.back()->Init(ctx, FakeSource.Get())) { + return false; + } + } + } + + auto options = Y(Q(Y(Q("mode"), Q(IsUser ? "createUser" : "createGroup")))); if (Params) { if (Params->IsPasswordEncrypted) { @@ -1780,6 +1791,9 @@ class TCreateRole final: public TAstListNode { } else { options = L(options, Q(Y(Q("nullPassword")))); } + if (!Params->Roles.empty()) { + options = L(options, Q(Y(Q("roles"), Q(new TAstListNodeImpl(Pos, std::move(roles)))))); + } } Add("block", Q(Y( @@ -1809,9 +1823,9 @@ TNodePtr BuildCreateUser(TPosition pos, const TString& service, const TDeferredA return new TCreateRole(pos, isUser, service, cluster, name, params, scoped); } -TNodePtr BuildCreateGroup(TPosition pos, const TString& service, const TDeferredAtom& cluster, const TDeferredAtom& name, TScopedStatePtr scoped) { +TNodePtr BuildCreateGroup(TPosition pos, const TString& service, const TDeferredAtom& cluster, const TDeferredAtom& name, const TMaybe& params, TScopedStatePtr scoped) { bool isUser = false; - return new TCreateRole(pos, isUser, service, cluster, name, {}, scoped); + return new TCreateRole(pos, isUser, service, cluster, name, params, scoped); } class TAlterUser final: public TAstListNode { diff --git a/ydb/library/yql/sql/v1/sql_query.cpp b/ydb/library/yql/sql/v1/sql_query.cpp index 1e1af91253fb..0f82bfe5f520 100644 --- a/ydb/library/yql/sql/v1/sql_query.cpp +++ b/ydb/library/yql/sql/v1/sql_query.cpp @@ -465,7 +465,7 @@ bool TSqlQuery::Statement(TVector& blocks, const TRule_sql_stmt_core& break; } case TRule_sql_stmt_core::kAltSqlStmtCore23: { - // create_group_stmt: CREATE GROUP role_name; + // create_group_stmt: CREATE GROUP role_name (WITH USER role_name (COMMA role_name)* COMMA?)?; Ctx.BodyPart(); auto& node = core.GetAlt_sql_stmt_core23().GetRule_create_group_stmt1(); @@ -485,7 +485,25 @@ bool TSqlQuery::Statement(TVector& blocks, const TRule_sql_stmt_core& return false; } - AddStatementToBlocks(blocks, BuildCreateGroup(pos, service, cluster, roleName, Ctx.Scoped)); + TRoleParameters roleParams; + if (node.HasBlock4()) { + auto& addDropNode = node.GetBlock4(); + TVector roles; + bool allowSystemRoles = false; + roleParams.Roles.emplace_back(); + if (!RoleNameClause(addDropNode.GetRule_role_name3(), roleParams.Roles.back(), allowSystemRoles)) { + return false; + } + + for (auto& item : addDropNode.GetBlock4()) { + roleParams.Roles.emplace_back(); + if (!RoleNameClause(item.GetRule_role_name2(), roleParams.Roles.back(), allowSystemRoles)) { + return false; + } + } + } + + AddStatementToBlocks(blocks, BuildCreateGroup(pos, service, cluster, roleName, roleParams, Ctx.Scoped)); break; } case TRule_sql_stmt_core::kAltSqlStmtCore24: {