Skip to content

Commit 54ce71d

Browse files
authored
Merge 49903fb into c59858d
2 parents c59858d + 49903fb commit 54ce71d

File tree

4 files changed

+33
-12
lines changed

4 files changed

+33
-12
lines changed

ydb/core/kqp/executer_actor/kqp_scheme_executer.cpp

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,8 @@ class TKqpSchemeExecuter : public TActorBootstrapped<TKqpSchemeExecuter> {
500500
failedOnAlreadyExists = ev->Record.GetTransaction().GetModifyScheme().GetFailedOnAlreadyExists();
501501
}
502502

503+
const auto operationType = ev->Record.GetTransaction().GetModifyScheme().GetOperationType();
504+
503505
IActor* requestHandler = new TSchemeOpRequestHandler(
504506
ev.Release(),
505507
promise,
@@ -510,9 +512,19 @@ class TKqpSchemeExecuter : public TActorBootstrapped<TKqpSchemeExecuter> {
510512

511513
auto actorSystem = TActivationContext::ActorSystem();
512514
auto selfId = SelfId();
513-
promise.GetFuture().Subscribe([actorSystem, selfId](const TFuture<IKqpGateway::TGenericResult>& future) {
515+
promise.GetFuture().Subscribe([actorSystem, selfId, operationType](const TFuture<IKqpGateway::TGenericResult>& future) {
516+
const auto& value = future.GetValue();
514517
auto ev = MakeHolder<TEvPrivate::TEvResult>();
515-
ev->Result = future.GetValue();
518+
ev->Result.SetStatus(value.Status());
519+
520+
if (value.Issues()) {
521+
NYql::TIssue rootIssue(TStringBuilder() << "Executing " << NKikimrSchemeOp::EOperationType_Name(operationType));
522+
rootIssue.SetCode(ev->Result.Status(), NYql::TSeverityIds::S_INFO);
523+
for (const auto& issue : value.Issues()) {
524+
rootIssue.AddSubIssue(MakeIntrusive<NYql::TIssue>(issue));
525+
}
526+
ev->Result.AddIssue(rootIssue);
527+
}
516528

517529
actorSystem->Send(selfId, ev.Release());
518530
});
@@ -546,15 +558,20 @@ class TKqpSchemeExecuter : public TActorBootstrapped<TKqpSchemeExecuter> {
546558
auto resultFuture = cBehaviour->GetOperationsManager()->ExecutePrepared(schemeOp, SelfId().NodeId(), cBehaviour, context);
547559

548560
using TResultFuture = NThreading::TFuture<NMetadata::NModifications::IOperationsManager::TYqlConclusionStatus>;
549-
resultFuture.Subscribe([actorSystem, selfId](const TResultFuture& f) {
561+
resultFuture.Subscribe([actorSystem, selfId, objectType = schemeOp.GetObjectType()](const TResultFuture& f) {
550562
const auto& status = f.GetValue();
551563
auto ev = MakeHolder<TEvPrivate::TEvResult>();
552564
if (status.Ok()) {
553565
ev->Result.SetSuccess();
554566
} else {
555567
ev->Result.SetStatus(status.GetStatus());
556568
if (TString message = status.GetErrorMessage()) {
557-
ev->Result.AddIssue(NYql::TIssue(message).SetCode(status.GetStatus(), NYql::TSeverityIds::S_ERROR));
569+
const auto createIssue = [status = status.GetStatus()](const TString& message) {
570+
return NYql::TIssue(message).SetCode(status, NYql::TSeverityIds::S_ERROR);
571+
};
572+
ev->Result.AddIssue(createIssue(TStringBuilder() << "Executing operation with object \"" << objectType << "\"")
573+
.AddSubIssue(MakeIntrusive<NYql::TIssue>(createIssue(status.GetErrorMessage())))
574+
);
558575
}
559576
}
560577
actorSystem->Send(selfId, ev.Release());

ydb/core/kqp/gateway/utils/metadata_helpers.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "metadata_helpers.h"
22

33
#include <ydb/core/kqp/gateway/actors/scheme.h>
4+
#include <ydb/core/protos/schemeshard/operations.pb.h>
45

56
namespace NKikimr::NKqp {
67

@@ -32,15 +33,15 @@ TAsyncStatus SendSchemeRequest(const NKikimrSchemeOp::TModifyScheme& schemeTx, c
3233
auto promise = NThreading::NewPromise<NKqp::TSchemeOpRequestHandler::TResult>();
3334
context.GetActorSystem()->Register(new TSchemeOpRequestHandler(request.release(), promise, schemeTx.GetFailedOnAlreadyExists(), schemeTx.GetSuccessOnNotExist()));
3435

35-
return promise.GetFuture().Apply([](const NThreading::TFuture<NKqp::TSchemeOpRequestHandler::TResult>& f) {
36+
return promise.GetFuture().Apply([operationType = schemeTx.GetOperationType()](const NThreading::TFuture<NKqp::TSchemeOpRequestHandler::TResult>& f) {
3637
try {
3738
auto response = f.GetValue();
3839
if (response.Success()) {
3940
return TYqlConclusionStatus::Success();
4041
}
41-
return TYqlConclusionStatus::Fail(response.Status(), response.Issues().ToString());
42+
return TYqlConclusionStatus::Fail(response.Status(), TStringBuilder() << "Failed to execute " << NKikimrSchemeOp::EOperationType_Name(operationType) << ": " << response.Issues().ToString());
4243
} catch (...) {
43-
return TYqlConclusionStatus::Fail(NYql::TIssuesIds::KIKIMR_SCHEME_ERROR, TStringBuilder() << "Scheme error: " << CurrentExceptionMessage());
44+
return TYqlConclusionStatus::Fail(NYql::TIssuesIds::KIKIMR_SCHEME_ERROR, TStringBuilder() << "Failed to execute " << NKikimrSchemeOp::EOperationType_Name(operationType) << ", got scheme error: " << CurrentExceptionMessage());
4445
}
4546
});
4647
}

ydb/core/kqp/gateway/utils/ya.make

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ PEERDIR(
1010
ydb/core/kqp/gateway/actors
1111
ydb/core/kqp/provider
1212
ydb/core/protos
13+
ydb/core/protos/schemeshard
1314
)
1415

1516
YQL_LAST_ABI_VERSION()

ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7315,9 +7315,11 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
73157315
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "External data sources are disabled for serverless domains. Please contact your system administrator to enable it", result.GetIssues().ToString());
73167316
};
73177317

7318-
auto checkNotFound = [](const auto& result) {
7319-
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, result.GetIssues().ToString());
7320-
UNIT_ASSERT_STRING_CONTAINS_C(result.GetIssues().ToString(), "Path does not exist", result.GetIssues().ToString());
7318+
auto checkNotFound = [](const auto& result, const TString& error) {
7319+
const auto& issuesString = result.GetIssues().ToString();
7320+
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, issuesString);
7321+
UNIT_ASSERT_STRING_CONTAINS_C(issuesString, "Path does not exist", issuesString);
7322+
UNIT_ASSERT_STRING_CONTAINS_C(issuesString, error, issuesString);
73217323
};
73227324

73237325
const auto& createSourceSql = R"(
@@ -7360,8 +7362,8 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
73607362
settings.Database(ydb->GetSettings().GetServerlessTenantName()).NodeIndex(2);
73617363
checkDisabled(ydb->ExecuteQuery(createSourceSql, settings));
73627364
checkDisabled(ydb->ExecuteQuery(createTableSql, settings));
7363-
checkNotFound(ydb->ExecuteQuery(dropTableSql, settings));
7364-
checkNotFound(ydb->ExecuteQuery(dropSourceSql, settings));
7365+
checkNotFound(ydb->ExecuteQuery(dropTableSql, settings), "Executing ESchemeOpDropExternalTable");
7366+
checkNotFound(ydb->ExecuteQuery(dropSourceSql, settings), "Executing operation with object \"EXTERNAL_DATA_SOURCE\"");
73657367
}
73667368

73677369
Y_UNIT_TEST(CreateExternalDataSource) {

0 commit comments

Comments
 (0)