Skip to content

Commit

Permalink
Fix commits, add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
EgorkaZ committed May 28, 2024
1 parent 45edfdf commit bd60670
Show file tree
Hide file tree
Showing 13 changed files with 556 additions and 220 deletions.
1 change: 1 addition & 0 deletions ydb/core/grpc_services/ya.make
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ PEERDIR(
ydb/core/io_formats/ydb_dump
ydb/core/kesus/tablet
ydb/core/kqp/common
ydb/core/kqp/session_actor
ydb/core/protos
ydb/core/scheme
ydb/core/sys_view
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ PEERDIR(
ydb/services/metadata/abstract
ydb/services/metadata/secret
ydb/core/kqp/gateway/actors
ydb/core/kqp/federated_query
ydb/core/kqp/gateway/utils
ydb/core/kqp/gateway/behaviour/tablestore/operations
)
Expand Down
27 changes: 18 additions & 9 deletions ydb/core/kqp/gateway/kqp_metadata_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -473,8 +473,11 @@ NThreading::TFuture<TEvDescribeSecretsResponse::TDescription> LoadExternalDataSo
return DescribeExternalDataSourceSecrets(authDescription, userToken ? userToken->GetUserSID() : "", actorSystem, maximalSecretsSnapshotWaitTime);
}

} // anonymous namespace

NExternalSource::TAuth MakeAuth(const NYql::TExternalSource& metadata) {
switch (metadata.DataSourceAuth.identity_case()) {
case NKikimrSchemeOp::TAuth::IDENTITY_NOT_SET:
case NKikimrSchemeOp::TAuth::kNone:
return NExternalSource::NAuth::MakeNone();
case NKikimrSchemeOp::TAuth::kServiceAccount:
Expand All @@ -484,7 +487,6 @@ NExternalSource::TAuth MakeAuth(const NYql::TExternalSource& metadata) {
case NKikimrSchemeOp::TAuth::kBasic:
case NKikimrSchemeOp::TAuth::kMdbBasic:
case NKikimrSchemeOp::TAuth::kToken:
case NKikimrSchemeOp::TAuth::IDENTITY_NOT_SET:
Y_ABORT("Unimplemented external source auth: %d", metadata.DataSourceAuth.identity_case());
break;
}
Expand All @@ -496,6 +498,7 @@ std::shared_ptr<NExternalSource::TMetadata> ConvertToExternalSourceMetadata(cons
metadata->TableLocation = tableMetadata.ExternalSource.TableLocation;
metadata->DataSourceLocation = tableMetadata.ExternalSource.DataSourceLocation;
metadata->DataSourcePath = tableMetadata.ExternalSource.DataSourcePath;
metadata->Type = tableMetadata.ExternalSource.Type;
metadata->Attributes = tableMetadata.Attributes;
metadata->Auth = MakeAuth(tableMetadata.ExternalSource);
return metadata;
Expand Down Expand Up @@ -525,11 +528,13 @@ bool EnrichMetadata(NYql::TKikimrTableMetadata& tableMetadata, const NExternalSo
++id;
}
tableMetadata.Attributes = dynamicMetadata.Attributes;
tableMetadata.ExternalSource.TableLocation = dynamicMetadata.TableLocation;
tableMetadata.ExternalSource.DataSourceLocation = dynamicMetadata.DataSourceLocation;
tableMetadata.ExternalSource.DataSourcePath = dynamicMetadata.DataSourcePath;
tableMetadata.ExternalSource.Type = dynamicMetadata.Type;
return true;
}

} // anonymous namespace


TVector<NKikimrKqp::TKqpTableMetadataProto> TKqpTableMetadataLoader::GetCollectedSchemeData() {
TVector<NKikimrKqp::TKqpTableMetadataProto> result(std::move(CollectedSchemeData));
Expand Down Expand Up @@ -842,12 +847,16 @@ NThreading::TFuture<TTableMetadataResult> TKqpTableMetadataLoader::LoadTableMeta
externalSource->LoadDynamicMetadata(std::move(externalSourceMeta))
.Subscribe([promise = std::move(promise), externalDataSourceMetadata](const TFuture<std::shared_ptr<NExternalSource::TMetadata>>& result) mutable {
TTableMetadataResult wrapper;
if (result.HasValue() && (!result.GetValue()->Changed || EnrichMetadata(*externalDataSourceMetadata.Metadata, *result.GetValue()))) {
wrapper.SetSuccess();
wrapper.Metadata = externalDataSourceMetadata.Metadata;
} else {
// TODO: forward exception from result
wrapper.SetException(yexception() << "LoadDynamicMetadata failed");
try {
auto& dynamicMetadata = result.GetValue();
if (!dynamicMetadata->Changed || EnrichMetadata(*externalDataSourceMetadata.Metadata, *dynamicMetadata)) {
wrapper.SetSuccess();
wrapper.Metadata = externalDataSourceMetadata.Metadata;
} else {
wrapper.SetException(yexception() << "couldn't enrich metadata with dynamically loaded part");
}
} catch (const std::exception& exception) {
wrapper.SetException(yexception() << "couldn't load table metadata: " << exception.what());
}
promise.SetValue(wrapper);
});
Expand Down
5 changes: 5 additions & 0 deletions ydb/core/kqp/gateway/kqp_metadata_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@

namespace NKikimr::NKqp {

// only exposed to be unit-tested
NExternalSource::TAuth MakeAuth(const NYql::TExternalSource& metadata);
std::shared_ptr<NExternalSource::TMetadata> ConvertToExternalSourceMetadata(const NYql::TKikimrTableMetadata& tableMetadata);
bool EnrichMetadata(NYql::TKikimrTableMetadata& tableMetadata, const NExternalSource::TMetadata& dynamicMetadata);

class TKqpTableMetadataLoader : public NYql::IKikimrGateway::IKqpTableMetadataLoader {
public:

Expand Down
78 changes: 78 additions & 0 deletions ydb/core/kqp/gateway/ut/metadata_conversion.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
#include <library/cpp/testing/gtest/gtest.h>

#include <ydb/core/kqp/gateway/kqp_metadata_loader.h>
#include <ydb/core/kqp/provider/yql_kikimr_gateway.h>

using namespace NKikimr;

TEST(MetadataConversion, MakeAuthTest) {
NYql::TExternalSource externalSource;
auto auth = NKqp::MakeAuth(externalSource);
ASSERT_TRUE(std::holds_alternative<NExternalSource::NAuth::TNone>(auth));

externalSource.DataSourceAuth.MutableNone();
auth = NKqp::MakeAuth(externalSource);
ASSERT_TRUE(std::holds_alternative<NExternalSource::NAuth::TNone>(auth));

externalSource.DataSourceAuth.ClearNone();
auto& serviceAccount = *externalSource.DataSourceAuth.MutableServiceAccount();
{
serviceAccount.SetId("sa-id");
serviceAccount.SetSecretName("sa-name-of-secret");
externalSource.ServiceAccountIdSignature = "sa-id-signature";
}
auth = NKqp::MakeAuth(externalSource);
ASSERT_TRUE(std::holds_alternative<NExternalSource::NAuth::TServiceAccount>(auth));
{
auto& saAuth = std::get<NExternalSource::NAuth::TServiceAccount>(auth);
ASSERT_EQ(saAuth.ServiceAccountId, "sa-id");
ASSERT_EQ(saAuth.ServiceAccountIdSignature, "sa-id-signature");
}

externalSource.DataSourceAuth.ClearServiceAccount();
auto& awsAccount = *externalSource.DataSourceAuth.MutableAws();
{
awsAccount.SetAwsRegion("aws-test");
awsAccount.SetAwsAccessKeyIdSecretName("aws-ak-secret-name");
awsAccount.SetAwsSecretAccessKeySecretName("aws-sak-secret-name");
externalSource.AwsAccessKeyId = "aws-ak";
externalSource.AwsSecretAccessKey = "aws-sak";
}
auth = NKqp::MakeAuth(externalSource);
ASSERT_TRUE(std::holds_alternative<NExternalSource::NAuth::TAws>(auth));
{
auto& awsAuth = std::get<NExternalSource::NAuth::TAws>(auth);
ASSERT_EQ(awsAuth.Region, "aws-test");
ASSERT_EQ(awsAuth.AccessKey, "aws-ak");
ASSERT_EQ(awsAuth.SecretAccessKey, "aws-sak");
}
}

TEST(MetadataConversion, ConvertingExternalSourceMetadata) {
NYql::TExternalSource externalSource{
.Type = "type",
.TableLocation = "table-loc",
.DataSourcePath = "ds-path",
.DataSourceLocation = "ds-loc",
};
THashMap<TString, TString> attributes{{"key1", "val1"}, {"key2", "val2"}};

std::shared_ptr<NExternalSource::TMetadata> externalMetadata;
{
NYql::TKikimrTableMetadata tableMetadata;
tableMetadata.ExternalSource = externalSource;
tableMetadata.Attributes = attributes;
externalMetadata = NKqp::ConvertToExternalSourceMetadata(tableMetadata);
}
ASSERT_TRUE(externalMetadata);
ASSERT_TRUE(std::holds_alternative<NExternalSource::NAuth::TNone>(externalMetadata->Auth));

NYql::TKikimrTableMetadata tableMetadata;
ASSERT_TRUE(NKqp::EnrichMetadata(tableMetadata, *externalMetadata));

ASSERT_EQ(tableMetadata.ExternalSource.Type, externalSource.Type);
ASSERT_EQ(tableMetadata.ExternalSource.TableLocation, externalSource.TableLocation);
ASSERT_EQ(tableMetadata.ExternalSource.DataSourcePath, externalSource.DataSourcePath);
ASSERT_EQ(tableMetadata.ExternalSource.DataSourceLocation, externalSource.DataSourceLocation);
ASSERT_THAT(tableMetadata.Attributes, testing::ContainerEq(attributes));
}
17 changes: 17 additions & 0 deletions ydb/core/kqp/gateway/ut/ya.make
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
GTEST()

SRCS(
metadata_conversion.cpp
)

PEERDIR(
ydb/core/kqp/gateway
ydb/library/yql/parser/pg_wrapper
ydb/library/yql/public/udf/service/stub
ydb/services/metadata
)

YQL_LAST_ABI_VERSION()

END()

2 changes: 2 additions & 0 deletions ydb/core/kqp/gateway/ya.make
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,5 @@ RECURSE(
local_rpc
utils
)

RECURSE_FOR_TESTS(ut)
Loading

0 comments on commit bd60670

Please sign in to comment.