From 9f0b79086591c1d68ea821d4b992fc0abb8293ee Mon Sep 17 00:00:00 2001 From: Vladislav Gogov Date: Wed, 23 Oct 2024 14:43:50 +0300 Subject: [PATCH] AlterColumnTable (#10672) --- ydb/core/kqp/gateway/kqp_ic_gateway.cpp | 5 +- ydb/core/kqp/gateway/kqp_metadata_loader.cpp | 4 + ydb/core/kqp/host/kqp_gateway_proxy.cpp | 24 ++--- ydb/core/kqp/provider/yql_kikimr_exec.cpp | 11 +-- ydb/core/kqp/provider/yql_kikimr_gateway.h | 2 +- ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp | 1 + ydb/core/ydb_convert/table_description.cpp | 92 ++++++++++++++++++-- ydb/core/ydb_convert/table_description.h | 10 ++- 8 files changed, 115 insertions(+), 34 deletions(-) diff --git a/ydb/core/kqp/gateway/kqp_ic_gateway.cpp b/ydb/core/kqp/gateway/kqp_ic_gateway.cpp index dfd2457dc7f8..ca97971ff209 100644 --- a/ydb/core/kqp/gateway/kqp_ic_gateway.cpp +++ b/ydb/core/kqp/gateway/kqp_ic_gateway.cpp @@ -1071,10 +1071,9 @@ class TKikimrIcGateway : public IKqpGateway { return NotImplemented(); } - TFuture AlterColumnTable(const TString& cluster, - const NYql::TAlterColumnTableSettings& settings) override { + TFuture AlterColumnTable(const TString& cluster, Ydb::Table::AlterTableRequest&& req) override { Y_UNUSED(cluster); - Y_UNUSED(settings); + Y_UNUSED(req); return NotImplemented(); } diff --git a/ydb/core/kqp/gateway/kqp_metadata_loader.cpp b/ydb/core/kqp/gateway/kqp_metadata_loader.cpp index 1d368ccdb300..9bd481829fa7 100644 --- a/ydb/core/kqp/gateway/kqp_metadata_loader.cpp +++ b/ydb/core/kqp/gateway/kqp_metadata_loader.cpp @@ -149,10 +149,14 @@ TTableMetadataResult GetTableMetadataResult(const NSchemeCache::TSchemeCacheNavi switch (entry.Kind) { case EKind::KindTable: tableMeta->Kind = NYql::EKikimrTableKind::Datashard; + tableMeta->TableType = NYql::ETableType::Table; + tableMeta->StoreType = NYql::EStoreType::Row; break; case EKind::KindColumnTable: tableMeta->Kind = NYql::EKikimrTableKind::Olap; + tableMeta->TableType = NYql::ETableType::Table; + tableMeta->StoreType = NYql::EStoreType::Column; break; default: diff --git a/ydb/core/kqp/host/kqp_gateway_proxy.cpp b/ydb/core/kqp/host/kqp_gateway_proxy.cpp index 3da683e12156..8bbf4d72cc9a 100644 --- a/ydb/core/kqp/host/kqp_gateway_proxy.cpp +++ b/ydb/core/kqp/host/kqp_gateway_proxy.cpp @@ -1524,9 +1524,7 @@ class TKqpGatewayProxy : public IKikimrGateway { } } - TFuture AlterColumnTable(const TString& cluster, - const TAlterColumnTableSettings& settings) override - { + TFuture AlterColumnTable(const TString& cluster, Ydb::Table::AlterTableRequest&& req) override { CHECK_PREPARED_DDL(AlterColumnTable); try { @@ -1534,20 +1532,16 @@ class TKqpGatewayProxy : public IKikimrGateway { return MakeFuture(ResultFromError("Invalid cluster: " + cluster)); } - std::pair pathPair; - { - TString error; - if (!NSchemeHelpers::SplitTablePath(settings.Table, GetDatabase(), pathPair, error, false)) { - return MakeFuture(ResultFromError(error)); - } - } - NKikimrSchemeOp::TModifyScheme schemeTx; - schemeTx.SetWorkingDir(pathPair.first); - schemeTx.SetOperationType(NKikimrSchemeOp::ESchemeOpAlterColumnTable); - NKikimrSchemeOp::TAlterColumnTable* alter = schemeTx.MutableAlterColumnTable(); - alter->SetName(settings.Table); + Ydb::StatusIds::StatusCode code; + TString error; + if (!BuildAlterColumnTableModifyScheme(&req, &schemeTx, code, error)) { + IKqpGateway::TGenericResult errResult; + errResult.AddIssue(NYql::TIssue(error)); + errResult.SetStatus(NYql::YqlStatusFromYdbStatus(code)); + return MakeFuture(errResult); + } if (IsPrepare()) { auto& phyQuery = *SessionCtx->Query().PreparingQuery->MutablePhysicalQuery(); diff --git a/ydb/core/kqp/provider/yql_kikimr_exec.cpp b/ydb/core/kqp/provider/yql_kikimr_exec.cpp index 4088bd30a0e4..048f60728a6b 100644 --- a/ydb/core/kqp/provider/yql_kikimr_exec.cpp +++ b/ydb/core/kqp/provider/yql_kikimr_exec.cpp @@ -292,12 +292,6 @@ namespace { }; } - TAlterColumnTableSettings ParseAlterColumnTableSettings(TKiAlterTable alter) { - return TAlterColumnTableSettings{ - .Table = TString(alter.Table()) - }; - } - TSequenceSettings ParseSequenceSettings(const TCoNameValueTupleList& sequenceSettings) { TSequenceSettings result; for (const auto& setting: sequenceSettings) { @@ -1979,10 +1973,11 @@ class TKiSinkCallableExecutionTransformer : public TAsyncCallbackTransformer future; - bool isTableStore = (table.Metadata->TableType == ETableType::TableStore); + bool isTableStore = (table.Metadata->TableType == ETableType::TableStore); // Doesn't set, so always false bool isColumn = (table.Metadata->StoreType == EStoreType::Column); if (isTableStore) { + AFL_VERIFY(false); if (!isColumn) { ctx.AddError(TIssue(ctx.GetPosition(input->Pos()), TStringBuilder() << "TABLESTORE with not COLUMN store")); @@ -1990,7 +1985,7 @@ class TKiSinkCallableExecutionTransformer : public TAsyncCallbackTransformerAlterTableStore(cluster, ParseAlterTableStoreSettings(maybeAlter.Cast())); } else if (isColumn) { - future = Gateway->AlterColumnTable(cluster, ParseAlterColumnTableSettings(maybeAlter.Cast())); + future = Gateway->AlterColumnTable(cluster, std::move(alterTableRequest)); } else { TMaybe requestType; if (!SessionCtx->Query().DocumentApiRestricted) { diff --git a/ydb/core/kqp/provider/yql_kikimr_gateway.h b/ydb/core/kqp/provider/yql_kikimr_gateway.h index 0002b3df26c8..7963fa1399bb 100644 --- a/ydb/core/kqp/provider/yql_kikimr_gateway.h +++ b/ydb/core/kqp/provider/yql_kikimr_gateway.h @@ -1061,7 +1061,7 @@ class IKikimrGateway : public TThrRefBase { virtual NThreading::TFuture CreateColumnTable( TKikimrTableMetadataPtr metadata, bool createDir, bool existingOk = false) = 0; - virtual NThreading::TFuture AlterColumnTable(const TString& cluster, const TAlterColumnTableSettings& settings) = 0; + virtual NThreading::TFuture AlterColumnTable(const TString& cluster, Ydb::Table::AlterTableRequest&& req) = 0; virtual NThreading::TFuture CreateTableStore(const TString& cluster, const TCreateTableStoreSettings& settings, bool existingOk = false) = 0; diff --git a/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp b/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp index 8fb77f45f5e3..02d906b472d4 100644 --- a/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp +++ b/ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp @@ -5116,6 +5116,7 @@ Y_UNIT_TEST_SUITE(KqpScheme) { Y_UNIT_TEST(AlterColumnTableTiering) { TKikimrSettings runnerSettings; runnerSettings.WithSampleTables = false; + runnerSettings.SetEnableTieringInColumnShard(true); TKikimrRunner kikimr(runnerSettings); auto db = kikimr.GetTableClient(); auto session = db.CreateSession().GetValueSync().GetSession(); diff --git a/ydb/core/ydb_convert/table_description.cpp b/ydb/core/ydb_convert/table_description.cpp index 5fce0f1f88a7..ecaa606e448d 100644 --- a/ydb/core/ydb_convert/table_description.cpp +++ b/ydb/core/ydb_convert/table_description.cpp @@ -733,10 +733,17 @@ bool FillColumnDescription(NKikimrSchemeOp::TTableDescription& out, return true; } -bool FillColumnDescription(NKikimrSchemeOp::TColumnTableDescription& out, - const google::protobuf::RepeatedPtrField& in, Ydb::StatusIds::StatusCode& status, TString& error) { - auto* schema = out.MutableSchema(); +NKikimrSchemeOp::TOlapColumnDescription* GetAddColumn(NKikimrSchemeOp::TColumnTableDescription& out) { + return out.MutableSchema()->AddColumns(); +} + +NKikimrSchemeOp::TOlapColumnDescription* GetAddColumn(NKikimrSchemeOp::TAlterColumnTable& out) { + return out.MutableAlterSchema()->AddAddColumns(); +} +template +bool FillColumnDescriptionImpl(TColumnTable& out, const google::protobuf::RepeatedPtrField& in, + Ydb::StatusIds::StatusCode& status, TString& error) { for (const auto& column : in) { if (column.type().has_pg_type()) { status = Ydb::StatusIds::BAD_REQUEST; @@ -744,7 +751,7 @@ bool FillColumnDescription(NKikimrSchemeOp::TColumnTableDescription& out, return false; } - auto* columnDesc = schema->AddColumns(); + auto* columnDesc = GetAddColumn(out); columnDesc->SetName(column.name()); NScheme::TTypeInfo typeInfo; @@ -763,6 +770,81 @@ bool FillColumnDescription(NKikimrSchemeOp::TColumnTableDescription& out, return true; } +bool FillColumnDescription(NKikimrSchemeOp::TColumnTableDescription& out, const google::protobuf::RepeatedPtrField& in, + Ydb::StatusIds::StatusCode& status, TString& error) { + return FillColumnDescriptionImpl(out, in, status, error); +} + +bool FillColumnDescription(NKikimrSchemeOp::TAlterColumnTable& out, const google::protobuf::RepeatedPtrField& in, + Ydb::StatusIds::StatusCode& status, TString& error) { + return FillColumnDescriptionImpl(out, in, status, error); +} + +bool BuildAlterColumnTableModifyScheme(const TString& path, const Ydb::Table::AlterTableRequest* req, + NKikimrSchemeOp::TModifyScheme* modifyScheme, Ydb::StatusIds::StatusCode& status, TString& error) { + const auto ops = GetAlterOperationKinds(req); + if (ops.empty()) { + status = Ydb::StatusIds::BAD_REQUEST; + error = "Empty alter"; + return false; + } + + if (ops.size() > 1) { + status = Ydb::StatusIds::UNSUPPORTED; + error = "Mixed alter is unsupported"; + return false; + } + + const auto OpType = *ops.begin(); + + std::pair pathPair; + try { + pathPair = SplitPathIntoWorkingDirAndName(path); + } catch (const std::exception&) { + status = Ydb::StatusIds::BAD_REQUEST; + return false; + } + + const auto& workingDir = pathPair.first; + const auto& name = pathPair.second; + modifyScheme->SetWorkingDir(workingDir); + + if (OpType == EAlterOperationKind::Common) { + auto alterColumnTable = modifyScheme->MutableAlterColumnTable(); + alterColumnTable->SetName(name); + modifyScheme->SetOperationType(NKikimrSchemeOp::EOperationType::ESchemeOpAlterColumnTable); + + for (const auto& drop : req->drop_columns()) { + alterColumnTable->MutableAlterSchema()->AddDropColumns()->SetName(drop); + } + + if (!FillColumnDescription(*alterColumnTable, req->add_columns(), status, error)) { + return false; + } + + if (req->has_set_ttl_settings()) { + if (!FillTtlSettings(*alterColumnTable->MutableAlterTtlSettings()->MutableEnabled(), req->Getset_ttl_settings(), status, error)) { + return false; + } + } else if (req->has_drop_ttl_settings()) { + alterColumnTable->MutableAlterTtlSettings()->MutableDisabled(); + } + + if (req->has_set_tiering()) { + alterColumnTable->MutableAlterTtlSettings()->SetUseTiering(req->set_tiering()); + } else if (req->has_drop_tiering()) { + alterColumnTable->MutableAlterTtlSettings()->SetUseTiering(""); + } + } + + return true; +} + +bool BuildAlterColumnTableModifyScheme( + const Ydb::Table::AlterTableRequest* req, NKikimrSchemeOp::TModifyScheme* modifyScheme, Ydb::StatusIds::StatusCode& code, TString& error) { + return BuildAlterColumnTableModifyScheme(req->path(), req, modifyScheme, code, error); +} + template void FillTableBoundaryImpl(TYdbProto& out, const NKikimrSchemeOp::TTableDescription& in, const NKikimrMiniKQL::TType& splitKeyType) { @@ -1667,4 +1749,4 @@ bool FillSequenceDescription(NKikimrSchemeOp::TSequenceDescription& out, const Y return true; } -} // namespace NKikimr +} // namespace NKikimr diff --git a/ydb/core/ydb_convert/table_description.h b/ydb/core/ydb_convert/table_description.h index aada5d1f767c..636e6461eb6b 100644 --- a/ydb/core/ydb_convert/table_description.h +++ b/ydb/core/ydb_convert/table_description.h @@ -37,6 +37,10 @@ bool BuildAlterTableModifyScheme(const TString& path, const Ydb::Table::AlterTab bool BuildAlterTableModifyScheme(const Ydb::Table::AlterTableRequest* req, NKikimrSchemeOp::TModifyScheme* modifyScheme, const TTableProfiles& profiles, const TPathId& resolvedPathId, Ydb::StatusIds::StatusCode& status, TString& error); +bool BuildAlterColumnTableModifyScheme(const TString& path, const Ydb::Table::AlterTableRequest* req, + NKikimrSchemeOp::TModifyScheme* modifyScheme, Ydb::StatusIds::StatusCode& status, TString& error); +bool BuildAlterColumnTableModifyScheme( + const Ydb::Table::AlterTableRequest* req, NKikimrSchemeOp::TModifyScheme* modifyScheme, Ydb::StatusIds::StatusCode& status, TString& error); bool FillAlterTableSettingsDesc(NKikimrSchemeOp::TTableDescription& out, const Ydb::Table::AlterTableRequest& in, const TTableProfiles& profiles, @@ -55,8 +59,10 @@ void FillColumnDescription(Ydb::Table::DescribeTableResult& out, const NKikimrSc // in bool FillColumnDescription(NKikimrSchemeOp::TTableDescription& out, const google::protobuf::RepeatedPtrField& in, Ydb::StatusIds::StatusCode& status, TString& error); -bool FillColumnDescription(NKikimrSchemeOp::TColumnTableDescription& out, - const google::protobuf::RepeatedPtrField& in, Ydb::StatusIds::StatusCode& status, TString& error); +bool FillColumnDescription(NKikimrSchemeOp::TColumnTableDescription& out, const google::protobuf::RepeatedPtrField& in, + Ydb::StatusIds::StatusCode& status, TString& error); +bool FillColumnDescription(NKikimrSchemeOp::TAlterColumnTable& out, const google::protobuf::RepeatedPtrField& in, + Ydb::StatusIds::StatusCode& status, TString& error); bool ExtractColumnTypeInfo(NScheme::TTypeInfo& outTypeInfo, TString& outTypeMod, const Ydb::Type& inType, Ydb::StatusIds::StatusCode& status, TString& error);